Merge branch 'fix/uriencodefilename' into 'main'

URI encode filename fields (again)

See merge request apt-team/apt!175

8 files changed, 91 insertions, 37 deletions

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/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 <algorithm>
 #include <string>
+#include <sstream>
 #include <vector>
 #include <ctype.h>
 #include <stdlib.h>
@@ -170,6 +171,7 @@ bool debSrcRecordParser::Files(std::vector<pkgSrcRecords::File> &List)
 
    std::vector<std::string> 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<pkgSrcRecords::File> &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<pkgSrcRecords::File> &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/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 <config.h>
 
+#include <apt-pkg/acquire.h>
 #include <apt-pkg/aptconfiguration.h>
 #include <apt-pkg/configuration.h>
 #include <apt-pkg/deblistparser.h>
@@ -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			/*{{{*/
diff --git a/apt-private/private-download.cc b/apt-private/private-download.cc
index 16d11255b..eddb901d0 100644
--- a/apt-private/private-download.cc
+++ b/apt-private/private-download.cc
@@ -211,6 +211,7 @@ bool DoDownload(CommandLine &CmdL)
 	       I->Owner->FileSize << ' ' << I->Owner->HashSum() << std::endl;
       return true;
    }
+   auto const storecopy = storefile;
 
    if (_error->PendingError() == true || CheckAuth(Fetcher, false) == false)
       return false;
@@ -220,19 +221,22 @@ bool DoDownload(CommandLine &CmdL)
       return false;
 
    // copy files in local sources to the current directory
+   i = 0;
    for (pkgAcquire::ItemIterator I = Fetcher.ItemsBegin(); I != Fetcher.ItemsEnd(); ++I)
    {
-      std::string const filename = cwd + flNotDir((*I)->DestFile);
+      if (dynamic_cast<pkgAcqArchive*>(*I) == nullptr)
+	 continue;
+
       if ((*I)->Local == true &&
-          filename != (*I)->DestFile &&
-          (*I)->Status == pkgAcquire::Item::StatDone &&
-	  dynamic_cast<pkgAcqArchive*>(*I) != nullptr)
+	  (*I)->Status == pkgAcquire::Item::StatDone &&
+	  (*I)->DestFile != storecopy[i])
       {
 	 std::ifstream src((*I)->DestFile.c_str(), std::ios::binary);
-	 std::ofstream dst(filename.c_str(), std::ios::binary);
+	 std::ofstream dst(storecopy[i].c_str(), std::ios::binary);
 	 dst << src.rdbuf();
-	 chmod(filename.c_str(), 0644);
+	 chmod(storecopy[i].c_str(), 0644);
       }
+      ++i;
    }
    return Failed == false;
 }
