From 355e1aceac1dd05c4c7daf3420b09bd860fd169d Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 27 Oct 2017 19:09:45 +0200 Subject: implement fallback to alternative URIs for all items For deb files we always supported falling back from one server to the other if one failed to download the deb, but that was hardwired in the handling of this specific item. Moving this alongside the retry infrastructure we can implement it for all items and allow methods to use this as well by providing additional URIs in a redirect. --- apt-pkg/acquire-item.cc | 308 ++++++++++++----------- apt-pkg/acquire-item.h | 4 + apt-pkg/acquire-worker.cc | 53 +++- apt-pkg/indexfile.cc | 4 + apt-pkg/indexfile.h | 1 + test/integration/framework | 2 +- test/integration/test-bug-869859-retry-downloads | 18 ++ 7 files changed, 230 insertions(+), 160 deletions(-) diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 0e73b3b8c..dc45a6acd 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -683,6 +683,13 @@ class APT_HIDDEN CleanupItem : public pkgAcqTransactionItem /*{{{*/ class pkgAcquire::Item::Private { public: + struct AlternateURI + { + std::string const URI; + std::unordered_map changefields; + AlternateURI(std::string &&u, decltype(changefields) &&cf) : URI(u), changefields(cf) {} + }; + std::list AlternativeURIs; std::vector PastRedirections; std::unordered_map CustomFields; unsigned int Retries; @@ -723,6 +730,32 @@ std::unordered_map &pkgAcquire::Item::ModifyCustomFiel return d->CustomFields; } /*}}}*/ +bool pkgAcquire::Item::PopAlternativeURI(std::string &NewURI) /*{{{*/ +{ + if (d->AlternativeURIs.empty()) + return false; + auto const AltUri = d->AlternativeURIs.front(); + d->AlternativeURIs.pop_front(); + NewURI = AltUri.URI; + auto &CustomFields = ModifyCustomFields(); + for (auto const &f : AltUri.changefields) + { + if (f.second.empty()) + CustomFields.erase(f.first); + else + CustomFields[f.first] = f.second; + } + return true; +} + /*}}}*/ +void pkgAcquire::Item::PushAlternativeURI(std::string &&NewURI, std::unordered_map &&fields, bool const at_the_back) /*{{{*/ +{ + 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)); +} + /*}}}*/ unsigned int &pkgAcquire::Item::ModifyRetries() /*{{{*/ { return d->Retries; @@ -1070,6 +1103,7 @@ pkgAcqTransactionItem::pkgAcqTransactionItem(pkgAcquire * const Owner, /*{{{*/ {"Target-Release", Target.Option(IndexTarget::RELEASE)}, {"Target-Architecture", Target.Option(IndexTarget::ARCHITECTURE)}, {"Target-Language", Target.Option(IndexTarget::LANGUAGE)}, + {"Target-Type", "index"}, }; } /*}}}*/ @@ -3244,12 +3278,12 @@ pkgAcqIndex::~pkgAcqIndex() {} // --------------------------------------------------------------------- /* This just sets up the initial fetch environment and queues the first possibilitiy */ -pkgAcqArchive::pkgAcqArchive(pkgAcquire * const Owner,pkgSourceList * const Sources, - pkgRecords * const Recs,pkgCache::VerIterator const &Version, - string &StoreFilename) : - Item(Owner), d(NULL), LocalSource(false), Version(Version), Sources(Sources), Recs(Recs), - StoreFilename(StoreFilename), Vf(Version.FileList()), - Trusted(false) +APT_IGNORE_DEPRECATED_PUSH +pkgAcqArchive::pkgAcqArchive(pkgAcquire *const Owner, pkgSourceList *const Sources, + pkgRecords *const Recs, pkgCache::VerIterator const &Version, + string &StoreFilename) : Item(Owner), d(NULL), LocalSource(false), Version(Version), Sources(Sources), Recs(Recs), + StoreFilename(StoreFilename), Vf(), + Trusted(false) { if (Version.Arch() == 0) { @@ -3259,32 +3293,6 @@ pkgAcqArchive::pkgAcqArchive(pkgAcquire * const Owner,pkgSourceList * const Sour Version.ParentPkg().FullName().c_str()); return; } - - /* We need to find a filename to determine the extension. We make the - assumption here that all the available sources for this version share - the same extension.. */ - // Skip not source sources, they do not have file fields. - for (; Vf.end() == false; ++Vf) - { - if (Vf.File().Flagged(pkgCache::Flag::NotSource)) - continue; - break; - } - - // Does not really matter here.. we are going to fail out below - if (Vf.end() != true) - { - // If this fails to get a file name we will bomb out below. - pkgRecords::Parser &Parse = Recs->Lookup(Vf); - if (_error->PendingError() == true) - return; - - // Generate the final file name as: package_version_arch.foo - StoreFilename = QuoteString(Version.ParentPkg().Name(),"_:") + '_' + - QuoteString(Version.VerStr(),"_:") + '_' + - QuoteString(Version.Arch(),"_:.") + - "." + flExtension(Parse.FileName()); - } // check if we have one trusted source for the package. if so, switch // to "TrustedOnly" mode - but only if not in AllowUnauthenticated mode @@ -3317,58 +3325,60 @@ pkgAcqArchive::pkgAcqArchive(pkgAcquire * const Owner,pkgSourceList * const Sour if (allowUnauth == true && seenUntrusted == true) Trusted = false; - // Select a source - if (QueueNext() == false && _error->PendingError() == false) - _error->Error(_("Can't find a source to download version '%s' of '%s'"), - Version.VerStr(), Version.ParentPkg().FullName(false).c_str()); -} - /*}}}*/ -// AcqArchive::QueueNext - Queue the next file source /*{{{*/ -// --------------------------------------------------------------------- -/* This queues the next available file version for download. It checks if - the archive is already available in the cache and stashs the MD5 for - checking later. */ -bool pkgAcqArchive::QueueNext() -{ - for (; Vf.end() == false; ++Vf) + StoreFilename.clear(); + std::set targetComponents, targetCodenames, targetSuites; + for (auto Vf = Version.FileList(); Vf.end() == false; ++Vf) { - pkgCache::PkgFileIterator const PkgF = Vf.File(); - // Ignore not source sources + auto const PkgF = Vf.File(); + if (unlikely(PkgF.end())) + continue; if (PkgF.Flagged(pkgCache::Flag::NotSource)) continue; - - // Try to cross match against the source list pkgIndexFile *Index; if (Sources->FindIndex(PkgF, Index) == false) - continue; - LocalSource = PkgF.Flagged(pkgCache::Flag::LocalSource); - - // only try to get a trusted package from another source if that source - // is also trusted - if(Trusted && !Index->IsTrusted()) + continue; + if (Trusted && Index->IsTrusted() == false) continue; - // Grab the text package record pkgRecords::Parser &Parse = Recs->Lookup(Vf); - if (_error->PendingError() == true) - return false; - - string PkgFile = Parse.FileName(); - ExpectedHashes = Parse.Hashes(); - - if (PkgFile.empty() == true) - return _error->Error(_("The package index files are corrupted. No Filename: " - "field for package %s."), - Version.ParentPkg().Name()); + // collect the hashes from the indexes + auto hsl = Parse.Hashes(); + if (ExpectedHashes.empty()) + ExpectedHashes = hsl; + else + { + // bad things will likely happen, but the user might be "lucky" still + // if the sources provide the same hashtypes (so that they aren't mixed) + for (auto const &hs : hsl) + if (ExpectedHashes.push_back(hs) == false) + { + _error->Warning("Sources disagree on hashes for supposely identical version '%s' of '%s'.", + Version.VerStr(), Version.ParentPkg().FullName(false).c_str()); + break; + } + } + // only allow local volatile sources to have no hashes + if (PkgF.Flagged(pkgCache::Flag::LocalSource)) + LocalSource = true; + else if (hsl.empty()) + continue; - Desc.URI = Index->ArchiveURI(PkgFile); - Desc.Description = Index->ArchiveInfo(Version); - Desc.Owner = this; - Desc.ShortDesc = Version.ParentPkg().FullName(true); + std::string poolfilename = Parse.FileName(); + if (poolfilename.empty()) + continue; - auto fields = ModifyCustomFields(); - if (PkgF->Architecture != 0) - fields.emplace("Target-Architecture", PkgF.Architecture()); + std::remove_reference::type fields; + { + auto const debIndex = dynamic_cast(Index); + if (debIndex != nullptr) + { + auto const IT = debIndex->GetIndexTarget(); + fields.emplace("Target-Repo-URI", IT.Option(IndexTarget::REPO_URI)); + fields.emplace("Target-Release", IT.Option(IndexTarget::RELEASE)); + fields.emplace("Target-Site", IT.Option(IndexTarget::SITE)); + } + } + fields.emplace("Target-Base-URI", Index->ArchiveURI("")); if (PkgF->Component != 0) fields.emplace("Target-Component", PkgF.Component()); auto const RelF = PkgF.ReleaseFile(); @@ -3379,78 +3389,91 @@ bool pkgAcqArchive::QueueNext() if (RelF->Archive != 0) fields.emplace("Target-Suite", RelF.Archive()); } + fields.emplace("Target-Architecture", Version.Arch()); + fields.emplace("Target-Type", flExtension(poolfilename)); - // See if we already have the file. (Legacy filenames) - FileSize = Version->Size; - string FinalFile = _config->FindDir("Dir::Cache::Archives") + flNotDir(PkgFile); - struct stat Buf; - if (stat(FinalFile.c_str(),&Buf) == 0) + if (StoreFilename.empty()) { - // Make sure the size matches - if ((unsigned long long)Buf.st_size == Version->Size) - { - Complete = true; - Local = true; - Status = StatDone; - StoreFilename = DestFile = FinalFile; - return true; - } - - /* Hmm, we have a file and its size does not match, this means it is - an old style mismatched arch */ - RemoveFile("pkgAcqArchive::QueueNext", FinalFile); - } - - // Check it again using the new style output filenames - FinalFile = _config->FindDir("Dir::Cache::Archives") + flNotDir(StoreFilename); - if (stat(FinalFile.c_str(),&Buf) == 0) - { - // Make sure the size matches - if ((unsigned long long)Buf.st_size == Version->Size) - { - Complete = true; - Local = true; - Status = StatDone; - StoreFilename = DestFile = FinalFile; - return true; - } - - /* Hmm, we have a file and its size does not match, this shouldn't - happen.. */ - RemoveFile("pkgAcqArchive::QueueNext", FinalFile); - } - - DestFile = _config->FindDir("Dir::Cache::Archives") + "partial/" + flNotDir(StoreFilename); - - // Check the destination file - if (stat(DestFile.c_str(),&Buf) == 0) - { - // Hmm, the partial file is too big, erase it - if ((unsigned long long)Buf.st_size > Version->Size) - RemoveFile("pkgAcqArchive::QueueNext", DestFile); - else - PartialSize = Buf.st_size; + /* We pick a filename based on the information we have for the version, + but we don't know what extension such a file should have, so we look + at the filenames used online and assume that they are the same for + all repositories containing this file */ + StoreFilename = QuoteString(Version.ParentPkg().Name(), "_:") + '_' + + QuoteString(Version.VerStr(), "_:") + '_' + + QuoteString(Version.Arch(), "_:.") + + "." + flExtension(poolfilename); + + Desc.URI = Index->ArchiveURI(poolfilename); + Desc.Description = Index->ArchiveInfo(Version); + Desc.Owner = this; + Desc.ShortDesc = Version.ParentPkg().FullName(true); + auto &customfields = ModifyCustomFields(); + for (auto const &f : fields) + customfields[f.first] = f.second; + FileSize = Version->Size; } + else + PushAlternativeURI(Index->ArchiveURI(poolfilename), std::move(fields), true); + } + if (StoreFilename.empty()) + { + _error->Error(_("Can't find a source to download version '%s' of '%s'"), + Version.VerStr(), Version.ParentPkg().FullName(false).c_str()); + return; + } - // Disables download of archives - useful if no real installation follows, - // e.g. if we are just interested in proposed installation order - if (_config->FindB("Debug::pkgAcqArchive::NoQueue", false) == true) + // Check if we already downloaded the file + struct stat Buf; + auto FinalFile = _config->FindDir("Dir::Cache::Archives") + flNotDir(StoreFilename); + if (stat(FinalFile.c_str(), &Buf) == 0) + { + // Make sure the size matches + if ((unsigned long long)Buf.st_size == Version->Size) { Complete = true; Local = true; Status = StatDone; StoreFilename = DestFile = FinalFile; - return true; + return; } - // Create the item - Local = false; - ++Vf; - QueueURI(Desc); - return true; + /* Hmm, we have a file and its size does not match, this shouldn't + happen.. */ + RemoveFile("pkgAcqArchive::QueueNext", FinalFile); } + + // Check the destination file + DestFile = _config->FindDir("Dir::Cache::Archives") + "partial/" + flNotDir(StoreFilename); + if (stat(DestFile.c_str(), &Buf) == 0) + { + // Hmm, the partial file is too big, erase it + if ((unsigned long long)Buf.st_size > Version->Size) + RemoveFile("pkgAcqArchive::QueueNext", DestFile); + else + PartialSize = Buf.st_size; + } + + // Disables download of archives - useful if no real installation follows, + // e.g. if we are just interested in proposed installation order + if (_config->FindB("Debug::pkgAcqArchive::NoQueue", false) == true) + { + Complete = true; + Local = true; + Status = StatDone; + StoreFilename = DestFile = FinalFile; + return; + } + + // Create the item + Local = false; + QueueURI(Desc); +} +APT_IGNORE_DEPRECATED_POP + /*}}}*/ +bool pkgAcqArchive::QueueNext() /*{{{*/ +{ return false; -} +} /*}}}*/ // AcqArchive::Done - Finished fetching /*{{{*/ // --------------------------------------------------------------------- @@ -3483,25 +3506,6 @@ void pkgAcqArchive::Done(string const &Message, HashStringList const &Hashes, void pkgAcqArchive::Failed(string const &Message,pkgAcquire::MethodConfig const * const Cnf) { Item::Failed(Message,Cnf); - - /* We don't really want to retry on failed media swaps, this prevents - that. An interesting observation is that permanent failures are not - recorded. */ - if (Cnf->Removable == true && - StringToBool(LookupTag(Message,"Transient-Failure"),false) == true) - { - // Vf = Version.FileList(); - while (Vf.end() == false) ++Vf; - StoreFilename = string(); - return; - } - - Status = StatIdle; - if (QueueNext() == false) - { - StoreFilename = string(); - Status = StatError; - } } /*}}}*/ APT_PURE bool pkgAcqArchive::IsTrusted() const /*{{{*/ diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h index 7f5f75195..cf227d1b5 100644 --- a/apt-pkg/acquire-item.h +++ b/apt-pkg/acquire-item.h @@ -244,6 +244,9 @@ class pkgAcquire::Item : public WeakPointable /*{{{*/ APT_HIDDEN unsigned int &ModifyRetries(); // this is more a hack than a proper external interface, hence hidden APT_HIDDEN std::unordered_map &ModifyCustomFields(); + // this isn't the super nicest interface either… + APT_HIDDEN bool PopAlternativeURI(std::string &NewURI); + APT_HIDDEN void PushAlternativeURI(std::string &&NewURI, std::unordered_map &&fields, bool const at_the_back); /** \brief A "descriptive" URI-like string. * @@ -993,6 +996,7 @@ class pkgAcqArchive : public pkgAcquire::Item std::string &StoreFilename; /** \brief The next file for this version to try to download. */ + APT_DEPRECATED_MSG("Unused member") pkgCache::VerFileIterator Vf; /** \brief How many (more) times to try to find a new source from diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index a763d9242..995750dea 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -211,6 +211,39 @@ static bool isDoomedItem(pkgAcquire::Item const * const Itm) return false; return TransItm->TransactionManager->State != pkgAcqTransactionItem::TransactionStarted; } +static HashStringList GetHashesFromMessage(std::string const &Prefix, std::string const &Message) +{ + HashStringList hsl; + for (char const *const *type = HashString::SupportedHashes(); *type != NULL; ++type) + { + std::string const tagname = Prefix + *type + "-Hash"; + std::string const hashsum = LookupTag(Message, tagname.c_str()); + if (hashsum.empty() == false) + hsl.push_back(HashString(*type, hashsum)); + } + return hsl; +} +static void APT_NONNULL(3) ChangeSiteIsMirrorChange(std::string const &NewURI, pkgAcquire::ItemDesc &desc, pkgAcquire::Item *const Owner) +{ + if (URI::SiteOnly(NewURI) == URI::SiteOnly(desc.URI)) + return; + + auto const firstSpace = desc.Description.find(" "); + if (firstSpace != std::string::npos) + { + std::string const OldSite = desc.Description.substr(0, firstSpace); + if (likely(APT::String::Startswith(desc.URI, OldSite))) + { + std::string const OldExtra = desc.URI.substr(OldSite.length() + 1); + if (likely(APT::String::Endswith(NewURI, OldExtra))) + { + std::string const NewSite = NewURI.substr(0, NewURI.length() - OldExtra.length()); + Owner->UsedMirror = URI::ArchiveOnly(NewSite); + desc.Description.replace(0, firstSpace, Owner->UsedMirror); + } + } + } +} bool pkgAcquire::Worker::RunMessages() { while (MessageQueue.empty() == false) @@ -377,13 +410,7 @@ bool pkgAcquire::Worker::RunMessages() std::string const givenfilename = LookupTag(Message, "Filename"); std::string const filename = givenfilename.empty() ? Itm->Owner->DestFile : givenfilename; // see if we got hashes to verify - for (char const * const * type = HashString::SupportedHashes(); *type != NULL; ++type) - { - std::string const tagname = std::string(*type) + "-Hash"; - std::string const hashsum = LookupTag(Message, tagname.c_str()); - if (hashsum.empty() == false) - ReceivedHashes.push_back(HashString(*type, hashsum)); - } + ReceivedHashes = GetHashesFromMessage("", Message); // not all methods always sent Hashes our way if (ReceivedHashes.usable() == false) { @@ -525,6 +552,7 @@ bool pkgAcquire::Worker::RunMessages() for (auto const Owner: ItmOwners) { + std::string NewURI; if (errTransient == true && Config->LocalOnly == false && Owner->ModifyRetries() != 0) { --Owner->ModifyRetries(); @@ -535,6 +563,17 @@ bool pkgAcquire::Worker::RunMessages() 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) diff --git a/apt-pkg/indexfile.cc b/apt-pkg/indexfile.cc index 3ac5b3679..0e205a562 100644 --- a/apt-pkg/indexfile.cc +++ b/apt-pkg/indexfile.cc @@ -273,6 +273,10 @@ std::string pkgDebianIndexTargetFile::GetProgressDescription() const { return Target.Description; } +IndexTarget pkgDebianIndexTargetFile::GetIndexTarget() const +{ + return Target; +} pkgDebianIndexRealFile::pkgDebianIndexRealFile(std::string const &pFile, bool const Trusted) :/*{{{*/ pkgDebianIndexFile(Trusted), d(NULL) diff --git a/apt-pkg/indexfile.h b/apt-pkg/indexfile.h index 10b15fde4..7ebbd66f1 100644 --- a/apt-pkg/indexfile.h +++ b/apt-pkg/indexfile.h @@ -199,6 +199,7 @@ public: virtual std::string Describe(bool const Short = false) const APT_OVERRIDE; virtual bool Exists() const APT_OVERRIDE; virtual unsigned long Size() const APT_OVERRIDE; + IndexTarget GetIndexTarget() const APT_HIDDEN; pkgDebianIndexTargetFile(IndexTarget const &Target, bool const Trusted); virtual ~pkgDebianIndexTargetFile(); diff --git a/test/integration/framework b/test/integration/framework index bff6c0e65..f9d98835c 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -1298,7 +1298,7 @@ webserverconfig() { rewritesourceslist() { local APTARCHIVE="file://$(readlink -f "${TMPWORKINGDIRECTORY}/aptarchive" | sed 's# #%20#g')" local APTARCHIVE2="copy://$(readlink -f "${TMPWORKINGDIRECTORY}/aptarchive" | sed 's# #%20#g')" - for LIST in $(find rootdir/etc/apt/sources.list.d/ -name 'apt-test-*.list'); do + for LIST in $(find "${TMPWORKINGDIRECTORY}/rootdir/etc/apt/sources.list.d/" -name 'apt-test-*.list'); do sed -i $LIST -e "s#$APTARCHIVE#${1}#" -e "s#$APTARCHIVE2#${1}#" \ -e "s#http://[^@]*@\?localhost:${APTHTTPPORT}/\?#${1}#" \ -e "s#https://[^@]*@\?localhost:${APTHTTPSPORT}/\?#${1}#" diff --git a/test/integration/test-bug-869859-retry-downloads b/test/integration/test-bug-869859-retry-downloads index a62429a53..86203f794 100755 --- a/test/integration/test-bug-869859-retry-downloads +++ b/test/integration/test-bug-869859-retry-downloads @@ -35,3 +35,21 @@ webserverconfig 'aptwebserver::failrequest' '404' webserverconfig 'aptwebserver::failrequest::pool/testpkg_1_all.deb' '2' testfailure apt download testpkg -o acquire::retries=3 testfailure test -f testpkg_1_all.deb + +cat ../rootdir/etc/apt/sources.list.d/apt-test-*.list > ../rootdir/etc/apt/sources.list.d/00http-source.list +changetohttpswebserver + +msgmsg 'Check download from alternative sources if first failed' +webserverconfig 'aptwebserver::failrequest::pool/testpkg_1_all.deb' '0' +testsuccess apt update +testsuccess apt download testpkg -o acquire::retries=0 +testsuccess test -f testpkg_1_all.deb +rm -f testpkg_1_all.deb + +# we make the first source fail by disabling http support +webserverconfig 'aptwebserver::support::http' 'false' +testsuccess apt download testpkg -o acquire::retries=0 +cp ../rootdir/tmp/testsuccess.output alt.output +testsuccess grep '^ 400 Bad Request' alt.output +testsuccess test -f testpkg_1_all.deb +rm -f testpkg_1_all.deb -- cgit v1.2.3-70-g09d2