Merge branch 'feature/byhashviaalturl' into 'master'

Don't force the same mirror for by-hash URIs

See merge request apt-team/apt!15

5 files changed, 194 insertions, 199 deletions

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index f5986a260..a366b8981 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -398,21 +398,66 @@ bool pkgAcqTransactionItem::QueueURI(pkgAcquire::ItemDesc &Item)
       Status = StatDone;
       return false;
    }
+   // this ensures we rewrite only once and only the first step
+   auto const OldBaseURI = Target.Option(IndexTarget::BASE_URI);
+   if (OldBaseURI.empty() || APT::String::Startswith(Item.URI, OldBaseURI) == false)
+      return pkgAcquire::Item::QueueURI(Item);
+   // the given URI is our last resort
+   PushAlternativeURI(std::string(Item.URI), {}, false);
    // If we got the InRelease file via a mirror, pick all indexes directly from this mirror, too
-   if (TransactionManager->BaseURI.empty() == false && UsedMirror.empty() &&
-	 URI::SiteOnly(Item.URI) != URI::SiteOnly(TransactionManager->BaseURI))
+   std::string SameMirrorURI;
+   if (TransactionManager->BaseURI.empty() == false && TransactionManager->UsedMirror.empty() == false &&
+       URI::SiteOnly(Item.URI) != URI::SiteOnly(TransactionManager->BaseURI))
    {
-      // this ensures we rewrite only once and only the first step
-      auto const OldBaseURI = Target.Option(IndexTarget::BASE_URI);
-      if (OldBaseURI.empty() == false && APT::String::Startswith(Item.URI, OldBaseURI))
+      auto ExtraPath = Item.URI.substr(OldBaseURI.length());
+      auto newURI = flCombine(TransactionManager->BaseURI, std::move(ExtraPath));
+      if (IsGoodAlternativeURI(newURI))
       {
-	 auto const ExtraPath = Item.URI.substr(OldBaseURI.length());
-	 Item.URI = flCombine(TransactionManager->BaseURI, ExtraPath);
-	 UsedMirror = TransactionManager->UsedMirror;
-	 if (Item.Description.find(" ") != string::npos)
-	    Item.Description.replace(0, Item.Description.find(" "), UsedMirror);
+	 SameMirrorURI = std::move(newURI);
+	 PushAlternativeURI(std::string(SameMirrorURI), {}, false);
       }
    }
+   // add URI and by-hash based on it
+   if (AcquireByHash())
+   {
+      // if we use the mirror transport, ask it for by-hash uris
+      // we need to stick to the same mirror only for non-unique filenames
+      auto const sameMirrorException = [&]() {
+	 if (Item.URI.find("mirror") == std::string::npos)
+	    return false;
+	 ::URI uri(Item.URI);
+	 return uri.Access == "mirror" || APT::String::Startswith(uri.Access, "mirror+") ||
+	    APT::String::Endswith(uri.Access, "+mirror") || uri.Access.find("+mirror+") != std::string::npos;
+      }();
+      if (sameMirrorException)
+	 SameMirrorURI.clear();
+      // now add the actual by-hash uris
+      auto const Expected = GetExpectedHashes();
+      auto const TargetHash = Expected.find(nullptr);
+      auto const PushByHashURI = [&](std::string U) {
+	 if (unlikely(TargetHash == nullptr))
+	    return false;
+	 auto const trailing_slash = U.find_last_of("/");
+	 if (unlikely(trailing_slash == std::string::npos))
+	    return false;
+	 auto byhashSuffix = "/by-hash/" + TargetHash->HashType() + "/" + TargetHash->HashValue();
+	 U.replace(trailing_slash, U.length() - trailing_slash, std::move(byhashSuffix));
+	 PushAlternativeURI(std::move(U), {}, false);
+	 return true;
+      };
+      PushByHashURI(Item.URI);
+      if (SameMirrorURI.empty() == false && PushByHashURI(SameMirrorURI) == false)
+	 SameMirrorURI.clear();
+   }
+   // the last URI added is the first one tried
+   if (unlikely(PopAlternativeURI(Item.URI) == false))
+      return false;
+   if (SameMirrorURI.empty() == false)
+   {
+      UsedMirror = TransactionManager->UsedMirror;
+      if (Item.Description.find(" ") != string::npos)
+	 Item.Description.replace(0, Item.Description.find(" "), UsedMirror);
+   }
    return pkgAcquire::Item::QueueURI(Item);
 }
 /* The transition manager InRelease itself (or its older sisters-in-law
@@ -619,6 +664,26 @@ bool pkgAcqDiffIndex::TransactionState(TransactionStates const state)
    return true;
 }
 									/*}}}*/
