From dfcf7f356b790338f0a3e9df3c5d6f159814fe53 Mon Sep 17 00:00:00 2001
From: David Kalnischkies <david@kalnischkies.de>
Date: Sun, 6 Mar 2016 12:03:34 +0100
Subject: [PATCH] do not move not-failed pdiff-patches into CWD on failure
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

If a single pdiff fails, we have to fail the entire patching endeavour
and fall back to getting the complete file instead. That is easy in
serverside merged pdiffs as we get them one by one. For clientside we
get them all at once through, which means that a failure in one has to
stop the entire pipeline, which works as expected (as proven by the
bugreporters as they don't even notice it happening). The problem is
just that the first failing pdiff will do the cleanup, so another pdiff
which happens to be successfully acquired after we processed the failure
doesn't find the file it is supposed to use as a basename anymore, so
the patch is renamed to what should be the unique extension and moved
into the current working directory. Processing is then stopped as the
patch realizes that it isn't the last one which completed downloading.

On the plus side this means this is neither us using a bad temporary
location nor a security problem. It "just" overrides unconditionally
files in your current working directory (if you happen to have them
named like a pdiff patch – a bit unlikely perhaps) and so drops files
there which are never used again.

I guess this was introduced in 4e3c5633b1e74b4f58b95f339cfbbf4cbf21ab3e
for real as I made the need for the existence of the base file rather
explicit, but the potential lingers in the code for far longer.

Closes: #816837
---
 apt-pkg/acquire-item.cc           | 13 +++++++++++++
 test/integration/test-pdiff-usage | 13 +++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 62d960633..ad40f8974 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -2476,8 +2476,21 @@ void pkgAcqIndexMergeDiffs::Done(string const &Message, HashStringList const &Ha
 
    Item::Done(Message, Hashes, Cnf);
 
+   if (std::any_of(allPatches->begin(), allPatches->end(),
+	    [](pkgAcqIndexMergeDiffs const * const P) { return P->State == StateErrorDiff; }))
+   {
+      if(Debug)
+	 std::clog << "Another patch failed already, no point in processing this one." << std::endl;
+      return;
+   }
+
    std::string const UncompressedUnpatchedFile = GetPartialFileNameFromURI(Target.URI);
    std::string const UnpatchedFile = GetExistingFilename(UncompressedUnpatchedFile);
+   if (UnpatchedFile.empty())
+   {
+      _error->Fatal("Unpatched file %s doesn't exist (anymore)!", UnpatchedFile.c_str());
+      return;
+   }
    std::string const PatchFile = GetMergeDiffsPatchFileName(UnpatchedFile, patch.file);
    std::string const PatchedFile = GetKeepCompressedFileName(UncompressedUnpatchedFile, Target);
 
diff --git a/test/integration/test-pdiff-usage b/test/integration/test-pdiff-usage
index 253abb92c..98d75d894 100755
--- a/test/integration/test-pdiff-usage
+++ b/test/integration/test-pdiff-usage
@@ -164,6 +164,19 @@ SHA256-Download:
 	testsuccessequal "$(cat Packages-future)
 " aptcache show apt newstuff futurestuff
 
+	# we reuse the archive state of the previous test here
+	msgmsg "Testcase: downloading a patch fails, but successful fallback: $*"
+	rm -rf rootdir/var/lib/apt/lists
+	cp -a rootdir/var/lib/apt/lists-bak rootdir/var/lib/apt/lists
+	cp Packages-future aptarchive/Packages
+	rm "${PATCHFILE}.gz"
+	testsuccess apt update "$@"
+	cp rootdir/tmp/testsuccess.output patchdownload.output
+	testsuccess grep '^Falling back to normal index file acquire' patchdownload.output
+	testnopackage oldstuff
+	testsuccessequal "$(cat Packages-future)
+" aptcache show apt newstuff futurestuff
+
 	msgmsg "Testcase: patch applying fails, but successful fallback: $*"
 	rm -rf rootdir/var/lib/apt/lists
 	cp -a rootdir/var/lib/apt/lists-bak rootdir/var/lib/apt/lists
-- 
2.45.2