diff options
author | David Kalnischkies <david@kalnischkies.de> | 2021-06-04 13:06:34 +0200 |
---|---|---|
committer | David Kalnischkies <david@kalnischkies.de> | 2021-06-04 16:43:41 +0200 |
commit | 149b23c2b9697bc262c0af1934c7a3f6114d903f (patch) | |
tree | 4d785c2b812e1284d1317e30ebd6df18fbb959cd | |
parent | aeae140b11220c8ca3692ef690bc51578f197992 (diff) |
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
-rw-r--r-- | apt-pkg/acquire-item.cc | 12 | ||||
-rw-r--r-- | apt-pkg/indexfile.cc | 5 | ||||
-rwxr-xr-x | test/integration/test-apt-get-install-deb | 11 | ||||
-rwxr-xr-x | test/integration/test-bug-723705-tagfile-truncates-fields | 4 | ||||
-rwxr-xr-x | test/integration/test-uri-encode-filename-field | 42 |
5 files changed, 59 insertions, 15 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/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/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..136cce8d2 --- /dev/null +++ b/test/integration/test-uri-encode-filename-field @@ -0,0 +1,42 @@ +#!/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 + + mv '../aptarchive/pool/foo_0+0~0_all.deb' '../aptarchive/pool/foo_0%3a0+0~0_all.deb' + testsuccess apt purge foo -y + testfailure apt download foo + testfailure apt install foo + + sed -i -e 's#_0+0~0_#_0%3a0+0~0_#' ../rootdir/var/lib/apt/lists/*Packages + testsuccess apt download foo + # FIXME: we shouldn't take filename from file:/ in 'apt download' + if [ "$1" = 'file' ]; then + testsuccess rm 'foo_0%3a0+0~0_all.deb' + else + testsuccess rm 'foo_0+0~0_all.deb' + fi + testsuccess apt install foo + + cd "$TMPWORKINGDIRECTORY" >/dev/null + mv 'aptarchive/pool/foo_0%3a0+0~0_all.deb' 'aptarchive/pool/foo_0+0~0_all.deb' +} + +runtest 'file' +changetowebserver +runtest 'http' |