From 97be873d782c5e9aaa8b4f4f4e6e18805d0fa51c Mon Sep 17 00:00:00 2001
From: David Kalnischkies <david@kalnischkies.de>
Date: Wed, 8 Jul 2020 17:51:40 +0200
Subject: Proper URI encoding for config requests to our test webserver

Our http method encodes the URI again which results in the double
encoding we have unwrap in the webserver (we did already, but we skip
the filename handling now which does the first decode).
---
 cmdline/apt-helper.cc                                 |  9 +++++++++
 test/integration/framework                            |  5 ++---
 test/integration/test-cve-2019-3462-dequote-injection | 15 ++++++++-------
 test/interactive-helper/aptwebserver.cc               | 14 ++++++++++----
 4 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/cmdline/apt-helper.cc b/cmdline/apt-helper.cc
index 3d6a692e0..85795e0d2 100644
--- a/cmdline/apt-helper.cc
+++ b/cmdline/apt-helper.cc
@@ -284,6 +284,14 @@ static bool AnalyzePattern(CommandLine &CmdL)				/*{{{*/
       return false;
    }
 
+   return true;
+}
+									/*}}}*/
+static bool DoQuoteString(CommandLine &CmdL)				/*{{{*/
+{
+   if (CmdL.FileSize() != 3)
+      return _error->Error("Expect two arguments, a string to quote and a string of additional characters to quote");
+   std::cout << QuoteString(CmdL.FileList[1], CmdL.FileList[2]) << '\n';
    return true;
 }
 									/*}}}*/
@@ -310,6 +318,7 @@ static std::vector<aptDispatchWithHelp> GetCommands()			/*{{{*/
        {"drop-privs", &DropPrivsAndRun, _("drop privileges before running given command")},
        {"analyze-pattern", &AnalyzePattern, _("analyse a pattern")},
        {"analyse-pattern", &AnalyzePattern, nullptr},
+       {"quote-string", &DoQuoteString, nullptr},
        {nullptr, nullptr, nullptr}};
 }
 									/*}}}*/