+// pkgAcqTransactionItem::AcquireByHash and specialisations for child classes	/*{{{*/
+bool pkgAcqTransactionItem::AcquireByHash() const
+{
+   if (TransactionManager->MetaIndexParser == nullptr)
+      return false;
+   auto const useByHashConf = Target.Option(IndexTarget::BY_HASH);
+   if (useByHashConf == "force")
+      return true;
+   return StringToBool(useByHashConf) == true && TransactionManager->MetaIndexParser->GetSupportsAcquireByHash();
+}
+// pdiff patches have a unique name already, no need for by-hash
+bool pkgAcqIndexMergeDiffs::AcquireByHash() const
+{
+   return false;
+}
+bool pkgAcqIndexDiffs::AcquireByHash() const
+{
+   return false;
+}
+									/*}}}*/
 
 class APT_HIDDEN NoActionItem : public pkgAcquire::Item			/*{{{*/
 /* The sole purpose of this class is having an item which does nothing to
@@ -686,11 +751,12 @@ class pkgAcquire::Item::Private
 public:
    struct AlternateURI
    {
-      std::string const URI;
+      std::string URI;
       std::unordered_map<std::string, std::string> changefields;
       AlternateURI(std::string &&u, decltype(changefields) &&cf) : URI(u), changefields(cf) {}
    };
    std::list<AlternateURI> AlternativeURIs;
+   std::vector<std::string> BadAlternativeSites;
    std::vector<std::string> PastRedirections;
    std::unordered_map<std::string, std::string> CustomFields;
    unsigned int Retries;
@@ -749,14 +815,32 @@ bool pkgAcquire::Item::PopAlternativeURI(std::string &NewURI) /*{{{*/
    return true;
 }
 									/*}}}*/
+bool pkgAcquire::Item::IsGoodAlternativeURI(std::string const &AltUri) const/*{{{*/
+{
+   return std::find(d->PastRedirections.cbegin(), d->PastRedirections.cend(), AltUri) == d->PastRedirections.cend() &&
+	 std::find(d->BadAlternativeSites.cbegin(), d->BadAlternativeSites.cend(), URI::SiteOnly(AltUri)) == d->BadAlternativeSites.cend();
+}
+									/*}}}*/
 void pkgAcquire::Item::PushAlternativeURI(std::string &&NewURI, std::unordered_map<std::string, std::string> &&fields, bool const at_the_back) /*{{{*/
 {
+   if (IsGoodAlternativeURI(NewURI) == false)
+      return;
    if (at_the_back)
       d->AlternativeURIs.emplace_back(std::move(NewURI), std::move(fields));
    else
       d->AlternativeURIs.emplace_front(std::move(NewURI), std::move(fields));
 }
 									/*}}}*/
+void pkgAcquire::Item::RemoveAlternativeSite(std::string &&OldSite) /*{{{*/
+{
+   d->AlternativeURIs.erase(std::remove_if(d->AlternativeURIs.begin(), d->AlternativeURIs.end(),
+					   [&](decltype(*d->AlternativeURIs.cbegin()) AltUri) {
+					      return URI::SiteOnly(AltUri.URI) == OldSite;
+					   }),
+			    d->AlternativeURIs.end());
+   d->BadAlternativeSites.push_back(std::move(OldSite));
+}
+									/*}}}*/
 unsigned int &pkgAcquire::Item::ModifyRetries() /*{{{*/
 {
    return d->Retries;
@@ -1429,7 +1513,6 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify)			/*{{{*/
 
    std::set<std::string> targetsSeen;
    bool const hasReleaseFile = TransactionManager->MetaIndexParser != NULL;
