Allow HashSum mismatches to fallback to other mirrors

Perhaps a common error in the past nowadays thanks to by-hash we rarely
get mismatches due to our calling behaviour and more because the mirror
is genuinely bad especially if this mirror didn't work with by-hash.

Using our "normal" error handling allows falling back to other mirrors
which hopefully do support by-hash and/or are not broken on a filesystem
level making us more resistent to the occasional bad apple in a wild
mirror forest.

3 files changed, 94 insertions, 48 deletions

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index c50f46909..2c3f45076 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -950,7 +950,7 @@ void pkgAcquire::Item::FailMessage(string const &Message)
       failreason = WEAK_HASHSUMS;
    else if (FailReason == "RedirectionLoop")
       failreason = REDIRECTION_LOOP;
-   else if (Status == StatAuthError)
+   else if (Status == StatAuthError || FailReason == "HashSumMismatch")
       failreason = HASHSUM_MISMATCH;
 
    if(ErrorText.empty())
@@ -975,7 +975,7 @@ void pkgAcquire::Item::FailMessage(string const &Message)
 	    break;
       }
 
-      if (Status == StatAuthError)
+      if (Status == StatAuthError || failreason == HASHSUM_MISMATCH)
       {
 	 auto const ExpectedHashes = GetExpectedHashes();
 	 if (ExpectedHashes.empty() == false)
diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc
index 4b9d33505..55897945c 100644
--- a/apt-pkg/acquire-worker.cc
+++ b/apt-pkg/acquire-worker.cc
@@ -486,9 +486,21 @@ bool pkgAcquire::Worker::RunMessages()
 	    bool const isIMSHit = StringToBool(LookupTag(Message,"IMS-Hit"),false) ||
 	       StringToBool(LookupTag(Message,"Alt-IMS-Hit"),false);
 	    auto const forcedHash = _config->Find("Acquire::ForceHash");
+
+	    bool consideredOkay = true;
+	    HashStringList ExpectedHashes;
+	    bool const DebugAuth = _config->FindB("Debug::pkgAcquire::Auth", false);
+	    if (DebugAuth)
+	    {
+	       std::clog << "201 URI Done: " << URI << endl
+			 << "ReceivedHash:" << endl;
+	       for (auto const &hs : ReceivedHashes)
+		  std::clog << "\t- " << hs.toStr() << std::endl;
+	       std::clog << "ExpectedHash:" << endl;
+	    }
 	    for (auto const Owner: ItmOwners)
 	    {
-	       HashStringList const ExpectedHashes = [&]() {
+	       HashStringList const OwnerExpectedHashes = [&]() {
 		  if (AltFile)
 		  {
 		     auto const * const transOwner = dynamic_cast<pkgAcqTransactionItem const * const>(Owner);
@@ -501,46 +513,51 @@ bool pkgAcquire::Worker::RunMessages()
 		  }
 		  return Owner->GetExpectedHashes();
 	       }();
-	       if(_config->FindB("Debug::pkgAcquire::Auth", false) == true)
+	       for (auto const &h : OwnerExpectedHashes)
 	       {
-		  std::clog << "201 URI Done: " << Owner->DescURI() << endl
-		     << "ReceivedHash:" << endl;
-		  for (HashStringList::const_iterator hs = ReceivedHashes.begin(); hs != ReceivedHashes.end(); ++hs)
-		     std::clog <<  "\t- " << hs->toStr() << std::endl;
-		  std::clog << "ExpectedHash:" << endl;
-		  for (HashStringList::const_iterator hs = ExpectedHashes.begin(); hs != ExpectedHashes.end(); ++hs)
-		     std::clog <<  "\t- " << hs->toStr() << std::endl;
-		  std::clog << endl;
-	       }
-
-	       // decide if what we got is what we expected
-	       bool consideredOkay = false;
-	       if ((forcedHash.empty() && ExpectedHashes.empty() == false) ||
-		     (forcedHash.empty() == false && ExpectedHashes.usable()))
-	       {
-		  if (ReceivedHashes.empty())
+		  if (not ExpectedHashes.push_back(h))
 		  {
-		     /* IMS-Hits can't be checked here as we will have uncompressed file,
-			but the hashes for the compressed file. What we have was good through
-			so all we have to ensure later is that we are not stalled. */
-		     consideredOkay = isIMSHit;
-		  }
-		  else if (ReceivedHashes == ExpectedHashes)
-		     consideredOkay = true;
-		  else
 		     consideredOkay = false;
-
+		     std::clog << "\t- " << h.toStr() << " [conflict]" << std::endl;
+		  }
+		  else if (DebugAuth)
+		     std::clog << "\t- " << h.toStr() << std::endl;
+	       }
+	    }
+	    if (DebugAuth)
+	       std::clog << endl;
+
+	    // decide if what we got is what we expected
+	    if (not consideredOkay)
+	       ;
+	    else if ((forcedHash.empty() && not ExpectedHashes.empty()) ||
+		     (not forcedHash.empty() && ExpectedHashes.usable()))
+	    {
+	       if (ReceivedHashes.empty())
+	       {
+		  /* IMS-Hits can't be checked here as we will have uncompressed file,
+		     but the hashes for the compressed file. What we have was good through
+		     so all we have to ensure later is that we are not stalled. */
+		  consideredOkay = isIMSHit;
 	       }
+	       else if (ReceivedHashes == ExpectedHashes)
+		  consideredOkay = true;
 	       else
-		  consideredOkay = !Owner->HashesRequired();
-
-	       if (consideredOkay == true)
-		  consideredOkay = Owner->VerifyDone(Message, Config);
-	       else // hashsum mismatch
-		  Owner->Status = pkgAcquire::Item::StatAuthError;
+		  consideredOkay = false;
+	    }
+	    else
+	       consideredOkay = std::none_of(ItmOwners.begin(), ItmOwners.end(), [](auto const * const O) { return O->HashesRequired(); });
 
+	    bool otherReasons = false;
+	    if (consideredOkay && not std::all_of(ItmOwners.begin(), ItmOwners.end(), [&](auto * const O) { return O->VerifyDone(Message, Config); }))
+	    {
+	       consideredOkay = false;
+	       otherReasons = true;
+	    }
 
-	       if (consideredOkay == true)
+	    if (consideredOkay)
+	    {
+	       for (auto const Owner : ItmOwners)
 	       {
 		  if (isDoomedItem(Owner) == false)
 		     Owner->Done(Message, ReceivedHashes, Config);
@@ -552,22 +569,21 @@ bool pkgAcquire::Worker::RunMessages()
 			Log->Done(Owner->GetItemDesc());
 		  }
 	       }
+	    }
+	    else
+	    {
+	       if (otherReasons)
+		  HandleFailure(ItmOwners, Config, Log, Message, false, false);
 	       else
 	       {
-		  auto SavedDesc = Owner->GetItemDesc();
-		  if (isDoomedItem(Owner) == false)
+		  if (Message.find("\nFailReason:") == std::string::npos)
 		  {
-		     if (Message.find("\nFailReason:") == std::string::npos)
-		     {
-			if (ReceivedHashes != ExpectedHashes)
-			   Message.append("\nFailReason: HashSumMismatch");
-			else
-			   Message.append("\nFailReason: WeakHashSums");
-		     }
-		     Owner->Failed(Message,Config);
+		     if (ReceivedHashes != ExpectedHashes)
+			Message.append("\nFailReason: HashSumMismatch");
+		     else
+			Message.append("\nFailReason: WeakHashSums");
 		  }
-		  if (Log != nullptr)
-		     Log->Fail(SavedDesc);
+		  HandleFailure(ItmOwners, Config, Log, Message, false, true);
 	       }
 	    }
 	    ItemDone();