diff --git a/test/integration/framework b/test/integration/framework
index 412a96577..f14b4da64 100644
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1776,6 +1776,9 @@ msgfailoutput() {
 			shift
 		done
 		echo '#### cmp output ####'
+	elif [ "$1" = 'rm' ]; then
+		echo "#### Directory listing of: $(pwd) ####"
+		ls -l
 	fi
 	catfile "$OUTPUT"
 	msgfail "$MSG"
diff --git a/test/integration/test-apt-get-install-deb b/test/integration/test-apt-get-install-deb
index 6fde00708..3d0b9a80d 100755
--- a/test/integration/test-apt-get-install-deb
+++ b/test/integration/test-apt-get-install-deb
@@ -209,30 +209,31 @@ echo "Package: pkg-as-it-should-be
 Architecture: all
 Version: 0
 Installed-Size: 2903
-Filename: incoming/pkg-as-it-should-be_0_all.deb
+Filename: incoming/pkg-as-it-should-be_0%3a0+0_all.deb
 Size: $(stat -c %s incoming/pkg-as-it-should-be_0_all.deb)
 SHA256: $(sha256sum incoming/pkg-as-it-should-be_0_all.deb | cut -d' ' -f 1)
 " > Packages
+ln -s pkg-as-it-should-be_0_all.deb incoming/pkg-as-it-should-be_0%3a0+0_all.deb
 testdpkgnotinstalled 'pkg-as-it-should-be'
 testnopackage pkg-as-it-should-be
 testsuccess apt install --with-source ./Packages pkg-as-it-should-be -s
 testsuccess apt install --with-source ./Packages pkg-as-it-should-be --print-uris
 testsuccess apt show --with-source ./Packages pkg-as-it-should-be
 testequal 'Package: pkg-as-it-should-be' head -n1 rootdir/tmp/testsuccess.output
-testsuccess apt install -y --with-source ./Packages pkg-as-it-should-be
+testsuccess apt install -y --with-source ./Packages pkg-as-it-should-be -o Debug::pkgAcquire::Worker=1
 testdpkginstalled 'pkg-as-it-should-be'
 rm -f ./Packages
 
 echo 'dpkg::install::recursive "true";
 dpkg::install::recursive::force "true";
 dpkg::install::recursive::minimum "0";' > rootdir/etc/apt/apt.conf.d/lowerminimum.conf
-mv ./incoming/pkg-as-it-should-be_0_all.deb ./incoming/pkg-as-it-should-be_0_all.ddeb
-testsuccess aptget install -y ./incoming/pkg-as-it-should-be_0_all.ddeb --reinstall
+mv ./incoming/pkg-as-it-should-be_0_all.deb ./incoming/pkg-as-it-should-be_0%3a0+0_all.ddeb
+testsuccess aptget install -y ./incoming/pkg-as-it-should-be_0%3a0+0_all.ddeb --reinstall
 testfailure grep 'is already the newest version' rootdir/tmp/testsuccess.output
 testsuccess apt purge -y pkg-as-it-should-be
 testdpkgnotinstalled 'pkg-as-it-should-be'
 
-mv ./incoming/pkg-as-it-should-be_0_all.ddeb ./incoming/pkg-as-it-should-be_0_all.foobar
+mv ./incoming/pkg-as-it-should-be_0%3a0+0_all.ddeb ./incoming/pkg-as-it-should-be_0_all.foobar
 echo "Package: pkg-as-it-should-be
 Architecture: all
 Version: 0
diff --git a/test/integration/test-bug-723705-tagfile-truncates-fields b/test/integration/test-bug-723705-tagfile-truncates-fields
index bfa3598ae..d167a3f65 100755
--- a/test/integration/test-bug-723705-tagfile-truncates-fields
+++ b/test/integration/test-bug-723705-tagfile-truncates-fields
@@ -26,8 +26,8 @@ After this operation, 19.8 MB of additional disk space will be used.
 'file:///tmp/aptarchive/pool/main/c/cdebconf/cdebconf-gtk-udeb_0.185_amd64.udeb' cdebconf-gtk-udeb_0.185_amd64.udeb 27278 MD5Sum:a1bbbc1d4fb8e0615b5621abac021924
 'file:///tmp/aptarchive/pool/main/c/cdebconf/cdebconf-newt-udeb_0.185_amd64.udeb' cdebconf-newt-udeb_0.185_amd64.udeb 19192 MD5Sum:de27807f56dae2f2403b3322d5fe6bd2
 'file:///tmp/aptarchive/pool/main/g/glib2.0/libglib2.0-udeb_2.36.4-1_amd64.udeb' libglib2.0-udeb_2.36.4-1_amd64.udeb 1714604 MD5Sum:72da029f1bbb36057d874f1f82a5d00a
-'file:///tmp/aptarchive/pool/main/e/eglibc/libc6-udeb_2.17-92+b1_amd64.udeb' libc6-udeb_2.17-92+b1_amd64.udeb 1056000 MD5Sum:7fd7032eeeecf7f76eff79a0543fbd72
-'file:///tmp/aptarchive/pool/main/g/gtk+2.0/libgtk2.0-0-udeb_2.24.20-1_amd64.udeb' libgtk2.0-0-udeb_2.24.20-1_amd64.udeb 1643046 MD5Sum:25513478eb2e02e5766c0eea0b411ca9
+'file:///tmp/aptarchive/pool/main/e/eglibc/libc6-udeb_2.17-92%2bb1_amd64.udeb' libc6-udeb_2.17-92+b1_amd64.udeb 1056000 MD5Sum:7fd7032eeeecf7f76eff79a0543fbd72
+'file:///tmp/aptarchive/pool/main/g/gtk%2b2.0/libgtk2.0-0-udeb_2.24.20-1_amd64.udeb' libgtk2.0-0-udeb_2.24.20-1_amd64.udeb 1643046 MD5Sum:25513478eb2e02e5766c0eea0b411ca9
 'file:///tmp/aptarchive/pool/main/v/vte/libvte9-udeb_0.28.2-5_amd64.udeb' libvte9-udeb_1%3a0.28.2-5_amd64.udeb 216968 MD5Sum:7da7201effaf5ced19abd9d0b45aa2c6
 'file:///tmp/aptarchive/pool/main/c/cdebconf-terminal/cdebconf-gtk-terminal_0.22_amd64.udeb' cdebconf-gtk-terminal_0.22_amd64.udeb 14734 MD5Sum:f9c3a7354560cb88e0396e2b7ba54363
 'file:///tmp/aptarchive/pool/main/c/cdebconf-terminal/cdebconf-newt-terminal_0.22_amd64.udeb' cdebconf-newt-terminal_0.22_amd64.udeb 4538 MD5Sum:20db6152fce5081fcbf49c7c08f21246"
diff --git a/test/integration/test-uri-encode-filename-field b/test/integration/test-uri-encode-filename-field
new file mode 100755
index 000000000..233332d0a
--- /dev/null
+++ b/test/integration/test-uri-encode-filename-field
@@ -0,0 +1,47 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'amd64'
+
+buildsimplenativepackage 'foo' 'all' '0+0~0'
+setupaptarchive --no-update
+
+runtest() {
+	rm -rf rootdir/var/lib/apt/lists
+	testsuccess apt update
+
+	cd downloaded
+	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'
+changetowebserver
+runtest 'http'