use the same redirection mirror for all index files

Redirection services like httpredir.debian.org tend to use a set of
mirrors from which they pick a mirror at "random" for each requested
file, which is usually benefitial for the download of debs, but for the
index files this can quickly cause problems (aka hashsum mismatches) if
the two (or more) mirrors involved are only slightly out-of-sync.

This commit "resolves" this issue by using the mirror we ended up using
to get the (signed) Release file directly to get the index files
belonging to this Release file instead of asking the redirection
service which eliminates the risk of hitting out-of-sync mirrors.

As an obvious downside the redirection service can't serve partial
mirrors anymore for indexes and the download of indexes indexed in the
same Release file can't be done in parallel (from different mirrors).

This does not effect the download of non-index files like deb-files as
out-of-sync mirrors aren't a huge problem there, so the parallel
download outweights a potentially 404 error (also because this causes no
errenous downloads while hashsum mismatches download the entire file
before finding out that it was pointless).

The rational for this is that indexes are relative to the Release file.
If we would be talking about a HTML page including images, such a
behaviour is obvious and intended – not doing it means in the best case
a bunch of "useless" requests which will all be answered with a
redirect.

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index ceed5a510fc4a847c69fd5d8dd25eadbf67e758a..317db65b87e6277ec024bfae40727129f595fa85 100644 (file)
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -338,6 +338,21 @@ bool pkgAcqTransactionItem::QueueURI(pkgAcquire::ItemDesc &Item)
       Status = StatDone;
       return false;
    }
+   // If we got the InRelease file via a mirror, pick all indexes directly from this mirror, too
+   if (TransactionManager != nullptr && TransactionManager->BaseURI.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 (APT::String::Startswith(Item.URI, OldBaseURI))
+      {
+        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);
+      }
+   }
    return pkgAcquire::Item::QueueURI(Item);
 }
 /* The transition manager InRelease itself (or its older sisters-in-law
@@ -1087,6 +1102,15 @@ bool pkgAcqMetaBase::CheckDownloadDone(pkgAcqTransactionItem * const I, const st
    // We have just finished downloading a Release file (it is not
    // verified yet)
 
+   // Save the final base URI we got this Release file from
+   if (I->UsedMirror.empty() == false && _config->FindB("Acquire::SameMirrorForAllIndexes", true))
+   {
+      if (APT::String::Endswith(I->Desc.URI, "InRelease"))
+        TransactionManager->BaseURI = I->Desc.URI.substr(0, I->Desc.URI.length() - strlen("InRelease"));
+      else
+        TransactionManager->BaseURI = I->Desc.URI.substr(0, I->Desc.URI.length() - strlen("Release"));
+   }
+
    std::string const FileName = LookupTag(Message,"Filename");
    if (FileName != I->DestFile && RealFileExists(I->DestFile) == false)
    {

diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h
index 61037f1b731ad4b5b55029a0baa2d12c5b798472..41420a7c113b1e84e2b917e2ef19e9abf5c045ae 100644 (file)
--- a/apt-pkg/acquire-item.h
+++ b/apt-pkg/acquire-item.h
@@ -469,6 +469,7 @@ class APT_HIDDEN pkgAcqMetaBase : public pkgAcqTransactionItem              /*{{{*/
    // This refers more to the Transaction-Manager than the actual file
    bool IMSHit;
    TransactionStates State;
+   std::string BaseURI;
 
    virtual bool QueueURI(pkgAcquire::ItemDesc &Item) APT_OVERRIDE;
    virtual HashStringList GetExpectedHashes() const APT_OVERRIDE;

diff --git a/test/integration/test-handle-redirect-as-used-mirror-change b/test/integration/test-handle-redirect-as-used-mirror-change
index 3a8b1fecd905da1ce9de9616a29ffe3d9514d052..e9370930aac180ae5ce274ed423e55ef061810da 100755 (executable)
--- a/test/integration/test-handle-redirect-as-used-mirror-change
+++ b/test/integration/test-handle-redirect-as-used-mirror-change
@@ -20,5 +20,20 @@ Get:3 http://0.0.0.0:${APTHTTPPORT} unstable/main all Packages [$(stat -c %s apt
 Get:4 http://0.0.0.0:${APTHTTPPORT} unstable/main Translation-en [$(stat -c %s aptarchive/dists/unstable/main/i18n/Translation-en.gz) B]
 Reading package lists..." aptget update
 
+# ensure we asked the redirector only once
+testsuccessequal "Location: http://0.0.0.0:${APTHTTPPORT}/dists/unstable/InRelease" grep '^Location:' aptarchive/webserver.log
+
 testsuccessequal "Hit:1 http://0.0.0.0:${APTHTTPPORT} unstable InRelease
 Reading package lists..." aptget update
+
+testsuccessequal "Location: http://0.0.0.0:${APTHTTPPORT}/dists/unstable/InRelease
+Location: http://0.0.0.0:${APTHTTPPORT}/dists/unstable/InRelease" grep '^Location:' aptarchive/webserver.log
+
+rm -rf rootdir/var/lib/apt/lists
+testsuccess apt update -o Debug::Acquire::http=1 -o Acquire::SameMirrorForAllIndexes=0
+testsuccessequal "Location: http://0.0.0.0:${APTHTTPPORT}/dists/unstable/InRelease
+Location: http://0.0.0.0:${APTHTTPPORT}/dists/unstable/InRelease
+Location: http://0.0.0.0:${APTHTTPPORT}/dists/unstable/InRelease
+Location: http://0.0.0.0:${APTHTTPPORT}/dists/unstable/main/source/Sources.gz
+Location: http://0.0.0.0:${APTHTTPPORT}/dists/unstable/main/binary-all/Packages.gz
+Location: http://0.0.0.0:${APTHTTPPORT}/dists/unstable/main/i18n/Translation-en.gz" grep '^Location:' aptarchive/webserver.log