diff --git a/test/integration/test-apt-update-incomplete-file-mirror b/test/integration/test-apt-update-incomplete-file-mirror
index 2d2125582..959ddc1a6 100755
--- a/test/integration/test-apt-update-incomplete-file-mirror
+++ b/test/integration/test-apt-update-incomplete-file-mirror
@@ -25,9 +25,39 @@ testsuccess apt update
 
 rm -rf rootdir/var/lib/apt/lists
 
+
+msgmsg 'File mirror was hacked'
+mkdir aptarchive2
+cp -a aptarchive/dists aptarchive2/
+rm -rf rootdir/var/lib/apt/lists
 find aptarchive/dists -name 'Packages' | while read FILE; do
 	echo 'hacked' > $FILE
 done
 testfailure apt update -o Debug::pkgAcquire::Worker=1
 testsuccessequal '4' grep -c -- '- Filesize:' rootdir/tmp/testfailure.output
 testsuccessequal '2' grep -c '%0aAlt-Checksum-FileSize-Hash:%20' rootdir/tmp/testfailure.output
+
+
+msgmsg 'Fallback over hashsum errors'
+rm -f rootdir/etc/apt/sources.list rootdir/etc/apt/sources.list.d/*
+echo "deb mirror+file:${TMPWORKINGDIRECTORY}/mirror.list unstable main" > rootdir/etc/apt/sources.list
+rm -rf rootdir/var/lib/apt/lists
+cat > mirror.list <<EOF
+copy:${TMPWORKINGDIRECTORY}/aptarchive	priority:1
+file:${TMPWORKINGDIRECTORY}/aptarchive2	priority:2
+EOF
+testsuccess apt update
+
+rm -rf rootdir/var/lib/apt/lists
+cat > mirror.list <<EOF
+file:${TMPWORKINGDIRECTORY}/aptarchive	priority:1
+copy:${TMPWORKINGDIRECTORY}/aptarchive2	priority:2
+EOF
+testsuccess apt update
+
+rm -rf rootdir/var/lib/apt/lists
+cat > mirror.list <<EOF
+file:${TMPWORKINGDIRECTORY}/aptarchive	priority:1
+file:${TMPWORKINGDIRECTORY}/aptarchive2	priority:2
+EOF
+testsuccess apt update