From a4b8112b19763cbd2c12b81d55bc7d43a591d610 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 9 Jul 2015 12:21:20 +0200 Subject: [PATCH] handle site-changing redirects as mirror changes MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Redirectors like httpredir.debian.org orchestra the download from multiple (hopefully close) mirrors while having only a single central sources.list entry by using redirects. This has the effect that the progress report always shows the source it started with, not the mirror it ends up fetching from, which is especially problematic for error reporting as having a report for a "Hashsum mismatch" for the redirector URI is next to useless as nobody knows which URI it was really fetched from (regardless of it coming from a user or via the report script) from this output alone. You would need to enable debug output and hope for the same situation to arise again… We hence reuse the UsedMirror field of the mirror:// method and detect redirects which change the site and declare this new site as the UsedMirrror (and adapt the description). The disadvantage is that there is no obvious mapping anymore (it is relatively easy to guess through with some experience) from progress lines to sources.list lines, so error messages need to take care to use the Target description (rather than current Item description) if they want to refer to the sources.list entry. --- apt-pkg/acquire-item.cc | 4 +- apt-pkg/acquire-worker.cc | 48 ++++++++++++------- ...test-handle-redirect-as-used-mirror-change | 23 +++++++++ 3 files changed, 56 insertions(+), 19 deletions(-) create mode 100755 test/integration/test-handle-redirect-as-used-mirror-change diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 38f753cbb..8a566fea0 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -457,7 +457,6 @@ void pkgAcquire::Item::Failed(string const &Message,pkgAcquire::MethodConfig con { if(ErrorText.empty()) ErrorText = LookupTag(Message,"Message"); - UsedMirror = LookupTag(Message,"UsedMirror"); if (QueueCounter <= 1) { /* This indicates that the file is not available right now but might @@ -516,11 +515,10 @@ void pkgAcquire::Item::Start(string const &/*Message*/, unsigned long long const } /*}}}*/ // Acquire::Item::Done - Item downloaded OK /*{{{*/ -void pkgAcquire::Item::Done(string const &Message, HashStringList const &Hashes, +void pkgAcquire::Item::Done(string const &/*Message*/, HashStringList const &Hashes, pkgAcquire::MethodConfig const * const /*Cnf*/) { // We just downloaded something.. - UsedMirror = LookupTag(Message,"UsedMirror"); if (FileSize == 0) { unsigned long long const downloadedSize = Hashes.FileSize(); diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index c0f93f9ce..dc03a8870 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -204,19 +204,22 @@ bool pkgAcquire::Worker::RunMessages() return _error->Error("Invalid message from method %s: %s",Access.c_str(),Message.c_str()); string URI = LookupTag(Message,"URI"); - pkgAcquire::Queue::QItem *Itm = 0; + pkgAcquire::Queue::QItem *Itm = NULL; if (URI.empty() == false) Itm = OwnerQ->FindItem(URI,this); - // update used mirror - string UsedMirror = LookupTag(Message,"UsedMirror", ""); - if (!UsedMirror.empty() && - Itm && - Itm->Description.find(" ") != string::npos) + if (Itm != NULL) { - Itm->Description.replace(0, Itm->Description.find(" "), UsedMirror); - // FIXME: will we need this as well? - //Itm->ShortDesc = UsedMirror; + // update used mirror + string UsedMirror = LookupTag(Message,"UsedMirror", ""); + if (UsedMirror.empty() == false) + { + for (pkgAcquire::Queue::QItem::owner_iterator O = Itm->Owners.begin(); O != Itm->Owners.end(); ++O) + (*O)->UsedMirror = UsedMirror; + + if (Itm->Description.find(" ") != string::npos) + Itm->Description.replace(0, Itm->Description.find(" "), UsedMirror); + } } // Determine the message number and dispatch @@ -248,17 +251,14 @@ bool pkgAcquire::Worker::RunMessages() break; } - string NewURI = LookupTag(Message,"New-URI",URI.c_str()); + std::string const NewURI = LookupTag(Message,"New-URI",URI.c_str()); Itm->URI = NewURI; ItemDone(); // Change the status so that it can be dequeued for (pkgAcquire::Queue::QItem::owner_iterator O = Itm->Owners.begin(); O != Itm->Owners.end(); ++O) - { - pkgAcquire::Item *Owner = *O; - Owner->Status = pkgAcquire::Item::StatIdle; - } + (*O)->Status = pkgAcquire::Item::StatIdle; // Mark the item as done (taking care of all queues) // and then put it in the main queue again std::vector const ItmOwners = Itm->Owners; @@ -267,12 +267,28 @@ bool pkgAcquire::Worker::RunMessages() for (pkgAcquire::Queue::QItem::owner_iterator O = ItmOwners.begin(); O != ItmOwners.end(); ++O) { pkgAcquire::Item *Owner = *O; - pkgAcquire::ItemDesc desc = Owner->GetItemDesc(); + pkgAcquire::ItemDesc &desc = Owner->GetItemDesc(); + // if we change site, treat it as a mirror change + if (URI::SiteOnly(NewURI) != URI::SiteOnly(desc.URI)) + { + std::string const OldSite = desc.Description.substr(0, desc.Description.find(" ")); + 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); + if (desc.Description.find(" ") != string::npos) + desc.Description.replace(0, desc.Description.find(" "), Owner->UsedMirror); + } + } + } desc.URI = NewURI; OwnerQ->Owner->Enqueue(desc); if (Log != 0) - Log->Done(Owner->GetItemDesc()); + Log->Done(desc); } break; } diff --git a/test/integration/test-handle-redirect-as-used-mirror-change b/test/integration/test-handle-redirect-as-used-mirror-change new file mode 100755 index 000000000..08a39a5e6 --- /dev/null +++ b/test/integration/test-handle-redirect-as-used-mirror-change @@ -0,0 +1,23 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'amd64' +configcompression '.' 'gz' + +buildsimplenativepackage 'unrelated' 'all' '0.5~squeeze1' 'unstable' + +setupaptarchive --no-update +changetowebserver -o 'aptwebserver::redirect::replace::/redirectme/=http://0.0.0.0:8080/' +rewritesourceslist 'http://localhost:8080/redirectme' + +testsuccessequal "Get:1 http://0.0.0.0:8080 unstable InRelease [$(stat -c %s aptarchive/dists/unstable/InRelease) B] +Get:2 http://0.0.0.0:8080 unstable/main Sources [$(stat -c %s aptarchive/dists/unstable/main/source/Sources.gz) B] +Get:3 http://0.0.0.0:8080 unstable/main amd64 Packages [$(stat -c %s aptarchive/dists/unstable/main/binary-amd64/Packages.gz) B] +Get:4 http://0.0.0.0:8080 unstable/main Translation-en [$(stat -c %s aptarchive/dists/unstable/main/i18n/Translation-en.gz) B] +Reading package lists..." aptget update + +testsuccessequal 'Hit:1 http://0.0.0.0:8080 unstable InRelease +Reading package lists...' aptget update -- 2.45.2