From 9b8034a9fd40b4d05075fda719e61f6eb4c45678 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 16 Apr 2016 00:07:28 +0200 Subject: [PATCH] use the same redirection mirror for all index files MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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. --- apt-pkg/acquire-item.cc | 24 +++++++++++++++++++ apt-pkg/acquire-item.h | 1 + ...test-handle-redirect-as-used-mirror-change | 15 ++++++++++++ 3 files changed, 40 insertions(+) diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index ceed5a510..317db65b8 100644 --- 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 61037f1b7..41420a7c1 100644 --- 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 3a8b1fecd..e9370930a 100755 --- 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 -- 2.45.2