summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Kalnischkies <david@kalnischkies.de>2021-06-04 13:06:34 +0200
committerDavid Kalnischkies <david@kalnischkies.de>2021-06-04 16:43:41 +0200
commit149b23c2b9697bc262c0af1934c7a3f6114d903f (patch)
tree4d785c2b812e1284d1317e30ebd6df18fbb959cd
parentaeae140b11220c8ca3692ef690bc51578f197992 (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.cc12
-rw-r--r--apt-pkg/indexfile.cc5
-rwxr-xr-xtest/integration/test-apt-get-install-deb11
-rwxr-xr-xtest/integration/test-bug-723705-tagfile-truncates-fields4
-rwxr-xr-xtest/integration/test-uri-encode-filename-field42
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'