-   bool const metaBaseSupportsByHash = hasReleaseFile && TransactionManager->MetaIndexParser->GetSupportsAcquireByHash();
    bool hasHashes = true;
    auto IndexTargets = TransactionManager->MetaIndexParser->GetIndexTargets();
    if (hasReleaseFile && verify == false)
@@ -1565,15 +1648,6 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify)			/*{{{*/
 	 if (types.empty() == false)
 	 {
 	    std::ostringstream os;
-	    // add the special compressiontype byhash first if supported
-	    std::string const useByHashConf = Target.Option(IndexTarget::BY_HASH);
-	    bool useByHash = false;
-	    if(useByHashConf == "force")
-	       useByHash = true;
-	    else
-	       useByHash = StringToBool(useByHashConf) == true && metaBaseSupportsByHash;
-	    if (useByHash == true)
-	       os << "by-hash ";
 	    std::copy(types.begin(), types.end()-1, std::ostream_iterator<std::string>(os, " "));
 	    os << *types.rbegin();
 	    Target.Options["COMPRESSIONTYPES"] = os.str();
@@ -2219,8 +2293,6 @@ pkgAcqDiffIndex::pkgAcqDiffIndex(pkgAcquire * const Owner,
 	 CompressionExtensions = os.str();
       }
    }
-   if (Target.Option(IndexTarget::COMPRESSIONTYPES).find("by-hash") != std::string::npos)
-      CompressionExtensions = "by-hash " + CompressionExtensions;
    Init(GetDiffIndexURI(Target), GetDiffIndexFileName(Target.Description), Target.ShortDesc);
 
    if(Debug)
@@ -2231,7 +2303,7 @@ void pkgAcqDiffIndex::QueueOnIMSHit() const				/*{{{*/
 {
    // list cleanup needs to know that this file as well as the already
    // present index is ours, so we create an empty diff to save it for us
-   new pkgAcqIndexDiffs(Owner, TransactionManager, Target, UsedMirror, Target.URI);
+   new pkgAcqIndexDiffs(Owner, TransactionManager, Target);
 }
 									/*}}}*/
 static bool RemoveFileForBootstrapLinking(std::string &ErrorText, std::string const &For, std::string const &Boot)/*{{{*/
@@ -2585,7 +2657,7 @@ bool pkgAcqDiffIndex::ParseDiffIndex(string const &IndexDiffFile)	/*{{{*/
 									/*}}}*/
 void pkgAcqDiffIndex::Failed(string const &Message,pkgAcquire::MethodConfig const * const Cnf)/*{{{*/
 {
-   if (CommonFailed(GetDiffIndexURI(Target), GetDiffIndexFileName(Target.Description), Message, Cnf))
+   if (CommonFailed(GetDiffIndexURI(Target), Message, Cnf))
       return;
 
    RenameOnError(PDiffError);
@@ -2631,33 +2703,16 @@ void pkgAcqDiffIndex::Done(string const &Message,HashStringList const &Hashes,	/
    }
    else
    {
-      // we have something, queue the diffs
-      string::size_type const last_space = Description.rfind(" ");
-      if(last_space != string::npos)
-	 Description.erase(last_space, Description.size()-last_space);
-
-      std::string indexURI = Desc.URI;
-      auto const byhashidx = indexURI.find("/by-hash/");
-      if (byhashidx != std::string::npos)
-	 indexURI = indexURI.substr(0, byhashidx - strlen(".diff"));
-      else
-      {
-	 auto end = indexURI.length() - strlen(".diff/Index");
-	 if (CurrentCompressionExtension != "uncompressed")
-	    end -= (1 + CurrentCompressionExtension.length());
-	 indexURI = indexURI.substr(0, end);
-      }
-
       if (pdiff_merge == false)
-	 new pkgAcqIndexDiffs(Owner, TransactionManager, Target, UsedMirror, indexURI, available_patches);
+	 new pkgAcqIndexDiffs(Owner, TransactionManager, Target, available_patches);
       else
       {
 	 diffs = new std::vector<pkgAcqIndexMergeDiffs*>(available_patches.size());
 	 for(size_t i = 0; i < available_patches.size(); ++i)
 	    (*diffs)[i] = new pkgAcqIndexMergeDiffs(Owner, TransactionManager,
-		  Target, UsedMirror, indexURI,
-		  available_patches[i],
-		  diffs);
+						    Target,
+						    available_patches[i],
+						    diffs);
       }
    }
 
