From 2d6c6c8809c7b4c1a009df48387ba15066fda7e2 Mon Sep 17 00:00:00 2001
From: David Kalnischkies <david@kalnischkies.de>
Date: Sun, 14 Jan 2018 00:07:20 +0100
Subject: Drop alternative URIs we got a hash-based fail from

If we got a file but it produced a hash error, mismatched size or
similar we shouldn't fallback to alternative URIs as they likely result
in the same error. If we can we should instead use another mirror.

We used to be a lot stricter by stopping all trys for this file if we
got a non-404 (or a hash-based) failure, but that is too hard as we
really want to try other mirrors (if we have them) in the hope that they
have the expected and correct files.
---
 apt-pkg/acquire-worker.cc | 84 +++++++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

(limited to 'apt-pkg/acquire-worker.cc')

diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc
index d159ef84f..c2bbf8bed 100644
--- a/apt-pkg/acquire-worker.cc
+++ b/apt-pkg/acquire-worker.cc
@@ -321,28 +321,35 @@ 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();
-	       if (Owner->IsRedirectionLoop(NewURI))
+	       // for a simplified retry a method might redirect without URI change
+	       // see also IsRedirectionLoop implementation
+	       if (desc.URI != NewURI)
 	       {
-		  std::string msg = Message;
-		  msg.append("\nFailReason: RedirectionLoop");
-		  Owner->Failed(msg, Config);
-		  if (Log != nullptr)
-		     Log->Fail(Owner->GetItemDesc());
-		  continue;
-	       }
+		  auto newuri = NewURI;
+		  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");
+		     Owner->Failed(msg, Config);
+		     if (Log != nullptr)
+			Log->Fail(Owner->GetItemDesc());
+		     continue;
+		  }
 
-	       if (Log != nullptr)
-		  Log->Done(desc);
+		  if (Log != nullptr)
+		     Log->Done(desc);
 
-	       ChangeSiteIsMirrorChange(NewURI, desc, Owner);
-	       desc.URI = NewURI;
+		  ChangeSiteIsMirrorChange(NewURI, desc, Owner);
+		  desc.URI = NewURI;
+	       }
 	       if (isDoomedItem(Owner) == false)
-	       {
-		  for (auto alt = AltUris.crbegin(); alt != AltUris.crend(); ++alt)
-		     Owner->PushAlternativeURI(std::string(*alt), {}, false);
 		  OwnerQ->Owner->Enqueue(desc);
-	       }
 	    }
             break;
          }
@@ -608,28 +615,33 @@ void pkgAcquire::Worker::HandleFailure(std::vector<pkgAcquire::Item *> const &It
 	 if (isDoomedItem(Owner) == false)
 	    OwnerQ->Owner->Enqueue(SavedDesc);
       }
-      else if (Owner->PopAlternativeURI(NewURI))
-      {
-	 Owner->FailMessage(Message);
-	 auto &desc = Owner->GetItemDesc();
-	 if (Log != nullptr)
-	    Log->Fail(desc);
-	 ChangeSiteIsMirrorChange(NewURI, desc, Owner);
-	 desc.URI = NewURI;
-	 if (isDoomedItem(Owner) == false)
-	    OwnerQ->Owner->Enqueue(desc);
-      }
       else
       {
-	 if (errAuthErr && Owner->GetExpectedHashes().empty() == false)
-	    Owner->Status = pkgAcquire::Item::StatAuthError;
-	 else if (errTransient)
-	    Owner->Status = pkgAcquire::Item::StatTransientNetworkError;
-	 auto SavedDesc = Owner->GetItemDesc();
-	 if (isDoomedItem(Owner) == false)
-	    Owner->Failed(Message, Config);
-	 if (Log != nullptr)
-	    Log->Fail(SavedDesc);
+	 if (errAuthErr)
+	    Owner->RemoveAlternativeSite(URI::SiteOnly(Owner->GetItemDesc().URI));
+	 if (Owner->PopAlternativeURI(NewURI))
+	 {
+	    Owner->FailMessage(Message);
+	    auto &desc = Owner->GetItemDesc();
+	    if (Log != nullptr)
+	       Log->Fail(desc);
+	    ChangeSiteIsMirrorChange(NewURI, desc, Owner);
+	    desc.URI = NewURI;
+	    if (isDoomedItem(Owner) == false)
+	       OwnerQ->Owner->Enqueue(desc);
+	 }
+	 else
+	 {
+	    if (errAuthErr && Owner->GetExpectedHashes().empty() == false)
+	       Owner->Status = pkgAcquire::Item::StatAuthError;
+	    else if (errTransient)
+	       Owner->Status = pkgAcquire::Item::StatTransientNetworkError;
+	    auto SavedDesc = Owner->GetItemDesc();
+	    if (isDoomedItem(Owner) == false)
+	       Owner->Failed(Message, Config);
+	    if (Log != nullptr)
+	       Log->Fail(SavedDesc);
+	 }
       }
    }
 }
-- 
cgit v1.2.3