From dfcf7f356b790338f0a3e9df3c5d6f159814fe53 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 6 Mar 2016 12:03:34 +0100 Subject: do not move not-failed pdiff-patches into CWD on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 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 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'apt-pkg') 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); -- cgit v1.2.3