@@ -2681,28 +2736,20 @@ pkgAcqDiffIndex::~pkgAcqDiffIndex()
 /* The package diff is added to the queue. one object is constructed
  * for each diff and the index
  */
-pkgAcqIndexDiffs::pkgAcqIndexDiffs(pkgAcquire * const Owner,
-                                   pkgAcqMetaClearSig * const TransactionManager,
-                                   IndexTarget const &Target,
-				   std::string const &indexUsedMirror, std::string const &indexURI,
+pkgAcqIndexDiffs::pkgAcqIndexDiffs(pkgAcquire *const Owner,
+				   pkgAcqMetaClearSig *const TransactionManager,
+				   IndexTarget const &Target,
 				   vector<DiffInfo> const &diffs)
-   : pkgAcqBaseIndex(Owner, TransactionManager, Target), indexURI(indexURI),
-     available_patches(diffs)
+    : pkgAcqBaseIndex(Owner, TransactionManager, Target),
+      available_patches(diffs)
 {
    DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI), Target);
 
    Debug = _config->FindB("Debug::pkgAcquire::Diffs",false);
 
    Desc.Owner = this;
-   Description = Target.Description;
    Desc.ShortDesc = Target.ShortDesc;
 
-   UsedMirror = indexUsedMirror;
-   if (UsedMirror == "DIRECT")
-      UsedMirror.clear();
-   else if (UsedMirror.empty() == false && Description.find(" ") != string::npos)
-      Description.replace(0, Description.find(" "), UsedMirror);
-
    if(available_patches.empty() == true)
    {
       // we are done (yeah!), check hashes against the final file
@@ -2817,8 +2864,8 @@ bool pkgAcqIndexDiffs::QueueNextDiff()					/*{{{*/
    }
 
    // queue the right diff
-   Desc.URI = indexURI + ".diff/" + available_patches[0].file + ".gz";
-   Desc.Description = Description + " " + available_patches[0].file + string(".pdiff");
+   Desc.URI = Target.URI + ".diff/" + available_patches[0].file + ".gz";
+   Desc.Description = Target.Description + " " + available_patches[0].file + string(".pdiff");
    DestFile = GetKeepCompressedFileName(GetPartialFileNameFromURI(Target.URI + ".diff/" + available_patches[0].file), Target);
 
    if(Debug)
