From 149b23c2b9697bc262c0af1934c7a3f6114d903f Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 4 Jun 2021 13:06:34 +0200 Subject: URI encode Filename field of Packages files (again) Keeping URIs encoded in the acquire system depends on having them encoded in the first place. While many other places got the encoding 2 out of 3 ArchiveURI implementations were missed which are in practice responsible for nearly all of the URI building, just that index filename do not contain characters to escape and the Filename fields in Packages files usually aren't. Usually. Except if you happen to have e.g. an epoch featuring package with the colon encoded in the filename. On the upside, in most repositories the epoch isn't part of the filename. Reported-By: Johannes 'josch' Schauer on IRC References: e6c55283d235aa9404395d30f2db891f36995c49 --- apt-pkg/acquire-item.cc | 12 ++++++------ apt-pkg/indexfile.cc | 5 +++-- 2 files changed, 9 insertions(+), 8 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 225c4304a..d6ffaf34d 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -3853,16 +3853,16 @@ pkgAcqFile::pkgAcqFile(pkgAcquire *const Owner, string const &URI, HashStringLis const string &DestDir, const string &DestFilename, bool const IsIndexFile) : Item(Owner), d(NULL), IsIndexFile(IsIndexFile), ExpectedHashes(Hashes) { + ::URI url{URI}; + if (url.Path.find(' ') != std::string::npos || url.Path.find('%') == std::string::npos) + url.Path = pkgAcquire::URIEncode(url.Path); + if(!DestFilename.empty()) DestFile = DestFilename; else if(!DestDir.empty()) - DestFile = DestDir + "/" + flNotDir(URI); + DestFile = DestDir + "/" + DeQuoteString(flNotDir(url.Path)); else - DestFile = flNotDir(URI); - - ::URI url{URI}; - if (url.Path.find(' ') != std::string::npos || url.Path.find('%') == std::string::npos) - url.Path = pkgAcquire::URIEncode(url.Path); + DestFile = DeQuoteString(flNotDir(url.Path)); // Create the item Desc.URI = std::string(url); diff --git a/apt-pkg/indexfile.cc b/apt-pkg/indexfile.cc index ef3486ab8..f13b52940 100644 --- a/apt-pkg/indexfile.cc +++ b/apt-pkg/indexfile.cc @@ -9,6 +9,7 @@ // Include Files /*{{{*/ #include +#include #include #include #include @@ -174,7 +175,7 @@ pkgDebianIndexTargetFile::pkgDebianIndexTargetFile(IndexTarget const &Target, bo /*}}}*/ std::string pkgDebianIndexTargetFile::ArchiveURI(std::string const &File) const/*{{{*/ { - return Target.Option(IndexTarget::REPO_URI) + File; + return Target.Option(IndexTarget::REPO_URI) + pkgAcquire::URIEncode(File); } /*}}}*/ std::string pkgDebianIndexTargetFile::Describe(bool const Short) const /*{{{*/ @@ -281,7 +282,7 @@ std::string pkgDebianIndexRealFile::Describe(bool const /*Short*/) const/*{{{*/ /*}}}*/ std::string pkgDebianIndexRealFile::ArchiveURI(std::string const &/*File*/) const/*{{{*/ { - return "file:" + File; + return "file:" + pkgAcquire::URIEncode(File); } /*}}}*/ std::string pkgDebianIndexRealFile::IndexFileName() const /*{{{*/ -- cgit v1.2.3-70-g09d2 From a2406cda4dd0aca523183ed6a8b651f06e0e63f9 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 4 Jun 2021 16:15:45 +0200 Subject: No URL decode and quoting support for Files in Sources The code exists since ever, but no other client supports this and the specification like debian-policy isn't asking for this either. What it does do is breaking than all others continue working through: If the filename includes in fact URI encoded bits (hopefully no quotes) which is rather unlikely, but none the less possible. --- apt-pkg/deb/debsrcrecords.cc | 30 ++++++++++++------------- test/integration/test-uri-encode-filename-field | 10 +++++++++ 2 files changed, 24 insertions(+), 16 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/debsrcrecords.cc b/apt-pkg/deb/debsrcrecords.cc index 89f3f1667..1fd0fdc12 100644 --- a/apt-pkg/deb/debsrcrecords.cc +++ b/apt-pkg/deb/debsrcrecords.cc @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -170,6 +171,7 @@ bool debSrcRecordParser::Files(std::vector &List) std::vector const compExts = APT::Configuration::getCompressorExtensions(); + auto const &posix = std::locale::classic(); for (char const * const * type = HashString::SupportedHashes(); *type != NULL; ++type) { // derive field from checksum type @@ -182,27 +184,23 @@ bool debSrcRecordParser::Files(std::vector &List) string const Files = Sect.FindS(checksumField.c_str()); if (Files.empty() == true) continue; + std::istringstream ss(Files); + ss.imbue(posix); - // Iterate over the entire list grabbing each triplet - const char *C = Files.c_str(); - while (*C != 0) + while (ss.good()) { - string hash, size, path; - - // Parse each of the elements - if (ParseQuoteWord(C, hash) == false || - ParseQuoteWord(C, size) == false || - ParseQuoteWord(C, path) == false) - return _error->Error("Error parsing file record in %s of source package %s", checksumField.c_str(), Package().c_str()); - + std::string hash, path; + unsigned long long size; if (iIndex == nullptr && checksumField == "Files") { - // the Files field has a different format than the rest in deb-changes files std::string ignore; - if (ParseQuoteWord(C, ignore) == false || - ParseQuoteWord(C, path) == false) - return _error->Error("Error parsing file record in %s of source package %s", checksumField.c_str(), Package().c_str()); + ss >> hash >> size >> ignore >> ignore >> path; } + else + ss >> hash >> size >> path; + + if (ss.fail() || hash.empty() || path.empty()) + return _error->Error("Error parsing file record in %s of source package %s", checksumField.c_str(), Package().c_str()); HashString const hashString(*type, hash); if (Base.empty() == false) @@ -226,7 +224,7 @@ bool debSrcRecordParser::Files(std::vector &List) // we haven't seen this file yet pkgSrcRecords::File F; F.Path = path; - F.FileSize = strtoull(size.c_str(), NULL, 10); + F.FileSize = size; F.Hashes.push_back(hashString); F.Hashes.FileSize(F.FileSize); diff --git a/test/integration/test-uri-encode-filename-field b/test/integration/test-uri-encode-filename-field index dffee21aa..233332d0a 100755 --- a/test/integration/test-uri-encode-filename-field +++ b/test/integration/test-uri-encode-filename-field @@ -17,19 +17,29 @@ runtest() { testsuccess apt download foo testsuccess rm 'foo_0+0~0_all.deb' testsuccess apt install foo + testsuccess apt source foo + testsuccess rm 'foo_0+0~0.dsc' 'foo_0+0~0.tar.xz' + testsuccess rm -r 'foo-0+0~0' mv '../aptarchive/pool/foo_0+0~0_all.deb' '../aptarchive/pool/foo_0%3a0+0~0_all.deb' + mv '../aptarchive/pool/foo_0+0~0.dsc' '../aptarchive/pool/foo_0%3a0+0~0.dsc' testsuccess apt purge foo -y testfailure apt download foo testfailure apt install foo + testfailure apt source foo --dsc-only sed -i -e 's#_0+0~0_#_0%3a0+0~0_#' ../rootdir/var/lib/apt/lists/*Packages + sed -i -e 's#_0+0~0.d#_0%3a0+0~0.d#' ../rootdir/var/lib/apt/lists/*Sources testsuccess apt download foo testsuccess rm 'foo_0+0~0_all.deb' testsuccess apt install foo + testsuccess apt source foo + testsuccess rm 'foo_0%3a0+0~0.dsc' 'foo_0+0~0.tar.xz' + testsuccess rm -r 'foo-0+0~0' cd "$TMPWORKINGDIRECTORY" >/dev/null mv 'aptarchive/pool/foo_0%3a0+0~0_all.deb' 'aptarchive/pool/foo_0+0~0_all.deb' + mv 'aptarchive/pool/foo_0%3a0+0~0.dsc' 'aptarchive/pool/foo_0+0~0.dsc' } runtest 'file' -- cgit v1.2.3-70-g09d2