diff --git a/test/integration/framework b/test/integration/framework
index e30fa066c..20173da23 100644
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1327,14 +1327,13 @@ webserverconfig() {
 	local DOWNLOG="${TMPWORKINGDIRECTORY}/rootdir/tmp/download-testfile.log"
 	local STATUS="${TMPWORKINGDIRECTORY}/downloaded/webserverconfig.status"
 	rm -f "$STATUS" "$DOWNLOG"
-	# very very basic URI encoding
 	local URI
 	if [ -n "$2" ]; then
 		msgtest "Set webserver config option '${1}' to" "$2"
-		URI="${WEBSERVER}/_config/set/$(echo "${1}" | sed -e 's/\//%2f/g')/$(echo "${2}" | sed -e 's/\//%2f/g')"
+		URI="${WEBSERVER}/_config/set/$(apthelper quote-string "${1}" '/?#')/$(apthelper quote-string "${2}" '/?#')"
 	else
 		msgtest 'Clear webserver config option' "${1}"
-		URI="${WEBSERVER}/_config/clear/$(echo "${1}" | sed -e 's/\//%2f/g')"
+		URI="${WEBSERVER}/_config/clear/$(apthelper quote-string "${1}" '/?#')"
 	fi
 	if downloadfile "$URI" "$STATUS" > "$DOWNLOG"; then
 		msgpass
diff --git a/test/integration/test-cve-2019-3462-dequote-injection b/test/integration/test-cve-2019-3462-dequote-injection
index a1adec6de..74ab03ba5 100755
--- a/test/integration/test-cve-2019-3462-dequote-injection
+++ b/test/integration/test-cve-2019-3462-dequote-injection
@@ -15,13 +15,12 @@ ORIGINAL_SIZE=$(wc -c aptarchive/pool/alpha_1_all.deb | awk '{print $1}')
 SHA256="DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF"
 changetowebserver
 
-webserverconfig aptwebserver::redirect::replace::alpha_1_all.deb "beeta_1_all.deb%250a%250a201%2520URI%2520Done%250aURI:%2520http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%250aFilename:%2520${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%250aSize:%252020672%250aLast-Modified:%2520Fri,%252018%2520Jan%25202019%252009:52:02%2520+0000%250aSHA256-Hash:%2520${SHA256}%250aChecksum-FileSize-Hash:%252012345%250a%250a%0a"
+runwithbaduri() {
+	webserverconfig aptwebserver::redirect::replace::alpha_1_all.deb "$1"
 
+	testsuccess apt update -o debug::http=1 -o debug::pkgacquire::worker=1
 
-testsuccess apt update -o debug::http=1 -o debug::pkgacquire::worker=1
-
-
-testfailureequal "Reading package lists...
+	testfailureequal "Reading package lists...
 Building dependency tree...
 The following NEW packages will be installed:
   alpha
@@ -32,8 +31,10 @@ Err:1 http://localhost:${APTHTTPPORT} unstable/main all alpha all 1
   SECURITY: URL redirect target contains control characters, rejecting.
 E: Failed to fetch http://localhost:${APTHTTPPORT}/pool/alpha_1_all.deb  SECURITY: URL redirect target contains control characters, rejecting.
 E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?" aptget install alpha
-
-
+}
+runwithbaduri "beeta_1_all.deb%0a%0a201%20URI%20Done%0aURI:%20http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%0aFilename:%20${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%0aSize:%2020672%0aLast-Modified:%20Fri,%2018%20Jan%202019%2009:52:02%20+0000%0aSHA256-Hash:%20${SHA256}%0aChecksum-FileSize-Hash:%2012345%0a%0a%0a"
+rm -rf rootdir/var/lib/apt/lists
+runwithbaduri "beeta_1_all.deb%250a%250a201%2520URI%2520Done%250aURI:%2520http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%250aFilename:%2520${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%250aSize:%252020672%250aLast-Modified:%2520Fri,%252018%2520Jan%25202019%252009:52:02%2520+0000%250aSHA256-Hash:%2520${SHA256}%250aChecksum-FileSize-Hash:%252012345%250a%250a%0a"
 
 # For reference, the following is the original reproducer/bug. It has
 # been disabled using exit 0, as it will fail in fixed versions.
diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc
index f074cd148..57d215a65 100644
--- a/test/interactive-helper/aptwebserver.cc
+++ b/test/interactive-helper/aptwebserver.cc
@@ -573,6 +573,11 @@ static bool parseFirstLine(std::ostream &log, int const client, std::string cons
       params = filename.substr(paramspos + 1);
       filename.erase(paramspos);
    }
+   else if (APT::String::Startswith(filename, "/_config/"))
+   {
+      filename.erase(0, 1);
+      return true;
+   }
 
    filename = DeQuoteString(filename);
 
@@ -620,11 +625,12 @@ static bool parseFirstLine(std::ostream &log, int const client, std::string cons
 }
 									/*}}}*/
 static bool handleOnTheFlyReconfiguration(std::ostream &log, int const client,/*{{{*/
-      std::string const &request, std::vector<std::string> parts, std::list<std::string> &headers)
+      std::string const &request, std::vector<std::string> const &EncodedParts, std::list<std::string> &headers)
 {
-   size_t const pcount = parts.size();
+   size_t const pcount = EncodedParts.size();
+   std::vector<std::string> parts(pcount);
    for (size_t i = 0; i < pcount; ++i)
-      parts[i] = DeQuoteString(parts[i]);
+      parts[i] = DeQuoteString(DeQuoteString(EncodedParts[i]));
    if (pcount == 4 && parts[1] == "set")
    {
       _config->Set(parts[2], parts[3]);
@@ -707,7 +713,7 @@ static void * handleClient(int const client, size_t const id)		/*{{{*/
 	 // special webserver command request
 	 if (filename.length() > 1 && filename[0] == '_')
 	 {
-	    std::vector<std::string> parts = VectorizeString(filename, '/');
+	    auto const parts = VectorizeString(filename, '/');
 	    if (parts[0] == "_config")
 	    {
 	       handleOnTheFlyReconfiguration(log, client, *m, parts, headers);
-- 
cgit v1.2.3


From e6c55283d235aa9404395d30f2db891f36995c49 Mon Sep 17 00:00:00 2001
From: David Kalnischkies <david@kalnischkies.de>
Date: Thu, 9 Jul 2020 16:38:49 +0200
Subject: Keep URIs encoded in the acquire system

We do not deal a lot with URIs which need encoding, but then we do it is
a pain that we store it decoded in the acquire system as it means we
have to decode and reencode URIs eventually which is potentially giving
us slightly different URIs.

We see that in our own testing framework while setting up redirects as
the config options are effectively double-encoded and decoded to pass
them around successfully as otherwise %2f and / in an URI are treated
the same.

This commit adds the infrastructure for methods to opt into getting URIs
send in encoded form (and returning them to us in encoded form, too) so
that we eventually do not have to touch the URIs which is how it should
be. This means though that we have to deal with methods who do not
support this yet (aka: all at the moment) for which we decode and encode
while communicating with them.
---
 apt-pkg/acquire-item.cc                            | 42 +++++++------
 apt-pkg/acquire-method.cc                          |  3 +
 apt-pkg/acquire-method.h                           |  3 +-
 apt-pkg/acquire-worker.cc                          | 68 +++++++++++++++++-----
 apt-pkg/acquire.cc                                 | 46 +++++++++++++--
 apt-pkg/acquire.h                                  |  4 ++
 apt-pkg/deb/debmetaindex.cc                        | 13 ++++-
 apt-pkg/deb/debmetaindex.h                         |  2 +-
 apt-pkg/update.cc                                  |  2 +
 doc/examples/configure-index                       |  2 +
 doc/method.dbk                                     |  4 +-
 .../test-acquire-same-file-multiple-times          |  4 +-
 test/integration/test-apt-get-source-arch          | 12 ++--
 test/integration/test-apt-get-source-multisources  |  8 +--
 test/integration/test-apt-source-and-build-dep     | 24 ++++----
 .../test-bug-722207-print-uris-even-if-very-quiet  | 12 ++--
 16 files changed, 174 insertions(+), 75 deletions(-)

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 820dc7b59..34528f2ec 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -1380,7 +1380,7 @@ string pkgAcqMetaBase::Custom600Headers() const
 void pkgAcqMetaBase::QueueForSignatureVerify(pkgAcqTransactionItem * const I, std::string const &File, std::string const &Signature)
 {
    AuthPass = true;
-   I->Desc.URI = "gpgv:" + Signature;
+   I->Desc.URI = "gpgv:" + pkgAcquire::URIEncode(Signature);
    I->DestFile = File;
    QueueURI(I->Desc);
    I->SetActiveSubprocess("gpgv");
@@ -1415,7 +1415,7 @@ bool pkgAcqMetaBase::CheckDownloadDone(pkgAcqTransactionItem * const I, const st
    if (FileName != I->DestFile && RealFileExists(I->DestFile) == false)
    {
       I->Local = true;
-      I->Desc.URI = "copy:" + FileName;
+      I->Desc.URI = "copy:" + pkgAcquire::URIEncode(FileName);
       I->QueueURI(I->Desc);
       return false;
    }
@@ -2888,9 +2888,10 @@ bool pkgAcqIndexDiffs::QueueNextDiff()					/*{{{*/
    }
 
    // queue the right diff
-   Desc.URI = Target.URI + ".diff/" + available_patches[0].file + ".gz";
+   auto const BaseFileURI = Target.URI + ".diff/" + pkgAcquire::URIEncode(available_patches[0].file);
+   Desc.URI = BaseFileURI + ".gz";
    Desc.Description = Target.Description + " " + available_patches[0].file + string(".pdiff");
-   DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI + ".diff/" + available_patches[0].file), Target);
+   DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(BaseFileURI), Target);
 
    if(Debug)
       std::clog << "pkgAcqIndexDiffs::QueueNextDiff(): " << Desc.URI << std::endl;
@@ -2923,7 +2924,7 @@ void pkgAcqIndexDiffs::Done(string const &Message, HashStringList const &Hashes,
 	    std::clog << "Sending to rred method: " << UnpatchedFile << std::endl;
 	 State = StateApplyDiff;
 	 Local = true;
-	 Desc.URI = "rred:" + UnpatchedFile;
+	 Desc.URI = "rred:" + pkgAcquire::URIEncode(UnpatchedFile);
 	 QueueURI(Desc);
 	 SetActiveSubprocess("rred");
 	 return;
@@ -2979,9 +2980,9 @@ pkgAcqIndexMergeDiffs::pkgAcqIndexMergeDiffs(pkgAcquire *const Owner,
 
    Desc.Owner = this;
    Desc.ShortDesc = Target.ShortDesc;
-   Desc.URI = Target.URI + ".diff/" + patch.file + ".gz";
+   Desc.URI = Target.URI + ".diff/" + pkgAcquire::URIEncode(patch.file) + ".gz";
    Desc.Description = Target.Description + " " + patch.file + ".pdiff";
-   DestFile = GetPartialFileNameFromURI(Target.URI + ".diff/" + patch.file + ".gz");
+   DestFile = GetPartialFileNameFromURI(Desc.URI);
 
    if(Debug)
       std::clog << "pkgAcqIndexMergeDiffs: " << Desc.URI << std::endl;
@@ -3068,7 +3069,7 @@ void pkgAcqIndexMergeDiffs::Done(string const &Message, HashStringList const &Ha
 	    std::clog << "Sending to rred method: " << UnpatchedFile << std::endl;
 	 State = StateApplyDiff;
 	 Local = true;
-	 Desc.URI = "rred:" + UnpatchedFile;
+	 Desc.URI = "rred:" + pkgAcquire::URIEncode(UnpatchedFile);
 	 QueueURI(Desc);
 	 SetActiveSubprocess("rred");
 	 return;
@@ -3270,7 +3271,7 @@ void pkgAcqIndex::StageDownloadDone(string const &Message)
       if (symlink(Filename.c_str(), DestFile.c_str()) != 0)
 	 _error->WarningE("pkgAcqIndex::StageDownloadDone", "Symlinking file %s to %s failed", Filename.c_str(), DestFile.c_str());
       Stage = STAGE_DECOMPRESS_AND_VERIFY;
-      Desc.URI = "store:" + DestFile;
+      Desc.URI = "store:" + pkgAcquire::URIEncode(DestFile);
       QueueURI(Desc);
       SetActiveSubprocess(::URI(Desc.URI).Access);
       return;
@@ -3299,9 +3300,9 @@ void pkgAcqIndex::StageDownloadDone(string const &Message)
    Stage = STAGE_DECOMPRESS_AND_VERIFY;
    DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
    if (Filename != DestFile && flExtension(Filename) == flExtension(DestFile))
-      Desc.URI = "copy:" + Filename;
+      Desc.URI = "copy:" + pkgAcquire::URIEncode(Filename);
    else
-      Desc.URI = "store:" + Filename;
+      Desc.URI = "store:" + pkgAcquire::URIEncode(Filename);
    if (DestFile == Filename)
    {
       if (CurrentCompressionExtension == "uncompressed")
@@ -3627,7 +3628,7 @@ void pkgAcqChangelog::Init(std::string const &DestDir, std::string const &DestFi
 	 DestFile = SrcName + ".changelog";
       else
 	 DestFile = DestFilename;
-      Desc.URI = "changelog:/" + DestFile;
+      Desc.URI = "changelog:/" + pkgAcquire::URIEncode(DestFile);
       return;
    }
 
@@ -3792,13 +3793,13 @@ std::string pkgAcqChangelog::URI(std::string const &Template,
       return "";
 
    // the path is: COMPONENT/SRC/SRCNAME/SRCNAME_SRCVER, e.g. main/a/apt/apt_1.1 or contrib/liba/libapt/libapt_2.0
-   std::string Src = SrcName;
-   std::string path = APT::String::Startswith(SrcName, "lib") ? Src.substr(0, 4) : Src.substr(0,1);
-   path.append("/").append(Src).append("/");
-   path.append(Src).append("_").append(StripEpoch(SrcVersion));
+   std::string const Src{SrcName};
+   std::string path = pkgAcquire::URIEncode(APT::String::Startswith(SrcName, "lib") ? Src.substr(0, 4) : Src.substr(0,1));
+   path.append("/").append(pkgAcquire::URIEncode(Src)).append("/");
+   path.append(pkgAcquire::URIEncode(Src)).append("_").append(pkgAcquire::URIEncode(StripEpoch(SrcVersion)));
    // we omit component for releases without one (= flat-style repositories)
    if (Component != NULL && strlen(Component) != 0)
-      path = std::string(Component) + "/" + path;
+      path = pkgAcquire::URIEncode(Component) + "/" + path;
 
    return SubstVar(Template, "@CHANGEPATH@", path);
 }
@@ -3858,8 +3859,11 @@ pkgAcqFile::pkgAcqFile(pkgAcquire *const Owner, string const &URI, HashStringLis
    else
       DestFile = flNotDir(URI);
 
+   ::URI url{URI};
+   url.Path = pkgAcquire::URIEncode(url.Path);
+
    // Create the item
-   Desc.URI = URI;
+   Desc.URI = std::string(url);
    Desc.Description = Dsc;
    Desc.Owner = this;
 
@@ -3901,7 +3905,7 @@ void pkgAcqFile::Done(string const &Message,HashStringList const &CalcHashes,
       if (_config->FindB("Acquire::Source-Symlinks",true) == false ||
 	  Cnf->Removable == true)
       {
-	 Desc.URI = "copy:" + FileName;
+	 Desc.URI = "copy:" + pkgAcquire::URIEncode(FileName);
 	 QueueURI(Desc);
 	 return;
       }
diff --git a/apt-pkg/acquire-method.cc b/apt-pkg/acquire-method.cc
index 486098c77..6e1674f7f 100644
--- a/apt-pkg/acquire-method.cc
+++ b/apt-pkg/acquire-method.cc
@@ -75,6 +75,9 @@ pkgAcqMethod::pkgAcqMethod(const char *Ver,unsigned long Flags)
    if ((Flags & AuxRequests) == AuxRequests)
       try_emplace(fields, "AuxRequests", "true");
 
+   if ((Flags & SendURIEncoded) == SendURIEncoded)
+      try_emplace(fields, "Send-URI-Encoded", "true");
+
    SendMessage("100 Capabilities", std::move(fields));
 
    SetNonBlock(STDIN_FILENO,true);
diff --git a/apt-pkg/acquire-method.h b/apt-pkg/acquire-method.h
index dde28a6ff..e854f5ff1 100644
--- a/apt-pkg/acquire-method.h
+++ b/apt-pkg/acquire-method.h
@@ -112,7 +112,8 @@ class APT_PUBLIC pkgAcqMethod
       LocalOnly = (1 << 3),
       NeedsCleanup = (1 << 4),
       Removable = (1 << 5),
-      AuxRequests = (1 << 6)
+      AuxRequests = (1 << 6),
+      SendURIEncoded = (1 << 7),
    };
 
    void Log(const char *Format,...);
diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc
index eff79215d..3fbc5c09f 100644
--- a/apt-pkg/acquire-worker.cc
+++ b/apt-pkg/acquire-worker.cc
@@ -307,8 +307,17 @@ bool pkgAcquire::Worker::RunMessages()
                break;
             }
 
-	    std::string const NewURI = LookupTag(Message,"New-URI",URI.c_str());
-            Itm->URI = NewURI;
+	    std::string const GotNewURI = LookupTag(Message,"New-URI",URI.c_str());
+	    if (Config->GetSendURIEncoded())
+	       Itm->URI = GotNewURI;
+	    else
+	    {
+	       ::URI tmpuri{GotNewURI};
+	       tmpuri.Path = pkgAcquire::URIEncode(tmpuri.Path);
+	       Itm->URI = tmpuri;
+	    }
+	    auto NewURI = Itm->URI;
+
 	    auto const AltUris = VectorizeString(LookupTag(Message, "Alternate-URIs"), '\n');
 
 	    ItemDone();
@@ -323,18 +332,37 @@ bool pkgAcquire::Worker::RunMessages()
 	    Itm = nullptr;
 	    for (auto const &Owner: ItmOwners)
 	    {
-	       for (auto alt = AltUris.crbegin(); alt != AltUris.crend(); ++alt)
-		  Owner->PushAlternativeURI(std::string(*alt), {}, false);
-
 	       pkgAcquire::ItemDesc &desc = Owner->GetItemDesc();
+
 	       // for a simplified retry a method might redirect without URI change
 	       // see also IsRedirectionLoop implementation
-	       if (desc.URI != NewURI)
+	       bool simpleRetry = false;
+	       if (Config->GetSendURIEncoded())
 	       {
-		  auto newuri = NewURI;
-		  if (Owner->IsGoodAlternativeURI(newuri) == false && Owner->PopAlternativeURI(newuri) == false)
-		     newuri.clear();
-		  if (newuri.empty() || Owner->IsRedirectionLoop(newuri))
+		  for (auto alt = AltUris.crbegin(); alt != AltUris.crend(); ++alt)
+		     Owner->PushAlternativeURI(std::string(*alt), {}, false);
+		  if (desc.URI == GotNewURI)
+		     simpleRetry = true;
+	       }
+	       else
+	       {
+		  for (auto alt = AltUris.crbegin(); alt != AltUris.crend(); ++alt)
+		  {
+		     ::URI tmpuri{*alt};
+		     tmpuri.Path = pkgAcquire::URIEncode(tmpuri.Path);
+		     Owner->PushAlternativeURI(std::string(tmpuri), {}, false);
+		  }
+		  ::URI tmpuri{desc.URI};
+		  tmpuri.Path = DeQuoteString(tmpuri.Path);
+		  if (GotNewURI == std::string(tmpuri))
+		     simpleRetry = true;
+	       }
+
+	       if (not simpleRetry)
+	       {
+		  if (Owner->IsGoodAlternativeURI(NewURI) == false && Owner->PopAlternativeURI(NewURI) == false)
+		     NewURI.clear();
+		  if (NewURI.empty() || Owner->IsRedirectionLoop(NewURI))
 		  {
 		     std::string msg = Message;
 		     msg.append("\nFailReason: RedirectionLoop");
@@ -665,12 +693,18 @@ bool pkgAcquire::Worker::Capabilities(string Message)
    Config->NeedsCleanup = StringToBool(LookupTag(Message,"Needs-Cleanup"),false);
    Config->Removable = StringToBool(LookupTag(Message,"Removable"),false);
    Config->SetAuxRequests(StringToBool(LookupTag(Message, "AuxRequests"), false));
+   if (_config->FindB("Acquire::Send-URI-Encoded", true))
+      Config->SetSendURIEncoded(StringToBool(LookupTag(Message, "Send-URI-Encoded"), false));
 
    // Some debug text
    if (Debug == true)
    {
       clog << "Configured access method " << Config->Access << endl;
-      clog << "Version:" << Config->Version << " SingleInstance:" << Config->SingleInstance << " Pipeline:" << Config->Pipeline << " SendConfig:" << Config->SendConfig << " LocalOnly: " << Config->LocalOnly << " NeedsCleanup: " << Config->NeedsCleanup << " Removable: " << Config->Removable << " AuxRequests: " << Config->GetAuxRequests() << endl;
+      clog << "Version:" << Config->Version << " SingleInstance:" << Config->SingleInstance
+	   << " Pipeline:" << Config->Pipeline << " SendConfig:" << Config->SendConfig
+	   << " LocalOnly: " << Config->LocalOnly << " NeedsCleanup: " << Config->NeedsCleanup
+	   << " Removable: " << Config->Removable << " AuxRequests: " << Config->GetAuxRequests()
+	   << " SendURIEncoded: " << Config->GetSendURIEncoded() << '\n';
    }
 
    return true;
@@ -737,6 +771,8 @@ bool pkgAcquire::Worker::SendConfiguration()
       configuration tree */
    std::ostringstream Message;
    Message << "601 Configuration\n";
+   if (not _config->Exists("Acquire::Send-URI-Encoded"))
+      Message << "Config-Item: Acquire::Send-URI-Encoded=1\n";
    _config->Dump(Message, NULL, "Config-Item: %F=%V\n", false);
    Message << '\n';
 
@@ -763,10 +799,16 @@ bool pkgAcquire::Worker::QueueItem(pkgAcquire::Queue::QItem *Item)
 
    string Message = "600 URI Acquire\n";
    Message.reserve(300);
-   Message += "URI: " + Item->URI;
+   URI URL(Item->URI);
+   if (Config->GetSendURIEncoded())
+      Message += "URI: " + Item->URI;
+   else
+   {
+      URL.Path = DeQuoteString(URL.Path);
+      Message += "URI: " + std::string(URL);
+   }
    Message += "\nFilename: " + Item->Owner->DestFile;
 
-   URI URL(Item->URI);
    // FIXME: We should not hard code proxy protocols here.
    if (URL.Access == "http" || URL.Access == "https")
    {
diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc
index 1cffc0463..8693de930 100644
--- a/apt-pkg/acquire.cc
+++ b/apt-pkg/acquire.cc
@@ -51,6 +51,12 @@
 
 using namespace std;
 
+std::string pkgAcquire::URIEncode(std::string const &part)		/*{{{*/
+{
+   // The "+" is encoded as a workaround for an S3 bug (LP#1003633 and LP#1086997)
+   return QuoteString(part, _config->Find("Acquire::URIEncode", "+~ ").c_str());
+}
+									/*}}}*/
 // Acquire::pkgAcquire - Constructor					/*{{{*/
 // ---------------------------------------------------------------------
 /* We grab some runtime state from the configuration space */
@@ -662,10 +668,11 @@ static void CheckDropPrivsMustBeDisabled(pkgAcquire const &Fetcher)
 	 if (_config->Exists(conf) == true)
 	    continue;
 
-	 if (IsAccessibleBySandboxUser(source.Path, false) == false)
+	 auto const filepath = DeQuoteString(source.Path);
+	 if (not IsAccessibleBySandboxUser(filepath, false))
 	 {
 	    _error->NoticeE("pkgAcquire::Run", _("Download is performed unsandboxed as root as file '%s' couldn't be accessed by user '%s'."),
-		  source.Path.c_str(), SandboxUser.c_str());
+			    filepath.c_str(), SandboxUser.c_str());
 	    _config->CndSet("Binary::file::APT::Sandbox::User", "root");
 	    _config->CndSet("Binary::copy::APT::Sandbox::User", "root");
 	 }
@@ -880,6 +887,7 @@ class pkgAcquire::MethodConfig::Private
 {
    public:
    bool AuxRequests = false;
+   bool SendURIEncoded = false;
 };
 pkgAcquire::MethodConfig::MethodConfig() : d(new Private()), Next(0), SingleInstance(false),
 					   Pipeline(false), SendConfig(false), LocalOnly(false), NeedsCleanup(false),
@@ -897,6 +905,17 @@ void pkgAcquire::MethodConfig::SetAuxRequests(bool const value) /*{{{*/
    d->AuxRequests = value;
 }
 									/*}}}*/
+bool pkgAcquire::MethodConfig::GetSendURIEncoded() const		/*{{{*/
+{
+   return d->SendURIEncoded;
+}
+									/*}}}*/
+void pkgAcquire::MethodConfig::SetSendURIEncoded(bool const value)	/*{{{*/
+{
+   d->SendURIEncoded = value;
+}
+									/*}}}*/
+
 // Queue::Queue - Constructor						/*{{{*/
 // ---------------------------------------------------------------------
 /* */
@@ -1073,10 +1092,25 @@ bool pkgAcquire::Queue::Shutdown(bool Final)
 /* */
 pkgAcquire::Queue::QItem *pkgAcquire::Queue::FindItem(string URI,pkgAcquire::Worker *Owner)
 {
-   for (QItem *I = Items; I != 0; I = I->Next)
-      if (I->URI == URI && I->Worker == Owner)
-	 return I;
-   return 0;
+   if (Owner->Config->GetSendURIEncoded())
+   {
+      for (QItem *I = Items; I != nullptr; I = I->Next)
+	 if (I->URI == URI && I->Worker == Owner)
+	    return I;
+   }
+   else
+   {
+      for (QItem *I = Items; I != nullptr; I = I->Next)
+      {
+	 if (I->Worker != Owner)
+	    continue;
+	 ::URI tmpuri{I->URI};
+	 tmpuri.Path = DeQuoteString(tmpuri.Path);
+	 if (URI == std::string(tmpuri))
+	    return I;
+      }
+   }
+   return nullptr;
 }
 									/*}}}*/
 // Queue::ItemDone - Item has been completed				/*{{{*/
diff --git a/apt-pkg/acquire.h b/apt-pkg/acquire.h
index 8cb4d2532..a2c4fbc67 100644
--- a/apt-pkg/acquire.h
+++ b/apt-pkg/acquire.h
@@ -363,6 +363,8 @@ class APT_PUBLIC pkgAcquire
     */
    virtual ~pkgAcquire();
 
+   APT_HIDDEN static std::string URIEncode(std::string const &part);
+
    private:
    APT_HIDDEN void Initialize();
 };
@@ -680,6 +682,8 @@ struct APT_PUBLIC pkgAcquire::MethodConfig
 
    APT_HIDDEN bool GetAuxRequests() const;
    APT_HIDDEN void SetAuxRequests(bool const value);
+   APT_HIDDEN bool GetSendURIEncoded() const;
+   APT_HIDDEN void SetSendURIEncoded(bool const value);
 
    virtual ~MethodConfig();
 };
diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc
index a72f6d055..f24a5e79e 100644
--- a/apt-pkg/deb/debmetaindex.cc
+++ b/apt-pkg/deb/debmetaindex.cc
@@ -142,10 +142,10 @@ static std::string constructMetaIndexURI(std::string URI, std::string const &Dis
    if (Dist == "/")
       ;
    else if (Dist[Dist.size()-1] == '/')
-      URI += Dist;
+      URI += pkgAcquire::URIEncode(Dist);
    else
-      URI += "dists/" + Dist + "/";
-   return URI + Type;
+      URI += "dists/" + pkgAcquire::URIEncode(Dist) + "/";
+   return URI + pkgAcquire::URIEncode(Type);
 }
 std::string debReleaseIndex::MetaIndexURI(const char *Type) const
 {
@@ -420,6 +420,13 @@ void debReleaseIndex::AddComponent(std::string const &sourcesEntry,	/*{{{*/
       d->DebEntries.push_back(entry);
 }
 									/*}}}*/
+std::string debReleaseIndex::ArchiveURI(std::string const &File) const	/*{{{*/
+{
+   if (File.empty())
+      return URI;
+   return URI + pkgAcquire::URIEncode(File);
+}
+									/*}}}*/
 
 bool debReleaseIndex::Load(std::string const &Filename, std::string * const ErrorText)/*{{{*/
 {
diff --git a/apt-pkg/deb/debmetaindex.h b/apt-pkg/deb/debmetaindex.h
index 5576ff809..717f08e2b 100644
--- a/apt-pkg/deb/debmetaindex.h
+++ b/apt-pkg/deb/debmetaindex.h
@@ -32,7 +32,7 @@ class APT_HIDDEN debReleaseIndex : public metaIndex
    debReleaseIndex(std::string const &URI, std::string const &Dist, bool const Trusted, std::map<std::string,std::string> const &Options);
    virtual ~debReleaseIndex();
 
-   virtual std::string ArchiveURI(std::string const &File) const APT_OVERRIDE {return URI + File;};
+   virtual std::string ArchiveURI(std::string const &File) const APT_OVERRIDE;
    virtual bool GetIndexes(pkgAcquire *Owner, bool const &GetAll=false) APT_OVERRIDE;
    virtual std::vector<IndexTarget> GetIndexTargets() const APT_OVERRIDE;
 
diff --git a/apt-pkg/update.cc b/apt-pkg/update.cc
index 4c64eeb5d..1b25bafd6 100644
--- a/apt-pkg/update.cc
+++ b/apt-pkg/update.cc
@@ -87,6 +87,8 @@ bool AcquireUpdate(pkgAcquire &Fetcher, int const PulseInterval,
       ::URI uri((*I)->DescURI());
       uri.User.clear();
       uri.Password.clear();
+      if ((*I)->Local)
+	 uri.Path = DeQuoteString(uri.Path);
       std::string const descUri = std::string(uri);
       // Show an error for non-transient failures, otherwise only warn
       if ((*I)->Status == pkgAcquire::Item::StatTransientNetworkError)
diff --git a/doc/examples/configure-index b/doc/examples/configure-index
index 0af923811..b73166082 100644
--- a/doc/examples/configure-index
+++ b/doc/examples/configure-index
@@ -223,6 +223,8 @@ Acquire
   Retries "<INT>";
   Source-Symlinks "<BOOL>";
   ForceHash "<STRING>"; // hashmethod used for expected hash: sha256, sha1 or md5sum
+  Send-URI-Encoded "<BOOL>"; // false does the old encode/decode dance even if we could avoid it
+  URIEncode "<STRING>"; // characters to encode with percent encoding
 
   AllowTLS "<BOOL>";    // whether support for tls is enabled
 
diff --git a/doc/method.dbk b/doc/method.dbk
index 410d6898c..ea49c5b54 100644
--- a/doc/method.dbk
+++ b/doc/method.dbk
@@ -522,8 +522,8 @@ This is a list of which headers each status code can use
 <para>
 Displays the capabilities of the method. Methods should set the pipeline bit
 if their underlying protocol supports pipelining. The only known method that
-does support pipelining is http. Fields: Version, Single-Instance, Pre-Scan,
-Pipeline, Send-Config, Needs-Cleanup
+does support pipelining is http. Fields: Version, Single-Instance, Local-Only,
+Pipeline, Send-Config, Needs-Cleanup, Removable, AuxRequests, Send-URI-Encoded
 </para>
 </listitem>
 </varlistentry>
diff --git a/test/integration/test-acquire-same-file-multiple-times b/test/integration/test-acquire-same-file-multiple-times
index a6825b2c2..fc82412db 100755
--- a/test/integration/test-acquire-same-file-multiple-times
+++ b/test/integration/test-acquire-same-file-multiple-times
@@ -12,8 +12,8 @@ APTARCHIVE="$(readlink -f ./aptarchive)"
 
 filedown() {
 	msgtest 'Downloading the same URI twice over file' "$1"
-	testsuccess --nomsg apthelper download-file "file:///$APTARCHIVE/foo" './downloaded/foo1' '' \
-		"file:///$APTARCHIVE/foo" './downloaded/foo2' '' -o Debug::pkgAcquire::Worker=1
+	testsuccess --nomsg apthelper download-file "file://$APTARCHIVE/foo" './downloaded/foo1' '' \
+		"file://$APTARCHIVE/foo" './downloaded/foo2' '' -o Debug::pkgAcquire::Worker=1
 	cp rootdir/tmp/testsuccess.output download.log
 	testsuccess cmp "$TESTFILE" ./downloaded/foo1
 	testsuccess cmp ./downloaded/foo1 ./downloaded/foo2
diff --git a/test/integration/test-apt-get-source-arch b/test/integration/test-apt-get-source-arch
index ca586f46a..573ae1499 100755
--- a/test/integration/test-apt-get-source-arch
+++ b/test/integration/test-apt-get-source-arch
@@ -28,8 +28,8 @@ APTARCHIVE=$(readlink -f ./aptarchive)
 
 HEADER='Reading package lists...'
 DOWNLOAD10="Need to get 0 B/25 B of source archives.
-'file://${APTARCHIVE}/foo_1.0.dsc' foo_1.0.dsc 11 SHA256:ed7c25c832596339bee13e4e7c45cf49f869b60d2bf57252f18191d75866c2a7
-'file://${APTARCHIVE}/foo_1.0.tar.gz' foo_1.0.tar.gz 14 SHA256:f3da8c6ebc62c8ef2dae439a498dddcdacc1a07f45ff67ad12f44b6e2353c239"
+'file:${APTARCHIVE}/foo_1.0.dsc' foo_1.0.dsc 11 SHA256:ed7c25c832596339bee13e4e7c45cf49f869b60d2bf57252f18191d75866c2a7
+'file:${APTARCHIVE}/foo_1.0.tar.gz' foo_1.0.tar.gz 14 SHA256:f3da8c6ebc62c8ef2dae439a498dddcdacc1a07f45ff67ad12f44b6e2353c239"
 
 # pick :amd64
 testsuccessequal "$HEADER
@@ -38,15 +38,15 @@ $DOWNLOAD10" aptget source -q --print-uris foo:amd64
 # pick :i386
 testsuccessequal "$HEADER
 Need to get 0 B/25 B of source archives.
-'file://${APTARCHIVE}/foo_2.0.dsc' foo_2.0.dsc 11 SHA256:0fcb803ffbeef26db884625aaf06e75f3eda5c994634980e7c20fd37ed1fc104
-'file://${APTARCHIVE}/foo_2.0.tar.gz' foo_2.0.tar.gz 14 SHA256:ca9b0b828ca22372502af2b80f61f0bd9063910ece9fc34eeaf9d9e31aa8195a" aptget source -q --print-uris foo:i386
+'file:${APTARCHIVE}/foo_2.0.dsc' foo_2.0.dsc 11 SHA256:0fcb803ffbeef26db884625aaf06e75f3eda5c994634980e7c20fd37ed1fc104
+'file:${APTARCHIVE}/foo_2.0.tar.gz' foo_2.0.tar.gz 14 SHA256:ca9b0b828ca22372502af2b80f61f0bd9063910ece9fc34eeaf9d9e31aa8195a" aptget source -q --print-uris foo:i386
 
 # pick :i386 by release
 testsuccessequal "$HEADER
 Selected version '0.1' (oldstable) for foo
 Need to get 0 B/25 B of source archives.
-'file://${APTARCHIVE}/foo_0.1.dsc' foo_0.1.dsc 11 SHA256:72af24b0290fe1d13a3e25fddd2633e43c87ff79d249bc850009e47bcce73565
-'file://${APTARCHIVE}/foo_0.1.tar.gz' foo_0.1.tar.gz 14 SHA256:ec748ad88a71f98bfdc012e1a7632377d05fe3ebbf9c0922e0691fe4d79c0585" aptget source -q --print-uris foo:i386/oldstable
+'file:${APTARCHIVE}/foo_0.1.dsc' foo_0.1.dsc 11 SHA256:72af24b0290fe1d13a3e25fddd2633e43c87ff79d249bc850009e47bcce73565
+'file:${APTARCHIVE}/foo_0.1.tar.gz' foo_0.1.tar.gz 14 SHA256:ec748ad88a71f98bfdc012e1a7632377d05fe3ebbf9c0922e0691fe4d79c0585" aptget source -q --print-uris foo:i386/oldstable
 
 # pick :i386 by version
 testsuccessequal "$HEADER
diff --git a/test/integration/test-apt-get-source-multisources b/test/integration/test-apt-get-source-multisources
index 06fe19641..fbc52ccbd 100755
--- a/test/integration/test-apt-get-source-multisources
+++ b/test/integration/test-apt-get-source-multisources
@@ -20,10 +20,10 @@ APTARCHIVE=$(readlink -f ./aptarchive)
 HEADER='Reading package lists...'
 testsuccessequal "$HEADER
 Need to get 0 B/43 B of source archives.
-'file://${APTARCHIVE}/adduser_3.113+nmu3.dsc' adduser_3.113+nmu3.dsc 22 SHA256:19cc1abe85063976bf71c033f62f3e6bf6621647fe44a6ee31ed687e3fa5cbb7
-'file://${APTARCHIVE}/python-fll_0.9.11.dsc' python-fll_0.9.11.dsc 21 SHA256:51429e835ded66abf6bbc157865af29920435e74aea2836ba1f46443feae9285" aptget source -qdy --print-uris --dsc-only adduser=3.113 python-fll=0.9.11
+'file:${APTARCHIVE}/adduser_3.113%2bnmu3.dsc' adduser_3.113+nmu3.dsc 22 SHA256:19cc1abe85063976bf71c033f62f3e6bf6621647fe44a6ee31ed687e3fa5cbb7
+'file:${APTARCHIVE}/python-fll_0.9.11.dsc' python-fll_0.9.11.dsc 21 SHA256:51429e835ded66abf6bbc157865af29920435e74aea2836ba1f46443feae9285" aptget source -qdy --print-uris --dsc-only adduser=3.113 python-fll=0.9.11
 
 testsuccessequal "$HEADER
 Need to get 0 B/43 B of source archives.
-'file://${APTARCHIVE}/python-fll_0.9.11.dsc' python-fll_0.9.11.dsc 21 SHA256:51429e835ded66abf6bbc157865af29920435e74aea2836ba1f46443feae9285
-'file://${APTARCHIVE}/adduser_3.113+nmu3.dsc' adduser_3.113+nmu3.dsc 22 SHA256:19cc1abe85063976bf71c033f62f3e6bf6621647fe44a6ee31ed687e3fa5cbb7" aptget source -qdy --print-uris --dsc-only python-fll=0.9.11 adduser=3.113
+'file:${APTARCHIVE}/python-fll_0.9.11.dsc' python-fll_0.9.11.dsc 21 SHA256:51429e835ded66abf6bbc157865af29920435e74aea2836ba1f46443feae9285
+'file:${APTARCHIVE}/adduser_3.113%2bnmu3.dsc' adduser_3.113+nmu3.dsc 22 SHA256:19cc1abe85063976bf71c033f62f3e6bf6621647fe44a6ee31ed687e3fa5cbb7" aptget source -qdy --print-uris --dsc-only python-fll=0.9.11 adduser=3.113
diff --git a/test/integration/test-apt-source-and-build-dep b/test/integration/test-apt-source-and-build-dep
index 24790a578..a089d62fd 100755
--- a/test/integration/test-apt-source-and-build-dep
+++ b/test/integration/test-apt-source-and-build-dep
@@ -63,17 +63,17 @@ APTARCHIVE=$(readlink -f ./aptarchive)
 # normal operation gets highest version number
 HEADER='Reading package lists...'
 DOWNLOAD1="Need to get 0 B/25 B of source archives.
-'file://${APTARCHIVE}/foo_1.0.dsc' foo_1.0.dsc 11 SHA256:ed7c25c832596339bee13e4e7c45cf49f869b60d2bf57252f18191d75866c2a7
-'file://${APTARCHIVE}/foo_1.0.tar.gz' foo_1.0.tar.gz 14 SHA256:f3da8c6ebc62c8ef2dae439a498dddcdacc1a07f45ff67ad12f44b6e2353c239"
+'file:${APTARCHIVE}/foo_1.0.dsc' foo_1.0.dsc 11 SHA256:ed7c25c832596339bee13e4e7c45cf49f869b60d2bf57252f18191d75866c2a7
+'file:${APTARCHIVE}/foo_1.0.tar.gz' foo_1.0.tar.gz 14 SHA256:f3da8c6ebc62c8ef2dae439a498dddcdacc1a07f45ff67ad12f44b6e2353c239"
 DOWNLOAD2="Need to get 0 B/25 B of source archives.
-'file://${APTARCHIVE}/foo_2.0.dsc' foo_2.0.dsc 11 SHA256:0fcb803ffbeef26db884625aaf06e75f3eda5c994634980e7c20fd37ed1fc104
-'file://${APTARCHIVE}/foo_2.0.tar.gz' foo_2.0.tar.gz 14 SHA256:ca9b0b828ca22372502af2b80f61f0bd9063910ece9fc34eeaf9d9e31aa8195a"
+'file:${APTARCHIVE}/foo_2.0.dsc' foo_2.0.dsc 11 SHA256:0fcb803ffbeef26db884625aaf06e75f3eda5c994634980e7c20fd37ed1fc104
+'file:${APTARCHIVE}/foo_2.0.tar.gz' foo_2.0.tar.gz 14 SHA256:ca9b0b828ca22372502af2b80f61f0bd9063910ece9fc34eeaf9d9e31aa8195a"
 DOWNLOAD3="Need to get 0 B/25 B of source archives.
-'file://${APTARCHIVE}/baz_1.0.dsc' baz_1.0.dsc 11 SHA256:322245f56092b466801dda62d79c8687bba9724af6d16d450d655d29e41d3d7b
-'file://${APTARCHIVE}/baz_1.0.tar.gz' baz_1.0.tar.gz 14 SHA256:0870bc73164ff5ba1f52153fdcb48e140137f9c7c122d57592cea136a57f73c0"
+'file:${APTARCHIVE}/baz_1.0.dsc' baz_1.0.dsc 11 SHA256:322245f56092b466801dda62d79c8687bba9724af6d16d450d655d29e41d3d7b
+'file:${APTARCHIVE}/baz_1.0.tar.gz' baz_1.0.tar.gz 14 SHA256:0870bc73164ff5ba1f52153fdcb48e140137f9c7c122d57592cea136a57f73c0"
 DOWNLOAD4="Need to get 0 B/25 B of source archives.
-'file://${APTARCHIVE}/baz_2.0.dsc' baz_2.0.dsc 11 SHA256:47d062d29070b3f592d1c8aed8c1e7913804bbb67ca1d64877c8219dac5e0420
-'file://${APTARCHIVE}/baz_2.0.tar.gz' baz_2.0.tar.gz 14 SHA256:11c1b202c94a64ab6433d9f0ed5515fce1dc7b20e6bcf51cec9ef8b9455f5a41"
+'file:${APTARCHIVE}/baz_2.0.dsc' baz_2.0.dsc 11 SHA256:47d062d29070b3f592d1c8aed8c1e7913804bbb67ca1d64877c8219dac5e0420
+'file:${APTARCHIVE}/baz_2.0.tar.gz' baz_2.0.tar.gz 14 SHA256:11c1b202c94a64ab6433d9f0ed5515fce1dc7b20e6bcf51cec9ef8b9455f5a41"
 testsuccessequal "$HEADER
 $DOWNLOAD2" apt source -q --print-uris foo
 testsuccessequal "$HEADER
@@ -138,8 +138,8 @@ testsuccessequal "$(getbuilddep 'foo')" apt build-dep foo=1.0 -s
 # select by release with no binary package (Bug#731102) but ensure to get
 # highest version
 DOWNLOAD01="Need to get 0 B/25 B of source archives.
-'file://${APTARCHIVE}/foo_0.1.dsc' foo_0.1.dsc 11 SHA256:72af24b0290fe1d13a3e25fddd2633e43c87ff79d249bc850009e47bcce73565
-'file://${APTARCHIVE}/foo_0.1.tar.gz' foo_0.1.tar.gz 14 SHA256:ec748ad88a71f98bfdc012e1a7632377d05fe3ebbf9c0922e0691fe4d79c0585"
+'file:${APTARCHIVE}/foo_0.1.dsc' foo_0.1.dsc 11 SHA256:72af24b0290fe1d13a3e25fddd2633e43c87ff79d249bc850009e47bcce73565
+'file:${APTARCHIVE}/foo_0.1.tar.gz' foo_0.1.tar.gz 14 SHA256:ec748ad88a71f98bfdc012e1a7632377d05fe3ebbf9c0922e0691fe4d79c0585"
 testsuccessequal "$HEADER
 Selected version '0.1' (wheezy) for foo
 $DOWNLOAD01" apt source -q --print-uris foo/wheezy
@@ -155,8 +155,8 @@ E: Unable to find a source package for foo=9.9-not-there" apt build-dep -s foo=9
 
 # version and release
 DOWNLOAD001="Need to get 0 B/29 B of source archives.
-'file://${APTARCHIVE}/foo_0.0.1.dsc' foo_0.0.1.dsc 13 SHA256:649dfe03bbb70cebdfe7c6bf9036f9f2472510b8f52e823bdf5ade362ebaa76f
-'file://${APTARCHIVE}/foo_0.0.1.tar.gz' foo_0.0.1.tar.gz 16 SHA256:ab7ba789d178362ecc808e49705e2338988a7f5b9410ec11a6c9555c017de907"
+'file:${APTARCHIVE}/foo_0.0.1.dsc' foo_0.0.1.dsc 13 SHA256:649dfe03bbb70cebdfe7c6bf9036f9f2472510b8f52e823bdf5ade362ebaa76f
+'file:${APTARCHIVE}/foo_0.0.1.tar.gz' foo_0.0.1.tar.gz 16 SHA256:ab7ba789d178362ecc808e49705e2338988a7f5b9410ec11a6c9555c017de907"
 testsuccessequal "$HEADER
 $DOWNLOAD001" apt source -q --print-uris -t unstable foo=0.0.1
 testsuccessequal "$(getbuilddep 'foo')" apt build-dep foo=0.0.1 -s
diff --git a/test/integration/test-bug-722207-print-uris-even-if-very-quiet b/test/integration/test-bug-722207-print-uris-even-if-very-quiet
index 8d17507cb..d39df4dff 100755
--- a/test/integration/test-bug-722207-print-uris-even-if-very-quiet
+++ b/test/integration/test-bug-722207-print-uris-even-if-very-quiet
@@ -21,11 +21,11 @@ testsuccessequal "'file://${APTARCHIVE}/pool/main/apt/apt_2_all.deb' apt_2_all.d
 testsuccessequal "'file://${APTARCHIVE}/pool/main/apt/apt_2_all.deb' apt_2_all.deb 0 " aptget dist-upgrade -qq --print-uris
 testsuccessequal "'file://${APTARCHIVE}/pool/main/apt/apt_2_all.deb' apt_2_all.deb 0 " aptget install apt -qq --print-uris
 testsuccessequal "'file://${APTARCHIVE}/pool/main/apt/apt_2_all.deb' apt_2_all.deb 0 SHA256:0000000000000000000000000000000000000000000000000000000000000000" aptget download apt -qq --print-uris
-testsuccessequal "'file://${APTARCHIVE}/apt_2.dsc' apt_2.dsc 9 SHA256:7776436a6d741497f1cd958014e1a05b352224231428152aae39da3c17fd2fd4
-'file://${APTARCHIVE}/apt_2.tar.gz' apt_2.tar.gz 12 SHA256:f57f565eabe3fde0ec6e6e0bcc8db1d86fe2b4d6344a380a23520ddbb7728e99" aptget source apt -qq --print-uris
+testsuccessequal "'file:${APTARCHIVE}/apt_2.dsc' apt_2.dsc 9 SHA256:7776436a6d741497f1cd958014e1a05b352224231428152aae39da3c17fd2fd4
+'file:${APTARCHIVE}/apt_2.tar.gz' apt_2.tar.gz 12 SHA256:f57f565eabe3fde0ec6e6e0bcc8db1d86fe2b4d6344a380a23520ddbb7728e99" aptget source apt -qq --print-uris
 testsuccessequal "'https://metadata.ftp-master.debian.org/changelogs/main/a/apt/apt_2_changelog' apt.changelog" aptget changelog apt -qq --print-uris
 
-testsuccessequal "'file://${APTARCHIVE}/apt_2.dsc' apt_2.dsc 9 SHA256:7776436a6d741497f1cd958014e1a05b352224231428152aae39da3c17fd2fd4
-'file://${APTARCHIVE}/apt_2.tar.gz' apt_2.tar.gz 12 SHA256:f57f565eabe3fde0ec6e6e0bcc8db1d86fe2b4d6344a380a23520ddbb7728e99
-'file://${APTARCHIVE}/apt2_1.dsc' apt2_1.dsc 10 SHA256:5693ba5efbfa21216f13661d344611aabe70ce3c343554ab46d4d9c24fdfd13a
-'file://${APTARCHIVE}/apt2_1.tar.gz' apt2_1.tar.gz 13 SHA256:1464c609fd09934c270ec629020d5e248b080607f715e47ef088cc8ab8480541" aptget source apt apt2 -qq --print-uris
+testsuccessequal "'file:${APTARCHIVE}/apt_2.dsc' apt_2.dsc 9 SHA256:7776436a6d741497f1cd958014e1a05b352224231428152aae39da3c17fd2fd4
+'file:${APTARCHIVE}/apt_2.tar.gz' apt_2.tar.gz 12 SHA256:f57f565eabe3fde0ec6e6e0bcc8db1d86fe2b4d6344a380a23520ddbb7728e99
+'file:${APTARCHIVE}/apt2_1.dsc' apt2_1.dsc 10 SHA256:5693ba5efbfa21216f13661d344611aabe70ce3c343554ab46d4d9c24fdfd13a
+'file:${APTARCHIVE}/apt2_1.tar.gz' apt2_1.tar.gz 13 SHA256:1464c609fd09934c270ec629020d5e248b080607f715e47ef088cc8ab8480541" aptget source apt apt2 -qq --print-uris
-- 
cgit v1.2.3


From 97d6c3b2d05fe0d965657197adf56cc78f9edf81 Mon Sep 17 00:00:00 2001
From: David Kalnischkies <david@kalnischkies.de>
Date: Fri, 10 Jul 2020 00:02:25 +0200
Subject: Implement encoded URI handling in all methods

Every method opts in to getting the encoded URI passed along while
keeping compat in case we are operated by an older acquire system.

Effectively this is just a change for the http-based methods as the
others just decode the URI as they work with files directly.
---
 methods/aptmethod.h                                | 12 ++++++++++
 methods/basehttp.cc                                | 22 +++++++++++++++---
 methods/cdrom.cc                                   |  4 ++--
 methods/copy.cc                                    |  4 ++--
 methods/file.cc                                    |  4 ++--
 methods/ftp.cc                                     | 10 ++++----
 methods/gpgv.cc                                    |  4 ++--
 methods/http.cc                                    |  7 +++---
 methods/mirror.cc                                  |  2 +-
 methods/rred.cc                                    |  4 ++--
 methods/rsh.cc                                     | 10 ++++----
 methods/store.cc                                   |  4 ++--
 .../test-cve-2019-3462-dequote-injection           | 27 ++++++++++++++++------
 13 files changed, 77 insertions(+), 37 deletions(-)

diff --git a/methods/aptmethod.h b/methods/aptmethod.h
index 67d5a3a0b..7038131cf 100644
--- a/methods/aptmethod.h
+++ b/methods/aptmethod.h
@@ -475,6 +475,18 @@ protected:
 	 QueueBack = Queue;
       delete Tmp;
    }
+   static std::string URIEncode(std::string const &part)
+   {
+      // The "+" is encoded as a workaround for an S3 bug (LP#1003633 and LP#1086997)
+      return QuoteString(part, _config->Find("Acquire::URIEncode", "+~ ").c_str());
+   }
+
+   static std::string DecodeSendURI(std::string const &part)
+   {
+      if (_config->FindB("Acquire::Send-URI-Encoded", false))
+	 return DeQuoteString(part);
+      return part;
+   }
 
    aptMethod(std::string &&Binary, char const *const Ver, unsigned long const Flags) APT_NONNULL(3)
        : pkgAcqMethod(Ver, Flags), Binary(Binary), SeccompFlags(0), methodNames({Binary})
diff --git a/methods/basehttp.cc b/methods/basehttp.cc
index b8ab73155..8aac1090c 100644
--- a/methods/basehttp.cc
+++ b/methods/basehttp.cc
@@ -283,6 +283,14 @@ void ServerState::Reset()						/*{{{*/
 /* We look at the header data we got back from the server and decide what
    to do. Returns DealWithHeadersResult (see http.h for details).
  */
+static std::string fixURIEncoding(std::string const &part)
+{
+   // if the server sends a space this is not an encoded URI
+   // so other clients seem to encode it and we do it as well
+   if (part.find_first_of(" ") != std::string::npos)
+      return aptMethod::URIEncode(part);
+   return part;
+}
 BaseHttpMethod::DealWithHeadersResult
 BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req)
 {
@@ -318,7 +326,10 @@ BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req)
             NextURI = URI::SiteOnly(Uri);
 	 else
 	    NextURI.clear();
-	 NextURI.append(DeQuoteString(Req.Location));
+	 if (_config->FindB("Acquire::Send-URI-Encoded", false))
+	    NextURI.append(fixURIEncoding(Req.Location));
+	 else
+	    NextURI.append(DeQuoteString(Req.Location));
 	 if (Queue->Uri == NextURI)
 	 {
 	    SetFailReason("RedirectionLoop");
@@ -331,8 +342,12 @@ BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req)
       }
       else
       {
-	 NextURI = DeQuoteString(Req.Location);
-	 URI tmpURI(NextURI);
+	 bool const SendURIEncoded = _config->FindB("Acquire::Send-URI-Encoded", false);
+	 if (not SendURIEncoded)
+	    Req.Location = DeQuoteString(Req.Location);
+	 URI tmpURI(Req.Location);
+	 if (SendURIEncoded)
+	    tmpURI.Path = fixURIEncoding(tmpURI.Path);
 	 if (tmpURI.Access.find('+') != std::string::npos)
 	 {
 	    _error->Error("Server tried to trick us into using a specific implementation: %s", tmpURI.Access.c_str());
@@ -340,6 +355,7 @@ BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req)
 	       return ERROR_WITH_CONTENT_PAGE;
 	    return ERROR_UNRECOVERABLE;
 	 }
+	 NextURI = tmpURI;
 	 URI Uri(Queue->Uri);
 	 if (Binary.find('+') != std::string::npos)
 	 {
diff --git a/methods/cdrom.cc b/methods/cdrom.cc
index d836e1315..f534a06f5 100644
--- a/methods/cdrom.cc
+++ b/methods/cdrom.cc
@@ -57,7 +57,7 @@ class CDROMMethod : public aptMethod
 /* */
 CDROMMethod::CDROMMethod() : aptMethod("cdrom", "1.0",SingleInstance | LocalOnly |
 					  SendConfig | NeedsCleanup |
-					  Removable),
+					  Removable | SendURIEncoded),
                                           DatabaseLoaded(false),
 					  Debug(false),
                                           MountedByApt(false)
@@ -175,7 +175,7 @@ bool CDROMMethod::Fetch(FetchItem *Itm)
    FetchResult Res;
 
    URI Get(Itm->Uri);
-   string File = Get.Path;
+   std::string const File = DecodeSendURI(Get.Path);
    Debug = DebugEnabled();
 
    if (Debug)
diff --git a/methods/copy.cc b/methods/copy.cc
index 9a8665446..82eed150c 100644
--- a/methods/copy.cc
+++ b/methods/copy.cc
@@ -29,7 +29,7 @@ class CopyMethod : public aptMethod
    virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE;
 
    public:
-   CopyMethod() : aptMethod("copy", "1.0", SingleInstance | SendConfig)
+   CopyMethod() : aptMethod("copy", "1.0", SingleInstance | SendConfig | SendURIEncoded)
    {
       SeccompFlags = aptMethod::BASE;
    }
@@ -41,7 +41,7 @@ class CopyMethod : public aptMethod
 bool CopyMethod::Fetch(FetchItem *Itm)
 {
    // this ensures that relative paths work in copy
-   std::string const File = Itm->Uri.substr(Itm->Uri.find(':')+1);
+   std::string const File = DecodeSendURI(Itm->Uri.substr(Itm->Uri.find(':')+1));
 
    // Stat the file and send a start message
    struct stat Buf;
diff --git a/methods/file.cc b/methods/file.cc
index 80e47f1ad..b2fe133f2 100644
--- a/methods/file.cc
+++ b/methods/file.cc
@@ -32,7 +32,7 @@ class FileMethod : public aptMethod
    virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE;
 
    public:
-   FileMethod() : aptMethod("file", "1.0", SingleInstance | SendConfig | LocalOnly)
+   FileMethod() : aptMethod("file", "1.0", SingleInstance | SendConfig | LocalOnly | SendURIEncoded)
    {
       SeccompFlags = aptMethod::BASE;
    }
@@ -44,7 +44,7 @@ class FileMethod : public aptMethod
 bool FileMethod::Fetch(FetchItem *Itm)
 {
    URI Get(Itm->Uri);
-   std::string File = Get.Path;
+   std::string const File = DecodeSendURI(Get.Path);
    FetchResult Res;
    if (Get.Host.empty() == false)
       return _error->Error(_("Invalid URI, local URIS must not start with //"));
diff --git a/methods/ftp.cc b/methods/ftp.cc
index 98398341e..16236232a 100644
--- a/methods/ftp.cc
+++ b/methods/ftp.cc
@@ -988,7 +988,7 @@ bool FTPConn::Get(const char *Path,FileFd &To,unsigned long long Resume,
 // FtpMethod::FtpMethod - Constructor					/*{{{*/
 // ---------------------------------------------------------------------
 /* */
-FtpMethod::FtpMethod() : aptAuthConfMethod("ftp", "1.0", SendConfig)
+FtpMethod::FtpMethod() : aptAuthConfMethod("ftp", "1.0", SendConfig | SendURIEncoded)
 {
    SeccompFlags = aptMethod::BASE | aptMethod::NETWORK;
    signal(SIGTERM,SigTerm);
@@ -1038,7 +1038,7 @@ bool FtpMethod::Configuration(string Message)
 bool FtpMethod::Fetch(FetchItem *Itm)
 {
    URI Get(Itm->Uri);
-   const char *File = Get.Path.c_str();
+   auto const File = DecodeSendURI(Get.Path);
    FetchResult Res;
    Res.Filename = Itm->DestFile;
    Res.IMSHit = false;
@@ -1070,8 +1070,8 @@ bool FtpMethod::Fetch(FetchItem *Itm)
    // Get the files information
    Status(_("Query"));
    unsigned long long Size;
-   if (Server->Size(File,Size) == false ||
-       Server->ModTime(File,FailTime) == false)
+   if (not Server->Size(File.c_str(), Size) ||
+       not Server->ModTime(File.c_str(), FailTime))
    {
       Fail(true);
       return true;
@@ -1119,7 +1119,7 @@ bool FtpMethod::Fetch(FetchItem *Itm)
       FailFd = Fd.Fd();
       
       bool Missing;
-      if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing,Itm->MaximumSize,this) == false)
+      if (not Server->Get(File.c_str(), Fd, Res.ResumePoint, Hash, Missing, Itm->MaximumSize, this))
       {
 	 Fd.Close();
 
diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index 5597e7cff..08d030a17 100644
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -123,7 +123,7 @@ class GPGVMethod : public aptMethod
    protected:
    virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE;
    public:
-   GPGVMethod() : aptMethod("gpgv", "1.1", SingleInstance | SendConfig){};
+   GPGVMethod() : aptMethod("gpgv", "1.1", SingleInstance | SendConfig | SendURIEncoded){};
 };
 static void PushEntryWithKeyID(std::vector<std::string> &Signers, char * const buffer, bool const Debug)
 {
@@ -419,7 +419,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
 bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
 {
    URI const Get(Itm->Uri);
-   string const Path = Get.Host + Get.Path; // To account for relative paths
+   std::string const Path = DecodeSendURI(Get.Host + Get.Path); // To account for relative paths
    SignersStorage Signers;
 
    std::vector<std::string> keyFpts, keyFiles;
diff --git a/methods/http.cc b/methods/http.cc
index 0ef695166..b6d754037 100644
--- a/methods/http.cc
+++ b/methods/http.cc
@@ -901,9 +901,8 @@ void HttpMethod::SendReq(FetchItem *Itm)
    else
       requesturi = Uri;
 
-   // The "+" is encoded as a workaround for a amazon S3 bug
-   // see LP bugs #1003633 and #1086997.
-   requesturi = QuoteString(requesturi, "+~ ");
+   if (not _config->FindB("Acquire::Send-URI-Encoded", false))
+      requesturi = URIEncode(requesturi);
 
    /* Build the request. No keep-alive is included as it is the default
       in 1.1, can cause problems with proxies, and we are an HTTP/1.1
@@ -1022,7 +1021,7 @@ BaseHttpMethod::DealWithHeadersResult HttpMethod::DealWithHeaders(FetchResult &R
    return FILE_IS_OPEN;
 }
 									/*}}}*/
-HttpMethod::HttpMethod(std::string &&pProg) : BaseHttpMethod(std::move(pProg), "1.2", Pipeline | SendConfig) /*{{{*/
+HttpMethod::HttpMethod(std::string &&pProg) : BaseHttpMethod(std::move(pProg), "1.2", Pipeline | SendConfig | SendURIEncoded) /*{{{*/
 {
    SeccompFlags = aptMethod::BASE | aptMethod::NETWORK;
 
diff --git a/methods/mirror.cc b/methods/mirror.cc
index 3e382e497..5c4d341c2 100644
--- a/methods/mirror.cc
+++ b/methods/mirror.cc
@@ -97,7 +97,7 @@ class MirrorMethod : public aptMethod /*{{{*/
    void DealWithPendingItems(std::vector<std::string> const &baseuris, MirrorListInfo const &info, FetchItem *const Itm, std::function<void()> handler);
 
    public:
-   explicit MirrorMethod(std::string &&pProg) : aptMethod(std::move(pProg), "2.0", SingleInstance | Pipeline | SendConfig | AuxRequests), genrng(clock())
+   explicit MirrorMethod(std::string &&pProg) : aptMethod(std::move(pProg), "2.0", SingleInstance | Pipeline | SendConfig | AuxRequests | SendURIEncoded), genrng(clock())
    {
       SeccompFlags = aptMethod::BASE | aptMethod::DIRECTORY;
    }
diff --git a/methods/rred.cc b/methods/rred.cc
index 2164cd19e..981364a9e 100644
--- a/methods/rred.cc
+++ b/methods/rred.cc
@@ -604,7 +604,7 @@ class RredMethod : public aptMethod {
       virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE {
 	 Debug = DebugEnabled();
 	 URI Get(Itm->Uri);
-	 std::string Path = Get.Host + Get.Path; // rred:/path - no host
+	 std::string Path = DecodeSendURI(Get.Host + Get.Path); // rred:/path - no host
 
 	 FetchResult Res;
 	 Res.Filename = Itm->DestFile;
@@ -750,7 +750,7 @@ class RredMethod : public aptMethod {
       }
 
    public:
-   RredMethod() : aptMethod("rred", "2.0", SendConfig), Debug(false)
+   RredMethod() : aptMethod("rred", "2.0", SendConfig | SendURIEncoded), Debug(false)
    {
       SeccompFlags = aptMethod::BASE | aptMethod::DIRECTORY;
    }
diff --git a/methods/rsh.cc b/methods/rsh.cc
index cc42b43e7..09d72cbc5 100644
--- a/methods/rsh.cc
+++ b/methods/rsh.cc
@@ -382,7 +382,7 @@ bool RSHConn::Get(const char *Path,FileFd &To,unsigned long long Resume,
 									/*}}}*/
 
 // RSHMethod::RSHMethod - Constructor					/*{{{*/
-RSHMethod::RSHMethod(std::string &&pProg) : aptMethod(std::move(pProg),"1.0",SendConfig)
+RSHMethod::RSHMethod(std::string &&pProg) : aptMethod(std::move(pProg),"1.0",SendConfig | SendURIEncoded)
 {
    signal(SIGTERM,SigTerm);
    signal(SIGINT,SigTerm);
@@ -434,7 +434,7 @@ void RSHMethod::SigTerm(int)
 bool RSHMethod::Fetch(FetchItem *Itm)
 {
    URI Get(Itm->Uri);
-   const char *File = Get.Path.c_str();
+   auto const File = DecodeSendURI(Get.Path);
    FetchResult Res;
    Res.Filename = Itm->DestFile;
    Res.IMSHit = false;
@@ -458,8 +458,8 @@ bool RSHMethod::Fetch(FetchItem *Itm)
 
    // Get the files information
    unsigned long long Size;
-   if (Server->Size(File,Size) == false ||
-       Server->ModTime(File,FailTime) == false)
+   if (not Server->Size(File.c_str(), Size) ||
+       not Server->ModTime(File.c_str(), FailTime))
    {
       //Fail(true);
       //_error->Error(_("File not found")); // Will be handled by Size
@@ -505,7 +505,7 @@ bool RSHMethod::Fetch(FetchItem *Itm)
       FailFd = Fd.Fd();
 
       bool Missing;
-      if (Server->Get(File,Fd,Res.ResumePoint,Hash,Missing,Res.Size) == false)
+      if (not Server->Get(File.c_str(), Fd, Res.ResumePoint, Hash, Missing, Res.Size))
       {
 	 Fd.Close();
 
diff --git a/methods/store.cc b/methods/store.cc
index 1b0f07947..8ffc7b93d 100644
--- a/methods/store.cc
+++ b/methods/store.cc
@@ -36,7 +36,7 @@ class StoreMethod : public aptMethod
 
    public:
 
-   explicit StoreMethod(std::string &&pProg) : aptMethod(std::move(pProg),"1.2",SingleInstance | SendConfig)
+   explicit StoreMethod(std::string &&pProg) : aptMethod(std::move(pProg),"1.2",SingleInstance | SendConfig | SendURIEncoded)
    {
       SeccompFlags = aptMethod::BASE;
       if (Binary != "store")
@@ -64,7 +64,7 @@ static bool OpenFileWithCompressorByName(FileFd &fileFd, std::string const &File
 bool StoreMethod::Fetch(FetchItem *Itm)					/*{{{*/
 {
    URI Get(Itm->Uri);
-   std::string Path = Get.Host + Get.Path; // To account for relative paths
+   std::string Path = DecodeSendURI(Get.Host + Get.Path); // To account for relative paths
    
    FetchResult Res;
    Res.Filename = Itm->DestFile;
diff --git a/test/integration/test-cve-2019-3462-dequote-injection b/test/integration/test-cve-2019-3462-dequote-injection
index 74ab03ba5..23cef4fae 100755
--- a/test/integration/test-cve-2019-3462-dequote-injection
+++ b/test/integration/test-cve-2019-3462-dequote-injection
@@ -16,9 +16,16 @@ SHA256="DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF"
 changetowebserver
 
 runwithbaduri() {
-	webserverconfig aptwebserver::redirect::replace::alpha_1_all.deb "$1"
+	local BADURI="$1"
+	local ERRMSG="$2"
+	shift 2
+	local BADFETCH="http://localhost:${APTHTTPPORT}/pool/alpha_1_all.deb"
+	if [ "$#" = '0' ]; then
+		BADFETCH="http://localhost:${APTHTTPPORT}/pool/$BADURI"
+	fi
+	webserverconfig aptwebserver::redirect::replace::alpha_1_all.deb "$BADURI"
 
-	testsuccess apt update -o debug::http=1 -o debug::pkgacquire::worker=1
+	testsuccess apt update -o debug::http=1 -o debug::pkgacquire::worker=1 "$@"
 
 	testfailureequal "Reading package lists...
 Building dependency tree...
@@ -28,13 +35,19 @@ The following NEW packages will be installed:
 Need to get 20.7 kB of archives.
 After this operation, 11.3 kB of additional disk space will be used.
 Err:1 http://localhost:${APTHTTPPORT} unstable/main all alpha all 1
-  SECURITY: URL redirect target contains control characters, rejecting.
-E: Failed to fetch http://localhost:${APTHTTPPORT}/pool/alpha_1_all.deb  SECURITY: URL redirect target contains control characters, rejecting.
-E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?" aptget install alpha
+  $ERRMSG
+E: Failed to fetch $BADFETCH  $ERRMSG
+E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?" aptget install alpha "$@"
 }
-runwithbaduri "beeta_1_all.deb%0a%0a201%20URI%20Done%0aURI:%20http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%0aFilename:%20${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%0aSize:%2020672%0aLast-Modified:%20Fri,%2018%20Jan%202019%2009:52:02%20+0000%0aSHA256-Hash:%20${SHA256}%0aChecksum-FileSize-Hash:%2012345%0a%0a%0a"
+runwithbaduri "beeta_1_all.deb%0a%0a201%20URI%20Done%0aURI:%20http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%0aFilename:%20${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%0aSize:%2020672%0aLast-Modified:%20Fri,%2018%20Jan%202019%2009:52:02%20+0000%0aSHA256-Hash:%20${SHA256}%0aChecksum-FileSize-Hash:%2012345%0a%0a%0a" 'SECURITY: URL redirect target contains control characters, rejecting.' -o Acquire::Send-URI-Encoded=false
+rm -rf rootdir/var/lib/apt/lists
+runwithbaduri "beeta_1_all.deb%250a%250a201%2520URI%2520Done%250aURI:%2520http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%250aFilename:%2520${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%250aSize:%252020672%250aLast-Modified:%2520Fri,%252018%2520Jan%25202019%252009:52:02%2520+0000%250aSHA256-Hash:%2520${SHA256}%250aChecksum-FileSize-Hash:%252012345%250a%250a%0a" 'SECURITY: URL redirect target contains control characters, rejecting.' -o Acquire::Send-URI-Encoded=false
+
+# without de- and reencoding, we just trigger an error in our webserver as it refuses URIs containing '//'
+rm -rf rootdir/var/lib/apt/lists
+runwithbaduri "beeta_1_all.deb%0a%0a201%20URI%20Done%0aURI:%20http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%0aFilename:%20${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%0aSize:%2020672%0aLast-Modified:%20Fri,%2018%20Jan%202019%2009:52:02%20+0000%0aSHA256-Hash:%20${SHA256}%0aChecksum-FileSize-Hash:%2012345%0a%0a%0a" '400  Bad Request'
 rm -rf rootdir/var/lib/apt/lists
-runwithbaduri "beeta_1_all.deb%250a%250a201%2520URI%2520Done%250aURI:%2520http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%250aFilename:%2520${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%250aSize:%252020672%250aLast-Modified:%2520Fri,%252018%2520Jan%25202019%252009:52:02%2520+0000%250aSHA256-Hash:%2520${SHA256}%250aChecksum-FileSize-Hash:%252012345%250a%250a%0a"
+runwithbaduri "beeta_1_all.deb%250a%250a201%2520URI%2520Done%250aURI:%2520http://localhost:${APTHTTPPORT}/pool/beeta_1_all.deb%250aFilename:%2520${TMPWORKINGDIRECTORY}/rootdir/var/cache/apt/archives/partial/alpha_1_all.deb%250aSize:%252020672%250aLast-Modified:%2520Fri,%252018%2520Jan%25202019%252009:52:02%2520+0000%250aSHA256-Hash:%2520${SHA256}%250aChecksum-FileSize-Hash:%252012345%250a%250a%0a" '400  Bad Request'
 
 # For reference, the following is the original reproducer/bug. It has
 # been disabled using exit 0, as it will fail in fixed versions.
-- 
cgit v1.2.3


From a5859bafdaa6bcf12934d0fb1715a5940965e13a Mon Sep 17 00:00:00 2001
From: David Kalnischkies <david@kalnischkies.de>
Date: Fri, 10 Jul 2020 20:19:31 +0200
Subject: Don't re-encode encoded URIs in pkgAcqFile
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit potentially breaks code feeding apt an encoded URI using a
method which does not get URIs send encoded. The webserverconfig
requests in our tests are an example for this – but they only worked
before if the server was expecting a double encoding as that was what
was happening to an encoded URI: so unlikely to work as expected in
practice.

Now with the new methods we can drop this double encoding and rely on
the URI being passed properly (and without modification) between the
layers so that passing in encoded URIs should now work correctly.
---
 apt-pkg/acquire-item.cc                 | 3 ++-
 test/interactive-helper/aptwebserver.cc | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 34528f2ec..a4ad6b854 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -3860,7 +3860,8 @@ pkgAcqFile::pkgAcqFile(pkgAcquire *const Owner, string const &URI, HashStringLis
       DestFile = flNotDir(URI);
 
    ::URI url{URI};
-   url.Path = pkgAcquire::URIEncode(url.Path);
+   if (url.Path.find(' ') != std::string::npos || url.Path.find('%') == std::string::npos)
+      url.Path = pkgAcquire::URIEncode(url.Path);
 
    // Create the item
    Desc.URI = std::string(url);
diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc
index 57d215a65..58ba54f84 100644
--- a/test/interactive-helper/aptwebserver.cc
+++ b/test/interactive-helper/aptwebserver.cc
@@ -630,7 +630,7 @@ static bool handleOnTheFlyReconfiguration(std::ostream &log, int const client,/*
    size_t const pcount = EncodedParts.size();
    std::vector<std::string> parts(pcount);
    for (size_t i = 0; i < pcount; ++i)
-      parts[i] = DeQuoteString(DeQuoteString(EncodedParts[i]));
+      parts[i] = DeQuoteString(EncodedParts[i]);
    if (pcount == 4 && parts[1] == "set")
    {
       _config->Set(parts[2], parts[3]);
-- 
cgit v1.2.3