@@ -2870,7 +2917,7 @@ void pkgAcqIndexDiffs::Done(string const &Message, HashStringList const &Hashes,
 	 // see if there is more to download
 	 if(available_patches.empty() == false)
 	 {
-	    new pkgAcqIndexDiffs(Owner, TransactionManager, Target, UsedMirror, indexURI, available_patches);
+	    new pkgAcqIndexDiffs(Owner, TransactionManager, Target, available_patches);
 	    Finish();
 	 } else {
 	    DestFile = PatchedFile;
@@ -2896,28 +2943,20 @@ std::string pkgAcqIndexDiffs::Custom600Headers() const			/*{{{*/
 pkgAcqIndexDiffs::~pkgAcqIndexDiffs() {}
 
 // AcqIndexMergeDiffs::AcqIndexMergeDiffs - Constructor			/*{{{*/
-pkgAcqIndexMergeDiffs::pkgAcqIndexMergeDiffs(pkgAcquire * const Owner,
-                                             pkgAcqMetaClearSig * const TransactionManager,
-                                             IndexTarget const &Target,
-					     std::string const &indexUsedMirror, std::string const &indexURI,
-                                             DiffInfo const &patch,
-                                             std::vector<pkgAcqIndexMergeDiffs*> const * const allPatches)
-  : pkgAcqBaseIndex(Owner, TransactionManager, Target), indexURI(indexURI),
-    patch(patch), allPatches(allPatches), State(StateFetchDiff)
+pkgAcqIndexMergeDiffs::pkgAcqIndexMergeDiffs(pkgAcquire *const Owner,
+					     pkgAcqMetaClearSig *const TransactionManager,
+					     IndexTarget const &Target,
+					     DiffInfo const &patch,
+					     std::vector<pkgAcqIndexMergeDiffs *> const *const allPatches)
+    : pkgAcqBaseIndex(Owner, TransactionManager, Target),
+      patch(patch), allPatches(allPatches), State(StateFetchDiff)
 {
    Debug = _config->FindB("Debug::pkgAcquire::Diffs",false);
 
-   Description = Target.Description;
-   UsedMirror = indexUsedMirror;
-   if (UsedMirror == "DIRECT")
-      UsedMirror.clear();
-   else if (UsedMirror.empty() == false && Description.find(" ") != string::npos)
-      Description.replace(0, Description.find(" "), UsedMirror);
-
    Desc.Owner = this;
    Desc.ShortDesc = Target.ShortDesc;
-   Desc.URI = indexURI + ".diff/" + patch.file + ".gz";
-   Desc.Description = Description + " " + patch.file + ".pdiff";
+   Desc.URI = Target.URI + ".diff/" + patch.file + ".gz";
+   Desc.Description = Target.Description + " " + patch.file + ".pdiff";
    DestFile = GetPartialFileNameFromURI(Target.URI + ".diff/" + patch.file + ".gz");
 
    if(Debug)
@@ -3073,60 +3112,28 @@ pkgAcqIndex::pkgAcqIndex(pkgAcquire * const Owner,
 }
 									/*}}}*/
 // AcqIndex::Init - deferred Constructor				/*{{{*/
-static void NextCompressionExtension(std::string &CurrentCompressionExtension, std::string &CompressionExtensions, bool const preview)
+void pkgAcqIndex::Init(string const &URI, string const &URIDesc,
+                       string const &ShortDesc)
 {
+   Stage = STAGE_DOWNLOAD;
+
+   DestFile = GetPartialFileNameFromURI(URI);
    size_t const nextExt = CompressionExtensions.find(' ');
    if (nextExt == std::string::npos)
    {
       CurrentCompressionExtension = CompressionExtensions;
-      if (preview == false)
-	 CompressionExtensions.clear();
+      CompressionExtensions.clear();
    }
    else
    {
       CurrentCompressionExtension = CompressionExtensions.substr(0, nextExt);
-      if (preview == false)
-	 CompressionExtensions = CompressionExtensions.substr(nextExt+1);
+      CompressionExtensions = CompressionExtensions.substr(nextExt+1);
    }
-}
-void pkgAcqIndex::Init(string const &URI, string const &URIDesc,
-                       string const &ShortDesc)
-{
-   Stage = STAGE_DOWNLOAD;
-
-   DestFile = GetPartialFileNameFromURI(URI);
-   NextCompressionExtension(CurrentCompressionExtension, CompressionExtensions, false);
 
    if (CurrentCompressionExtension == "uncompressed")
    {
       Desc.URI = URI;
    }
-   else if (CurrentCompressionExtension == "by-hash")
-   {
-      NextCompressionExtension(CurrentCompressionExtension, CompressionExtensions, true);
-      if(unlikely(CurrentCompressionExtension.empty()))
-	 return;
-      if (CurrentCompressionExtension != "uncompressed")
-      {
-	 Desc.URI = URI + '.' + CurrentCompressionExtension;
-	 DestFile = DestFile + '.' + CurrentCompressionExtension;
-      }
-      else
-	 Desc.URI = URI;
-
-      HashStringList const Hashes = GetExpectedHashes();
-      HashString const * const TargetHash = Hashes.find(NULL);
-      if (unlikely(TargetHash == nullptr))
-	 return;
-      std::string const ByHash = "/by-hash/" + TargetHash->HashType() + "/" + TargetHash->HashValue();
-      size_t const trailing_slash = Desc.URI.find_last_of("/");
-      if (unlikely(trailing_slash == std::string::npos))
-	 return;
-      Desc.URI = Desc.URI.replace(
-	    trailing_slash,
-	    Desc.URI.substr(trailing_slash+1).size()+1,
-	    ByHash);
-   }
    else if (unlikely(CurrentCompressionExtension.empty()))
       return;
    else
@@ -3170,24 +3177,10 @@ string pkgAcqIndex::Custom600Headers() const
 }
 									/*}}}*/
 // AcqIndex::Failed - getting the indexfile failed			/*{{{*/
-bool pkgAcqIndex::CommonFailed(std::string const &TargetURI, std::string const TargetDesc,
-      std::string const &Message, pkgAcquire::MethodConfig const * const Cnf)
+bool pkgAcqIndex::CommonFailed(std::string const &TargetURI,
+			       std::string const &Message, pkgAcquire::MethodConfig const *const Cnf)
 {
    pkgAcqBaseIndex::Failed(Message,Cnf);
-
-   if (UsedMirror.empty() == false && UsedMirror != "DIRECT" &&
-	 LookupTag(Message, "FailReason") == "HttpError404")
-   {
-      UsedMirror = "DIRECT";
-      if (Desc.URI.find("/by-hash/") != std::string::npos)
-	 CompressionExtensions = "by-hash " + CompressionExtensions;
-      else
-	 CompressionExtensions = CurrentCompressionExtension + ' ' + CompressionExtensions;
-      Init(TargetURI, TargetDesc, Desc.ShortDesc);
-      Status = StatIdle;
-      return true;
-   }
-
    // authorisation matches will not be fixed by other compression types
    if (Status != StatAuthError)
    {
@@ -3202,7 +3195,7 @@ bool pkgAcqIndex::CommonFailed(std::string const &TargetURI, std::string const T
 }
 void pkgAcqIndex::Failed(string const &Message,pkgAcquire::MethodConfig const * const Cnf)
 {
-   if (CommonFailed(Target.URI, Target.Description, Message, Cnf))
+   if (CommonFailed(Target.URI, Message, Cnf))
       return;
 
    if(Target.IsOptional && GetExpectedHashes().empty() && Stage == STAGE_DOWNLOAD)
diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h
index 46d79df92..3a5a518c2 100644
--- a/apt-pkg/acquire-item.h
+++ b/apt-pkg/acquire-item.h
@@ -246,7 +246,9 @@ class pkgAcquire::Item : public WeakPointable				/*{{{*/
    APT_HIDDEN std::unordered_map<std::string, std::string> &ModifyCustomFields();
    // this isn't the super nicest interface either…
    APT_HIDDEN bool PopAlternativeURI(std::string &NewURI);
+   APT_HIDDEN bool IsGoodAlternativeURI(std::string const &AltUri) const;
    APT_HIDDEN void PushAlternativeURI(std::string &&NewURI, std::unordered_map<std::string, std::string> &&fields, bool const at_the_back);
+   APT_HIDDEN void RemoveAlternativeSite(std::string &&OldSite);
 
    /** \brief A "descriptive" URI-like string.
     *
@@ -405,7 +407,7 @@ class APT_HIDDEN pkgAcqTransactionItem: public pkgAcquire::Item		/*{{{*/
    virtual HashStringList GetExpectedHashes() const APT_OVERRIDE;
    virtual std::string GetMetaKey() const;
    virtual bool HashesRequired() const APT_OVERRIDE;
-
+   virtual bool AcquireByHash() const;
 
    pkgAcqTransactionItem(pkgAcquire * const Owner, pkgAcqMetaClearSig * const TransactionManager, IndexTarget const &Target) APT_NONNULL(2, 3);
    virtual ~pkgAcqTransactionItem();
@@ -690,8 +692,8 @@ class APT_HIDDEN pkgAcqIndex : public pkgAcqBaseIndex
    protected:
    APT_HIDDEN void Init(std::string const &URI, std::string const &URIDesc,
              std::string const &ShortDesc);
-   APT_HIDDEN bool CommonFailed(std::string const &TargetURI, std::string const TargetDesc,
-             std::string const &Message, pkgAcquire::MethodConfig const * const Cnf);
+   APT_HIDDEN bool CommonFailed(std::string const &TargetURI,
+				std::string const &Message, pkgAcquire::MethodConfig const *const Cnf);
 };
 									/*}}}*/
 struct APT_HIDDEN DiffInfo {						/*{{{*/
@@ -728,11 +730,6 @@ class APT_HIDDEN pkgAcqDiffIndex : public pkgAcqIndex
    /** \brief If \b true, debugging information will be written to std::clog. */
    bool Debug;
 
-   /** \brief A description of the Packages file (stored in
-    *  pkgAcquire::ItemDesc::Description).
-    */
-   std::string Description;
-
    /** \brief Get the full pathname of the final file for the current URI */
    virtual std::string GetFinalFilename() const APT_OVERRIDE;
 
@@ -790,8 +787,6 @@ class APT_HIDDEN pkgAcqDiffIndex : public pkgAcqIndex
  */
 class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
 {
-   std::string const indexURI;
-
    protected:
 
    /** \brief If \b true, debugging output will be written to
@@ -799,9 +794,6 @@ class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
     */
    bool Debug;
 
-   /** \brief description of the file being downloaded. */
-   std::string Description;
-
    /** \brief information about the current patch */
    struct DiffInfo const patch;
 
@@ -837,6 +829,7 @@ class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
    virtual std::string DescURI() const APT_OVERRIDE {return Target.URI + "Index";};
    virtual HashStringList GetExpectedHashes() const APT_OVERRIDE;
    virtual bool HashesRequired() const APT_OVERRIDE;
+   virtual bool AcquireByHash() const APT_OVERRIDE;
 
    /** \brief Create an index merge-diff item.
     *
@@ -850,10 +843,9 @@ class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
     *  \param allPatches contains all related items so that each item can
     *  check if it was the last one to complete the download step
     */
-   pkgAcqIndexMergeDiffs(pkgAcquire * const Owner, pkgAcqMetaClearSig * const TransactionManager,
-			 IndexTarget const &Target, std::string const &indexUsedMirror,
-			 std::string const &indexURI, DiffInfo const &patch,
-                         std::vector<pkgAcqIndexMergeDiffs*> const * const allPatches) APT_NONNULL(2, 3, 8);
+   pkgAcqIndexMergeDiffs(pkgAcquire *const Owner, pkgAcqMetaClearSig *const TransactionManager,
+			 IndexTarget const &Target, DiffInfo const &patch,
+			 std::vector<pkgAcqIndexMergeDiffs *> const *const allPatches) APT_NONNULL(2, 3, 6);
    virtual ~pkgAcqIndexMergeDiffs();
 };
 									/*}}}*/
@@ -870,8 +862,6 @@ class APT_HIDDEN pkgAcqIndexMergeDiffs : public pkgAcqBaseIndex
  */
 class APT_HIDDEN pkgAcqIndexDiffs : public pkgAcqBaseIndex
 {
-   std::string const indexURI;
-
    private:
 
    /** \brief Queue up the next diff download.
@@ -905,9 +895,6 @@ class APT_HIDDEN pkgAcqIndexDiffs : public pkgAcqBaseIndex
     */
    bool Debug;
 
-   /** A description of the file being downloaded. */
-   std::string Description;
-
    /** The patches that remain to be downloaded, including the patch
     *  being downloaded right now.  This list should be ordered so
     *  that each diff appears before any diff that depends on it.
@@ -943,6 +930,7 @@ class APT_HIDDEN pkgAcqIndexDiffs : public pkgAcqBaseIndex
    virtual std::string DescURI() const APT_OVERRIDE {return Target.URI + "IndexDiffs";};
    virtual HashStringList GetExpectedHashes() const APT_OVERRIDE;
    virtual bool HashesRequired() const APT_OVERRIDE;
+   virtual bool AcquireByHash() const APT_OVERRIDE;
 
    /** \brief Create an index diff item.
     *
@@ -957,10 +945,9 @@ class APT_HIDDEN pkgAcqIndexDiffs : public pkgAcqBaseIndex
     *  should be ordered so that each diff appears before any diff
     *  that depends on it.
     */
-   pkgAcqIndexDiffs(pkgAcquire * const Owner, pkgAcqMetaClearSig * const TransactionManager,
-                    IndexTarget const &Target,
-		    std::string const &indexUsedMirror, std::string const &indexURI,
-		    std::vector<DiffInfo> const &diffs=std::vector<DiffInfo>()) APT_NONNULL(2, 3);
+   pkgAcqIndexDiffs(pkgAcquire *const Owner, pkgAcqMetaClearSig *const TransactionManager,
+		    IndexTarget const &Target,
+		    std::vector<DiffInfo> const &diffs = std::vector<DiffInfo>()) APT_NONNULL(2, 3);
    virtual ~pkgAcqIndexDiffs();
 };
 									/*}}}*/
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);
+	 }
       }
    }
 }
diff --git a/test/integration/test-partial-file-support b/test/integration/test-partial-file-support
index 9b5eed1e5..88fa91324 100755
--- a/test/integration/test-partial-file-support
+++ b/test/integration/test-partial-file-support
@@ -24,7 +24,7 @@ testdownloadfile() {
 	else
 		msgpass
 	fi
-	sed -e '/^ <- / s#%20# #g' -e '/^ <- / s#%0a#\n#g' "$DOWNLOADLOG" | grep '^.*-Hash: ' > receivedhashes.log
+	sed -e '/^ <- / s#%20# #g' -e '/^ <- / s#%0a#\n#g' "$DOWNLOADLOG" | grep '^.*-Hash: ' > receivedhashes.log || true
 	testsuccess test -s receivedhashes.log
 	local HASHES_OK=0
 	local HASHES_BAD=0
diff --git a/test/integration/test-pdiff-usage b/test/integration/test-pdiff-usage
index 5a650ad83..7cda2ee45 100755
--- a/test/integration/test-pdiff-usage
+++ b/test/integration/test-pdiff-usage
@@ -138,6 +138,8 @@ SHA256-Download:
 	mkdir -p "${BYHASH}"
 	find "${NORMAL}/" -maxdepth 1 -name "Index*" -exec mv '{}' "$BYHASH" \;
 	ln -s "${BYHASH}/Index.gz" "${BYHASH}/$(sha256sum "${BYHASH}/Index.gz" | cut -f1 -d' ')"
+	echo 'foobar' > "${BYHASH}/$(sha256sum "$PATCHFILE" | cut -f1 -d' ')"
+	echo 'foobar' > "${BYHASH}/$(sha256sum "${PATCHFILE}.gz" | cut -f1 -d' ')"
 	rm -rf rootdir/var/lib/apt/lists
 	cp -a rootdir/var/lib/apt/lists-bak rootdir/var/lib/apt/lists
 	wasmergeused "$@" -o Acquire::By-Hash=force
@@ -398,7 +400,8 @@ testcase -o Acquire::IndexTargets::deb::Packages::KeepCompressed=true
 
 
 partialleftovers() { generatepartialleftovers "redirectme_Packages.${LOWCOSTEXT}" "redirectme_Packages-patched.${LOWCOSTEXT}"; }
-webserverconfig 'aptwebserver::redirect::replace::/redirectme/' "http://0.0.0.0:${APTHTTPPORT}/"
+# redirect the InRelease file only – the other files are auto-redirected by apt
+webserverconfig 'aptwebserver::redirect::replace::/redirectme/I' "http://0.0.0.0:${APTHTTPPORT}/I"
 rewritesourceslist "http://localhost:${APTHTTPPORT}/redirectme"
 aptautotest_apt_update() {
 	aptautotest_aptget_update "$@"