From 02567e3084d2faec92e8bf248e89fda6452e634b Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 9 Aug 2017 23:26:19 +0200 Subject: refactor message generation for methods The format isn't too hard to get right, but it gets funny with multiline fields (which we don't really have yet) and its just easier to deal with it once and for all which can be reused for more messages later. --- apt-pkg/acquire-method.cc | 190 ++++++++++++++++++++++++---------------------- 1 file changed, 100 insertions(+), 90 deletions(-) (limited to 'apt-pkg/acquire-method.cc') diff --git a/apt-pkg/acquire-method.cc b/apt-pkg/acquire-method.cc index 309b5dcf9..4e034b402 100644 --- a/apt-pkg/acquire-method.cc +++ b/apt-pkg/acquire-method.cc @@ -27,7 +27,10 @@ #include #include +#include #include +#include +#include #include #include #include @@ -39,33 +42,41 @@ using namespace std; +// poor mans unordered_map::try_emplace for C++11 as it is a C++17 feature /*{{{*/ +template +static void try_emplace(std::unordered_map &fields, std::string &&name, Arg &&value) +{ + if (fields.find(name) == fields.end()) + fields.emplace(std::move(name), std::forward(value)); +} + /*}}}*/ + // AcqMethod::pkgAcqMethod - Constructor /*{{{*/ // --------------------------------------------------------------------- /* This constructs the initialization text */ pkgAcqMethod::pkgAcqMethod(const char *Ver,unsigned long Flags) { - std::cout << "100 Capabilities\n" - << "Version: " << Ver << "\n"; - + std::unordered_map fields; + try_emplace(fields, "Version", Ver); if ((Flags & SingleInstance) == SingleInstance) - std::cout << "Single-Instance: true\n"; + try_emplace(fields, "Single-Instance", "true"); if ((Flags & Pipeline) == Pipeline) - std::cout << "Pipeline: true\n"; + try_emplace(fields, "Pipeline", "true"); if ((Flags & SendConfig) == SendConfig) - std::cout << "Send-Config: true\n"; + try_emplace(fields, "Send-Config", "true"); if ((Flags & LocalOnly) == LocalOnly) - std::cout <<"Local-Only: true\n"; + try_emplace(fields, "Local-Only", "true"); if ((Flags & NeedsCleanup) == NeedsCleanup) - std::cout << "Needs-Cleanup: true\n"; + try_emplace(fields, "Needs-Cleanup", "true"); if ((Flags & Removable) == Removable) - std::cout << "Removable: true\n"; + try_emplace(fields, "Removable", "true"); - std::cout << "\n" << std::flush; + SendMessage("100 Capabilities", std::move(fields)); SetNonBlock(STDIN_FILENO,true); @@ -73,6 +84,26 @@ pkgAcqMethod::pkgAcqMethod(const char *Ver,unsigned long Flags) QueueBack = 0; } /*}}}*/ +void pkgAcqMethod::SendMessage(std::string const &header, std::unordered_map &&fields) /*{{{*/ +{ + std::cout << header << '\n'; + for (auto const &f : fields) + { + if (f.second.empty()) + continue; + std::cout << f.first << ": "; + auto const lines = VectorizeString(f.second, '\n'); + if (likely(lines.empty() == false)) + { + std::copy(lines.begin(), std::prev(lines.end()), std::ostream_iterator(std::cout, "\n ")); + std::cout << *lines.rbegin(); + } + std::cout << '\n'; + } + std::cout << '\n' + << std::flush; +} + /*}}}*/ // AcqMethod::Fail - A fetch has failed /*{{{*/ // --------------------------------------------------------------------- /* */ @@ -97,40 +128,35 @@ void pkgAcqMethod::Fail(bool Transient) } /*}}}*/ // AcqMethod::Fail - A fetch has failed /*{{{*/ -// --------------------------------------------------------------------- -/* */ void pkgAcqMethod::Fail(string Err,bool Transient) { // Strip out junk from the error messages - for (string::iterator I = Err.begin(); I != Err.end(); ++I) - { - if (*I == '\r') - *I = ' '; - if (*I == '\n') - *I = ' '; - } - - if (Queue != 0) - { - std::cout << "400 URI Failure\nURI: " << Queue->Uri << "\n" - << "Message: " << Err; - if (IP.empty() == false && _config->FindB("Acquire::Failure::ShowIP", true) == true) - std::cout << " " << IP; - std::cout << "\n"; - Dequeue(); - } + std::transform(Err.begin(), Err.end(), Err.begin(), [](char const c) { + if (c == '\r' || c == '\n') + return ' '; + return c; + }); + if (IP.empty() == false && _config->FindB("Acquire::Failure::ShowIP", true) == true) + Err.append(" ").append(IP); + + std::unordered_map fields; + if (Queue != nullptr) + try_emplace(fields, "URI", Queue->Uri); else - std::cout << "400 URI Failure\nURI: \nMessage: " << Err << "\n"; + try_emplace(fields, "URI", ""); + try_emplace(fields, "Message", Err); if(FailReason.empty() == false) - std::cout << "FailReason: " << FailReason << "\n"; + try_emplace(fields, "FailReason", FailReason); if (UsedMirror.empty() == false) - std::cout << "UsedMirror: " << UsedMirror << "\n"; - // Set the transient flag + try_emplace(fields, "UsedMirror", UsedMirror); if (Transient == true) - std::cout << "Transient-Failure: true\n"; + try_emplace(fields, "Transient-Failure", "true"); - std::cout << "\n" << std::flush; + SendMessage("400 URI Failure", std::move(fields)); + + if (Queue != nullptr) + Dequeue(); } /*}}}*/ // AcqMethod::DropPrivsOrDie - Drop privileges or die /*{{{*/ @@ -146,96 +172,79 @@ void pkgAcqMethod::DropPrivsOrDie() /*}}}*/ // AcqMethod::URIStart - Indicate a download is starting /*{{{*/ -// --------------------------------------------------------------------- -/* */ void pkgAcqMethod::URIStart(FetchResult &Res) { if (Queue == 0) abort(); - std::cout << "200 URI Start\n" - << "URI: " << Queue->Uri << "\n"; + std::unordered_map fields; + try_emplace(fields, "URI", Queue->Uri); if (Res.Size != 0) - std::cout << "Size: " << std::to_string(Res.Size) << "\n"; - + try_emplace(fields, "Size", std::to_string(Res.Size)); if (Res.LastModified != 0) - std::cout << "Last-Modified: " << TimeRFC1123(Res.LastModified, true) << "\n"; - + try_emplace(fields, "Last-Modified", TimeRFC1123(Res.LastModified, true)); if (Res.ResumePoint != 0) - std::cout << "Resume-Point: " << std::to_string(Res.ResumePoint) << "\n"; - + try_emplace(fields, "Resume-Point", std::to_string(Res.ResumePoint)); if (UsedMirror.empty() == false) - std::cout << "UsedMirror: " << UsedMirror << "\n"; + try_emplace(fields, "UsedMirror", UsedMirror); - std::cout << "\n" << std::flush; + SendMessage("200 URI Start", std::move(fields)); } /*}}}*/ // AcqMethod::URIDone - A URI is finished /*{{{*/ -// --------------------------------------------------------------------- -/* */ -static void printHashStringList(char const *const Prefix, HashStringList const *const list) +static void printHashStringList(std::unordered_map &fields, std::string const &Prefix, HashStringList const &list) { - for (HashStringList::const_iterator hash = list->begin(); hash != list->end(); ++hash) - { - // very old compatibility name for MD5Sum - if (hash->HashType() == "MD5Sum") - std::cout << Prefix << "MD5-Hash: " << hash->HashValue() << "\n"; - std::cout << hash->HashType() << "-Hash: " << hash->HashValue() << "\n"; - } + for (auto const &hash : list) + { + // very old compatibility name for MD5Sum + if (hash.HashType() == "MD5Sum") + try_emplace(fields, Prefix + "MD5-Hash", hash.HashValue()); + try_emplace(fields, Prefix + hash.HashType() + "-Hash", hash.HashValue()); + } } void pkgAcqMethod::URIDone(FetchResult &Res, FetchResult *Alt) { if (Queue == 0) abort(); - std::cout << "201 URI Done\n" - << "URI: " << Queue->Uri << "\n"; - + std::unordered_map fields; + try_emplace(fields, "URI", Queue->Uri); if (Res.Filename.empty() == false) - std::cout << "Filename: " << Res.Filename << "\n"; - + try_emplace(fields, "Filename", Res.Filename); if (Res.Size != 0) - std::cout << "Size: " << std::to_string(Res.Size) << "\n"; - + try_emplace(fields, "Size", std::to_string(Res.Size)); if (Res.LastModified != 0) - std::cout << "Last-Modified: " << TimeRFC1123(Res.LastModified, true) << "\n"; - - printHashStringList("", &Res.Hashes); + try_emplace(fields, "Last-Modified", TimeRFC1123(Res.LastModified, true)); + printHashStringList(fields, "", Res.Hashes); if (UsedMirror.empty() == false) - std::cout << "UsedMirror: " << UsedMirror << "\n"; + try_emplace(fields, "UsedMirror", UsedMirror); if (Res.GPGVOutput.empty() == false) { - std::cout << "GPGVOutput:\n"; - for (vector::const_iterator I = Res.GPGVOutput.begin(); - I != Res.GPGVOutput.end(); ++I) - std::cout << " " << *I << "\n"; + std::ostringstream os; + std::copy(Res.GPGVOutput.begin(), Res.GPGVOutput.end() - 1, std::ostream_iterator(os, "\n")); + os << *Res.GPGVOutput.rbegin(); + try_emplace(fields, "GPGVOutput", os.str()); } - if (Res.ResumePoint != 0) - std::cout << "Resume-Point: " << std::to_string(Res.ResumePoint) << "\n"; - + try_emplace(fields, "Resume-Point", std::to_string(Res.ResumePoint)); if (Res.IMSHit == true) - std::cout << "IMS-Hit: true\n"; + try_emplace(fields, "IMS-Hit", "true"); - if (Alt != 0) + if (Alt != nullptr) { if (Alt->Filename.empty() == false) - std::cout << "Alt-Filename: " << Alt->Filename << "\n"; - + try_emplace(fields, "Alt-Filename", Alt->Filename); if (Alt->Size != 0) - std::cout << "Alt-Size: " << std::to_string(Alt->Size) << "\n"; - + try_emplace(fields, "Alt-Size", std::to_string(Alt->Size)); if (Alt->LastModified != 0) - std::cout << "Alt-Last-Modified: " << TimeRFC1123(Alt->LastModified, true) << "\n"; - - printHashStringList("Alt-", &Alt->Hashes); - + try_emplace(fields, "Alt-Last-Modified", TimeRFC1123(Alt->LastModified, true)); if (Alt->IMSHit == true) - std::cout << "Alt-IMS-Hit: true\n"; + try_emplace(fields, "Alt-IMS-Hit", "true"); + printHashStringList(fields, "Alt-", Alt->Hashes); } - std::cout << "\n" << std::flush; + SendMessage("201 URI Done", std::move(fields)); Dequeue(); } /*}}}*/ @@ -459,9 +468,10 @@ void pkgAcqMethod::Status(const char *Format,...) * the worker will enqueue again later on to the right queue */ void pkgAcqMethod::Redirect(const string &NewURI) { - std::cout << "103 Redirect\nURI: " << Queue->Uri << "\n" - << "New-URI: " << NewURI << "\n" - << "\n" << std::flush; + std::unordered_map fields; + try_emplace(fields, "URI", Queue->Uri); + try_emplace(fields, "New-URI", NewURI); + SendMessage("103 Redirect", std::move(fields)); Dequeue(); } /*}}}*/ -- cgit v1.2.3-70-g09d2 From 04ab37fecaf286f724bef2e0969d2b67ab5ac1b1 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 28 Oct 2017 00:01:27 +0200 Subject: require methods to request AuxRequest capability at startup Allowing a method to request work from other methods is a powerful capability which could be misused or exploited, so to slightly limited the surface let method opt-in into this capability on startup. --- apt-pkg/acquire-method.cc | 3 ++ apt-pkg/acquire-method.h | 14 ++++-- apt-pkg/acquire-worker.cc | 111 +++++++++++++++++++++++++++------------------- apt-pkg/acquire-worker.h | 3 ++ apt-pkg/acquire.cc | 23 +++++++--- apt-pkg/acquire.h | 8 +++- methods/mirror.cc | 2 +- 7 files changed, 107 insertions(+), 57 deletions(-) (limited to 'apt-pkg/acquire-method.cc') diff --git a/apt-pkg/acquire-method.cc b/apt-pkg/acquire-method.cc index 4e034b402..8934d87a0 100644 --- a/apt-pkg/acquire-method.cc +++ b/apt-pkg/acquire-method.cc @@ -76,6 +76,9 @@ pkgAcqMethod::pkgAcqMethod(const char *Ver,unsigned long Flags) if ((Flags & Removable) == Removable) try_emplace(fields, "Removable", "true"); + if ((Flags & AuxRequests) == AuxRequests) + try_emplace(fields, "AuxRequests", "true"); + SendMessage("100 Capabilities", std::move(fields)); SetNonBlock(STDIN_FILENO,true); diff --git a/apt-pkg/acquire-method.h b/apt-pkg/acquire-method.h index fa22085b9..664b95c18 100644 --- a/apt-pkg/acquire-method.h +++ b/apt-pkg/acquire-method.h @@ -108,10 +108,16 @@ class pkgAcqMethod void PrintStatus(char const * const header, const char* Format, va_list &args) const; public: - enum CnfFlags {SingleInstance = (1<<0), - Pipeline = (1<<1), SendConfig = (1<<2), - LocalOnly = (1<<3), NeedsCleanup = (1<<4), - Removable = (1<<5)}; + enum CnfFlags + { + SingleInstance = (1 << 0), + Pipeline = (1 << 1), + SendConfig = (1 << 2), + LocalOnly = (1 << 3), + NeedsCleanup = (1 << 4), + Removable = (1 << 5), + AuxRequests = (1 << 6) + }; void Log(const char *Format,...); void Status(const char *Format,...); diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index 016aebdcd..6cbf8b7aa 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -508,6 +508,26 @@ bool pkgAcquire::Worker::RunMessages() _error->Error("Method gave invalid Aux Request message"); break; } + else if (Config->GetAuxRequests() == false) + { + std::vector const ItmOwners = Itm->Owners; + Message.append("\nMessage: Method tried to make an Aux Request while not being allowed to do them"); + OwnerQ->ItemDone(Itm); + Itm = nullptr; + HandleFailure(ItmOwners, Config, Log, Message, false, false); + ItemDone(); + + std::string Msg = "600 URI Acquire\n"; + Msg.reserve(200); + Msg += "URI: " + LookupTag(Message, "Aux-URI", ""); + Msg += "\nFilename: /nonexistent/auxrequest.blocked"; + Msg += "\n\n"; + if (Debug == true) + clog << " -> " << Access << ':' << QuoteString(Msg, "\n") << endl; + OutQueue += Msg; + OutReady = true; + break; + } auto maxsizestr = LookupTag(Message, "MaximumSize", ""); unsigned long long const MaxSize = maxsizestr.empty() ? 0 : strtoull(maxsizestr.c_str(), nullptr, 10); @@ -554,44 +574,7 @@ bool pkgAcquire::Worker::RunMessages() errAuthErr = std::find(std::begin(reasons), std::end(reasons), failReason) != std::end(reasons); } } - - for (auto const Owner: ItmOwners) - { - std::string NewURI; - if (errTransient == true && Config->LocalOnly == false && Owner->ModifyRetries() != 0) - { - --Owner->ModifyRetries(); - Owner->FailMessage(Message); - auto SavedDesc = Owner->GetItemDesc(); - if (Log != nullptr) - Log->Fail(SavedDesc); - 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); - } - } + HandleFailure(ItmOwners, Config, Log, Message, errTransient, errAuthErr); ItemDone(); break; @@ -609,6 +592,49 @@ bool pkgAcquire::Worker::RunMessages() return true; } /*}}}*/ +void pkgAcquire::Worker::HandleFailure(std::vector const &ItmOwners, /*{{{*/ + pkgAcquire::MethodConfig *const Config, pkgAcquireStatus *const Log, + std::string const &Message, bool const errTransient, bool const errAuthErr) +{ + for (auto const Owner : ItmOwners) + { + std::string NewURI; + if (errTransient == true && Config->LocalOnly == false && Owner->ModifyRetries() != 0) + { + --Owner->ModifyRetries(); + Owner->FailMessage(Message); + auto SavedDesc = Owner->GetItemDesc(); + if (Log != nullptr) + Log->Fail(SavedDesc); + 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); + } + } +} + /*}}}*/ // Worker::Capabilities - 100 Capabilities handler /*{{{*/ // --------------------------------------------------------------------- /* This parses the capabilities message and dumps it into the configuration @@ -625,18 +651,13 @@ bool pkgAcquire::Worker::Capabilities(string Message) Config->LocalOnly = StringToBool(LookupTag(Message,"Local-Only"),false); Config->NeedsCleanup = StringToBool(LookupTag(Message,"Needs-Cleanup"),false); Config->Removable = StringToBool(LookupTag(Message,"Removable"),false); + Config->SetAuxRequests(StringToBool(LookupTag(Message, "AuxRequests"), false)); // Some debug text if (Debug == true) { clog << "Configured access method " << Config->Access << endl; - clog << "Version:" << Config->Version << - " SingleInstance:" << Config->SingleInstance << - " Pipeline:" << Config->Pipeline << - " SendConfig:" << Config->SendConfig << - " LocalOnly: " << Config->LocalOnly << - " NeedsCleanup: " << Config->NeedsCleanup << - " Removable: " << Config->Removable << endl; + clog << "Version:" << Config->Version << " SingleInstance:" << Config->SingleInstance << " Pipeline:" << Config->Pipeline << " SendConfig:" << Config->SendConfig << " LocalOnly: " << Config->LocalOnly << " NeedsCleanup: " << Config->NeedsCleanup << " Removable: " << Config->Removable << " AuxRequests: " << Config->GetAuxRequests() << endl; } return true; diff --git a/apt-pkg/acquire-worker.h b/apt-pkg/acquire-worker.h index 3c5c1cef6..f04db2b3c 100644 --- a/apt-pkg/acquire-worker.h +++ b/apt-pkg/acquire-worker.h @@ -329,6 +329,9 @@ class pkgAcquire::Worker : public WeakPointable private: APT_HIDDEN void PrepareFiles(char const * const caller, pkgAcquire::Queue::QItem const * const Itm); + APT_HIDDEN void HandleFailure(std::vector const &ItmOwners, + pkgAcquire::MethodConfig *const Config, pkgAcquireStatus *const Log, + std::string const &Message, bool const errTransient, bool const errAuthErr); }; /** @} */ diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc index 5fa456ce3..b25a4434a 100644 --- a/apt-pkg/acquire.cc +++ b/apt-pkg/acquire.cc @@ -855,12 +855,25 @@ pkgAcquire::UriIterator pkgAcquire::UriEnd() } /*}}}*/ // Acquire::MethodConfig::MethodConfig - Constructor /*{{{*/ -// --------------------------------------------------------------------- -/* */ -pkgAcquire::MethodConfig::MethodConfig() : d(NULL), Next(0), SingleInstance(false), - Pipeline(false), SendConfig(false), LocalOnly(false), NeedsCleanup(false), - Removable(false) +class pkgAcquire::MethodConfig::Private +{ + public: + bool AuxRequests = false; +}; +pkgAcquire::MethodConfig::MethodConfig() : d(new Private()), Next(0), SingleInstance(false), + Pipeline(false), SendConfig(false), LocalOnly(false), NeedsCleanup(false), + Removable(false) +{ +} + /*}}}*/ +bool pkgAcquire::MethodConfig::GetAuxRequests() const /*{{{*/ +{ + return d->AuxRequests; +} + /*}}}*/ +void pkgAcquire::MethodConfig::SetAuxRequests(bool const value) /*{{{*/ { + d->AuxRequests = value; } /*}}}*/ // Queue::Queue - Constructor /*{{{*/ diff --git a/apt-pkg/acquire.h b/apt-pkg/acquire.h index e58aeef65..1cf4da5bf 100644 --- a/apt-pkg/acquire.h +++ b/apt-pkg/acquire.h @@ -636,9 +636,10 @@ class pkgAcquire::UriIterator /** \brief Information about the properties of a single acquire method. {{{*/ struct pkgAcquire::MethodConfig { + class Private; /** \brief dpointer placeholder (for later in case we need it) */ - void * const d; - + Private *const d; + /** \brief The next link on the acquire method list. * * \todo Why not an STL container? @@ -688,6 +689,9 @@ struct pkgAcquire::MethodConfig */ MethodConfig(); + APT_HIDDEN bool GetAuxRequests() const; + APT_HIDDEN void SetAuxRequests(bool const value); + virtual ~MethodConfig(); }; /*}}}*/ diff --git a/methods/mirror.cc b/methods/mirror.cc index ad8867836..ee703aaae 100644 --- a/methods/mirror.cc +++ b/methods/mirror.cc @@ -64,7 +64,7 @@ class MirrorMethod : public aptMethod /*{{{*/ void DealWithPendingItems(std::vector const &baseuris, MirrorInfo const &info, FetchItem *const Itm, std::function handler); public: - MirrorMethod(std::string &&pProg) : aptMethod(std::move(pProg), "2.0", SingleInstance | Pipeline | SendConfig) + MirrorMethod(std::string &&pProg) : aptMethod(std::move(pProg), "2.0", SingleInstance | Pipeline | SendConfig | AuxRequests) { SeccompFlags = aptMethod::BASE | aptMethod::DIRECTORY; -- cgit v1.2.3-70-g09d2