From: Christos Tsantilas Date: Mon, 30 Jan 2017 12:46:15 +0000 (+0200) Subject: author: Eduard Bagdasaryan X-Git-Tag: M-staged-PR71~292 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=75d473407acdbf2d79f7a49b4d98083b95b83b9a;p=thirdparty%2Fsquid.git author: Eduard Bagdasaryan Two new ACLs implemented: annotate_transaction and annotate_client. Both ACLs always match and are useful for their side effect, immediately adding a key-value pair to the current transaction annotation (annotate_transaction) or to the current client-to-Squid connection annotation (annotate_client). Connection annotations are propagated to the current and all future master transactions on the annotated connection. Before this patch only 'clt_conn_tag' annotation tag could be used for a connection annotation. To reuse the existing notes parsing code, I had to refactor Note, Notes and NotePairs classes: * Made data members private and adjusted to follow 'rule of three'. Having public assess to containers with pointers may cause memory problems: for example ExternalACLEntry::update() called directly notes.entries.clear() without deleting the pointers. * None-fatal check for 'special' characters inside note name. * Used SBufs instead of Strings and const char* where possible. * Adjusted ACLNoteStrategy::matchNotes() to avoid 'expanding quoted values' code duplication inside Also fixed acl quoted flag parameters syntax. The old code improperly required quoting both flag and its parameter, e.g., "-m= ," whereas only parameter should be quoted: -m=" ,". Also moved UpdateRequestNotes() from Notes.cc to HttpRequest.cc to resolve dependency problems while bulding unit tests. TODO: transaction annotation matching code (ACLNoteData) performs parsing in its own way, using ACLStringData::parse(), lacking special characters/reserved keywords checks. Consider reusing the existing Notes parsing code instead. --- diff --git a/src/AccessLogEntry.cc b/src/AccessLogEntry.cc index 99eadcb0f6..5d4923d558 100644 --- a/src/AccessLogEntry.cc +++ b/src/AccessLogEntry.cc @@ -65,6 +65,18 @@ AccessLogEntry::getLogMethod() const return method; } +void +AccessLogEntry::syncNotes(HttpRequest *req) +{ + // XXX: auth code only has access to HttpRequest being authenticated + // so we must handle the case where HttpRequest is set without ALE being set. + assert(req); + if (!notes) + notes = req->notes(); + else + assert(notes == req->notes()); +} + AccessLogEntry::~AccessLogEntry() { safe_free(headers.request); diff --git a/src/AccessLogEntry.h b/src/AccessLogEntry.h index d9dee0a287..3e83ee2dfc 100644 --- a/src/AccessLogEntry.h +++ b/src/AccessLogEntry.h @@ -57,6 +57,8 @@ public: /// Fetch the transaction method string (ICP opcode, HTCP opcode or HTTP method) SBuf getLogMethod() const; + void syncNotes(HttpRequest *request); + SBuf url; /// TCP/IP level details about the client connection diff --git a/src/AclRegs.cc b/src/AclRegs.cc index dad76a2a28..9783721f8c 100644 --- a/src/AclRegs.cc +++ b/src/AclRegs.cc @@ -18,6 +18,9 @@ #include "acl/AdaptationServiceData.h" #endif #include "acl/AllOf.h" +#include "acl/AnnotateClient.h" +#include "acl/AnnotateTransaction.h" +#include "acl/AnnotationData.h" #include "acl/AnyOf.h" #if USE_SQUID_EUI #include "acl/Arp.h" @@ -224,6 +227,12 @@ Acl::AllOf Acl::AllOf::RegistryEntry_; ACL::Prototype ACLNote::RegistryProtoype(&ACLNote::RegistryEntry_, "note"); ACLStrategised ACLNote::RegistryEntry_(new ACLNoteData, ACLNoteStrategy::Instance(), "note"); +ACL::Prototype ACLAnnotateClient::RegistryProtoype(&ACLAnnotateClient::RegistryEntry_, "annotate_client"); +ACLStrategised ACLAnnotateClient::RegistryEntry_(new ACLAnnotationData, ACLAnnotateClientStrategy::Instance(), "annotate_client"); + +ACL::Prototype ACLAnnotateTransaction::RegistryProtoype(&ACLAnnotateTransaction::RegistryEntry_, "annotate_transaction"); +ACLStrategised ACLAnnotateTransaction::RegistryEntry_(new ACLAnnotationData, ACLAnnotateTransactionStrategy::Instance(), "annotate_transaction"); + #if USE_ADAPTATION ACL::Prototype ACLAdaptationService::RegistryProtoype(&ACLAdaptationService::RegistryEntry_, "adaptation_service"); ACLStrategised ACLAdaptationService::RegistryEntry_(new ACLAdaptationServiceData, ACLAdaptationServiceStrategy::Instance(), "adaptation_service"); diff --git a/src/ConfigParser.cc b/src/ConfigParser.cc index 43c2314b79..896ae5fb90 100644 --- a/src/ConfigParser.cc +++ b/src/ConfigParser.cc @@ -309,7 +309,7 @@ ConfigParser::TokenParse(const char * &nextToken, ConfigParser::TokenType &type) if (ConfigParser::StrictMode && type == ConfigParser::SimpleToken) { bool tokenIsNumber = true; for (const char *s = tokenStart; s != nextToken; ++s) { - const bool isValidChar = isalnum(*s) || strchr(".,()-=_/:", *s) || + const bool isValidChar = isalnum(*s) || strchr(".,()-=_/:+", *s) || (tokenIsNumber && *s == '%' && (s + 1 == nextToken)); if (!isdigit(*s)) diff --git a/src/ExternalACLEntry.cc b/src/ExternalACLEntry.cc index 298f2c22d1..0509b99b81 100644 --- a/src/ExternalACLEntry.cc +++ b/src/ExternalACLEntry.cc @@ -37,7 +37,7 @@ ExternalACLEntry::update(ExternalACLEntryData const &someData) result = someData.result; // replace all notes. not combine - notes.entries.clear(); + notes.clear(); notes.append(&someData.notes); #if USE_AUTH diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index 56d4570b58..3c5d05ed94 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -139,7 +139,7 @@ HttpRequest::clean() myportname.clean(); - notes = NULL; + theNotes = nullptr; tag.clean(); #if USE_AUTH @@ -249,7 +249,7 @@ HttpRequest::inheritProperties(const HttpMsg *aMsg) downloader = aReq->downloader; - notes = aReq->notes; + theNotes = aReq->theNotes; sources = aReq->sources; return true; @@ -667,3 +667,25 @@ HttpRequest::effectiveRequestUri() const return url.absolute(); } +NotePairs::Pointer +HttpRequest::notes() +{ + if (!theNotes) + theNotes = new NotePairs; + return theNotes; +} + +void +UpdateRequestNotes(ConnStateData *csd, HttpRequest &request, NotePairs const &helperNotes) +{ + // Tag client connection if the helper responded with clt_conn_tag=tag. + const char *cltTag = "clt_conn_tag"; + if (const char *connTag = helperNotes.findFirst(cltTag)) { + if (csd) { + csd->notes()->remove(cltTag); + csd->notes()->add(cltTag, connTag); + } + } + request.notes()->replaceOrAdd(&helperNotes); +} + diff --git a/src/HttpRequest.h b/src/HttpRequest.h index a43a07ce48..e84c738785 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -156,8 +156,6 @@ public: String myportname; // Internal tag name= value from port this requests arrived in. - NotePairs::Pointer notes; ///< annotations added by the note directive and helpers - String tag; /* Internal tag for this request */ String extacl_user; /* User name returned by extacl lookup */ @@ -220,9 +218,17 @@ public: void ignoreRange(const char *reason); int64_t getRangeOffsetLimit(); /* the result of this function gets cached in rangeOffsetLimit */ + /// \returns existing non-empty transaction annotations, + /// creates and returns empty annotations otherwise + NotePairs::Pointer notes(); + bool hasNotes() const { return bool(theNotes) && !theNotes->empty(); } + private: mutable int64_t rangeOffsetLimit; /* caches the result of getRangeOffsetLimit */ + /// annotations added by the note directive and helpers + /// and(or) by annotate_transaction/annotate_client ACLs. + NotePairs::Pointer theNotes; protected: virtual void packFirstLineInto(Packable * p, bool full_uri) const; @@ -233,5 +239,11 @@ protected: virtual bool inheritProperties(const HttpMsg *aMsg); }; +class ConnStateData; +/** + * Updates ConnStateData ids and HttpRequest notes from helpers received notes. + */ +void UpdateRequestNotes(ConnStateData *csd, HttpRequest &request, NotePairs const ¬es); + #endif /* SQUID_HTTPREQUEST_H */ diff --git a/src/Notes.cc b/src/Notes.cc index 4cc9130985..d30e5977c9 100644 --- a/src/Notes.cc +++ b/src/Notes.cc @@ -16,6 +16,8 @@ #include "http/Stream.h" #include "HttpReply.h" #include "HttpRequest.h" +#include "parser/Tokenizer.h" +#include "sbuf/StringConvert.h" #include "SquidConfig.h" #include "Store.h" #include "StrList.h" @@ -28,153 +30,236 @@ Note::Value::~Value() aclDestroyAclList(&aclList); } -Note::Value::Pointer -Note::addValue(const String &value) +Note::Value::Value(const char *aVal, const bool quoted, const char *descr, const Method m) + : aclList(nullptr), valueFormat(nullptr), theValue(aVal), theMethod(m) { - Value::Pointer v = new Value(value); - values.push_back(v); - return v; + if (quoted) { + valueFormat = new Format::Format(descr ? descr : "Notes"); + valueFormat->parse(theValue.c_str()); + } } -const char * -Note::match(HttpRequest *request, HttpReply *reply, const AccessLogEntry::Pointer &al) +const SBuf & +Note::Value::format(const AccessLogEntryPointer &al) { + if (al && valueFormat) { + static MemBuf mb; + mb.reset(); + valueFormat->assemble(mb, al, 0); + theFormattedValue.assign(mb.content()); + return theFormattedValue; + } + return theValue; +} - typedef Values::iterator VLI; - ACLFilledChecklist ch(NULL, request, NULL); +Note::Value::Pointer +Note::addValue(const char *value, const bool quoted, const char *descr, const Value::Method m) +{ + values.push_back(new Value(value, quoted, descr, m)); + return values.back(); +} + +bool +Note::match(HttpRequest *request, HttpReply *reply, const AccessLogEntry::Pointer &al, SBuf &matched) +{ + ACLFilledChecklist ch(nullptr, request, nullptr); ch.reply = reply; if (reply) HTTPMSGLOCK(ch.reply); - for (VLI i = values.begin(); i != values.end(); ++i ) { - const int ret= ch.fastCheck((*i)->aclList); - debugs(93, 5, HERE << "Check for header name: " << key << ": " << (*i)->value - <<", HttpRequest: " << request << " HttpReply: " << reply << " matched: " << ret); + for (auto v: values) { + assert(v->aclList); + const int ret = ch.fastCheck(v->aclList); + debugs(93, 5, "Check for header name: " << theKey << ": " << v->value() << + ", HttpRequest: " << request << " HttpReply: " << reply << " matched: " << ret); if (ret == ACCESS_ALLOWED) { - if (al != NULL && (*i)->valueFormat != NULL) { - static MemBuf mb; - mb.reset(); - (*i)->valueFormat->assemble(mb, al, 0); - return mb.content(); - } else - return (*i)->value.termedBuf(); + matched = v->format(al); + return true; } } - return NULL; + matched.clear(); + return false; } -Note::Pointer -Notes::add(const String ¬eKey) +void +Note::updateNotePairs(NotePairs::Pointer pairs, const CharacterSet *delimiters, const AccessLogEntryPointer &al) { - typedef Notes::NotesList::iterator AMLI; - for (AMLI i = notes.begin(); i != notes.end(); ++i) { - if ((*i)->key == noteKey) - return (*i); + for (auto v: values) { + const SBuf &formatted = v->format(al); + if (!pairs->empty() && v->method() == Value::mhReplace) + pairs->remove(theKey); + if (delimiters) + pairs->addStrList(key(), formatted, *delimiters); + else + pairs->add(key(), formatted); } +} - Note::Pointer note = new Note(noteKey); - notes.push_back(note); - return note; +void +Note::dump(StoreEntry *entry, const char *k) +{ + for (auto v: values) { + storeAppendPrintf(entry, "%s %.*s %s", + k, key().length(), key().rawContent(), ConfigParser::QuoteString(SBufToString(v->value()))); + dump_acl_list(entry, v->aclList); + storeAppendPrintf(entry, "\n"); + } +} + +SBuf +Note::toString(const char *sep) const +{ + SBuf result; + for (auto val: values) + result.appendf("%.*s: %.*s%s", key().length(), key().rawContent(), + val->value().length(), val->value().rawContent(), sep); + return result; } Note::Pointer -Notes::parse(ConfigParser &parser) +Notes::add(const SBuf ¬eKey) +{ + if (Note::Pointer p = find(noteKey)) + return p; + notes.push_back(new Note(noteKey)); + return notes.back(); +} + +Note::Pointer +Notes::find(const SBuf ¬eKey) +{ + for (auto n: notes) + if (n->key() == noteKey) + return n; + return nullptr; +} + +void +Notes::validateKey(const SBuf &key) const { - String key = ConfigParser::NextToken(); - ConfigParser::EnableMacros(); - String value = ConfigParser::NextQuotedToken(); - ConfigParser::DisableMacros(); - bool valueWasQuoted = ConfigParser::LastTokenWasQuoted(); - Note::Pointer note = add(key); - Note::Value::Pointer noteValue = note->addValue(value); - - String label(key); - label.append('='); - label.append(value); - aclParseAclList(parser, ¬eValue->aclList, label.termedBuf()); - if (formattedValues && valueWasQuoted) { - noteValue->valueFormat = new Format::Format(descr ? descr : "Notes"); - noteValue->valueFormat->parse(value.termedBuf()); - } if (blacklisted) { - for (int i = 0; blacklisted[i] != NULL; ++i) { - if (note->key.caseCmp(blacklisted[i]) == 0) { - fatalf("%s:%d: meta key \"%s\" is a reserved %s name", - cfg_filename, config_lineno, note->key.termedBuf(), + for (int i = 0; blacklisted[i] != nullptr; ++i) { + if (!key.cmp(blacklisted[i])) { + fatalf("%s:%d: meta key \"%.*s\" is a reserved %s name", + cfg_filename, config_lineno, key.length(), key.rawContent(), descr ? descr : ""); } } } + // TODO: fix code duplication: the same set of specials is produced + // by isKeyNameChar(). + static const CharacterSet allowedSpecials = CharacterSet::ALPHA + + CharacterSet::DIGIT + CharacterSet("specials", "-_"); + const auto specialIndex = key.findFirstNotOf(allowedSpecials); + if (specialIndex != SBuf::npos) { + debugs(28, DBG_CRITICAL, "Warning: used special character '" << + key[specialIndex] << "' within annotation name. " << + "Future Squid versions will not support this."); + } +} +Note::Pointer +Notes::parse(ConfigParser &parser) +{ + const char *tok = ConfigParser::NextToken(); + if (!tok) + fatalf("FATAL: Missing note key"); + SBuf key(tok); + validateKey(key); + ConfigParser::EnableMacros(); + const char *val = ConfigParser::NextQuotedToken(); + if (!val) + fatalf("FATAL: Missing note value"); + ConfigParser::DisableMacros(); + Note::Pointer note = add(key); + Note::Value::Pointer noteValue = note->addValue(val, formattedValues && ConfigParser::LastTokenWasQuoted(), descr); + key.append('='); + key.append(val); + aclParseAclList(parser, ¬eValue->aclList, key.c_str()); return note; } void -Notes::dump(StoreEntry *entry, const char *key) -{ - typedef Notes::NotesList::iterator AMLI; - for (AMLI m = notes.begin(); m != notes.end(); ++m) { - typedef Note::Values::iterator VLI; - for (VLI v =(*m)->values.begin(); v != (*m)->values.end(); ++v ) { - storeAppendPrintf(entry, "%s " SQUIDSTRINGPH " %s", - key, SQUIDSTRINGPRINT((*m)->key), ConfigParser::QuoteString((*v)->value)); - dump_acl_list(entry, (*v)->aclList); - storeAppendPrintf(entry, "\n"); +Notes::parseKvPair() { + char *k, *v; + int parsedPairs = 0; + while (ConfigParser::NextKvPair(k, v)) { + int keyLen = strlen(k); + const Note::Value::Method method = (k[keyLen - 1] == '+') ? Note::Value::mhAppend : Note::Value::mhReplace; + if (method == Note::Value::mhAppend) + keyLen--; + else { + assert(method == Note::Value::mhReplace); + if (Note::Pointer oldNote = find(SBuf(k, keyLen))) + debugs(28, DBG_CRITICAL, "Warning: annotation configuration with key " << k << + " already exists and will be overwritten"); } + SBuf key(k, keyLen); + validateKey(key); + Note::Pointer note = add(key); + (void)note->addValue(v, formattedValues && ConfigParser::LastTokenWasQuoted(), descr, method); + parsedPairs++; } + if (!parsedPairs) + fatalf("FATAL: Missing annotation kv pair"); } void -Notes::clean() +Notes::updateNotePairs(NotePairs::Pointer pairs, const CharacterSet *delimiters, const AccessLogEntry::Pointer &al) { - notes.clear(); + for (auto n: notes) + n->updateNotePairs(pairs, delimiters, al); } -NotePairs::~NotePairs() +void +Notes::dump(StoreEntry *entry, const char *key) { - while (!entries.empty()) { - delete entries.back(); - entries.pop_back(); - } + for (auto n: notes) + n->dump(entry, key); } const char * -NotePairs::find(const char *noteKey, const char *sep) const +Notes::toString(const char *sep) const +{ + static SBuf result; + result.clear(); + for (auto note: notes) + result.append(note->toString(sep)); + return result.isEmpty() ? nullptr : result.c_str(); +} + +bool +NotePairs::find(SBuf &resultNote, const char *noteKey, const char *sep) const { - static String value; - value.clean(); - for (std::vector::const_iterator i = entries.begin(); i != entries.end(); ++i) { - if ((*i)->name.cmp(noteKey) == 0) { - if (value.size()) - value.append(sep); - value.append((*i)->value); + resultNote.clear(); + for (auto e: entries) { + if (!e->name().cmp(noteKey)) { + if (!resultNote.isEmpty()) + resultNote.append(sep); + resultNote.append(e->value()); } } - return value.size() ? value.termedBuf() : NULL; + return resultNote.length(); } const char * NotePairs::toString(const char *sep) const { - static String value; - value.clean(); - for (std::vector::const_iterator i = entries.begin(); i != entries.end(); ++i) { - value.append((*i)->name); - value.append(": "); - value.append((*i)->value); - value.append(sep); - } - return value.size() ? value.termedBuf() : NULL; + static SBuf result; + result.clear(); + for (auto e: entries) + result.appendf("%.*s: %.*s%s", e->name().length(), e->name().rawContent(), + e->value().length(), e->value().rawContent(), sep); + return result.isEmpty() ? nullptr : result.c_str(); } const char * NotePairs::findFirst(const char *noteKey) const { - for (std::vector::const_iterator i = entries.begin(); i != entries.end(); ++i) { - if ((*i)->name.cmp(noteKey) == 0) - return (*i)->value.termedBuf(); - } - return NULL; + for (auto e: entries) + if (!e->name().cmp(noteKey)) + return const_cast(e->value()).c_str(); + return nullptr; } void @@ -183,96 +268,89 @@ NotePairs::add(const char *key, const char *note) entries.push_back(new NotePairs::Entry(key, note)); } +void +NotePairs::add(const SBuf &key, const SBuf ¬e) +{ + entries.push_back(new NotePairs::Entry(key, note)); +} + void NotePairs::remove(const char *key) { - std::vector::iterator i = entries.begin(); - while (i != entries.end()) { - if ((*i)->name.cmp(key) == 0) { - delete *i; - i = entries.erase(i); - } else { - ++i; - } - } + Entries::iterator i = entries.begin(); + while (i != entries.end()) + i = (*i)->name().cmp(key) ? i+1 : entries.erase(i); } void -NotePairs::addStrList(const char *key, const char *values) +NotePairs::remove(const SBuf &key) +{ + Entries::iterator i = entries.begin(); + while (i != entries.end()) + i = (*i)->name() == key ? entries.erase(i) : i+1; +} + +static void +AppendTokens(NotePairs::Entries &entries, const SBuf &key, const SBuf &val, const CharacterSet &delimiters) +{ + Parser::Tokenizer tok(val); + SBuf v; + while (tok.token(v, delimiters)) + entries.push_back(new NotePairs::Entry(key, v)); + v = tok.remaining(); + if (!v.isEmpty()) + entries.push_back(new NotePairs::Entry(key, v)); +} + +const NotePairs::Entries & +NotePairs::expandListEntries(const CharacterSet *delimiters) const { - String strValues(values); - const char *item; - const char *pos = NULL; - int ilen = 0; - while (strListGetItem(&strValues, ',', &item, &ilen, &pos)) { - String v; - v.append(item, ilen); - entries.push_back(new NotePairs::Entry(key, v.termedBuf())); + if (delimiters) { + static NotePairs::Entries expandedEntries; + expandedEntries.clear(); + for(auto entry: entries) + AppendTokens(expandedEntries, entry->name(), entry->value(), *delimiters); + return expandedEntries; } + return entries; +} + +void +NotePairs::addStrList(const SBuf &key, const SBuf &values, const CharacterSet &delimiters) +{ + AppendTokens(entries, key, values, delimiters); } bool -NotePairs::hasPair(const char *key, const char *value) const +NotePairs::hasPair(const SBuf &key, const SBuf &value) const { - for (std::vector::const_iterator i = entries.begin(); i != entries.end(); ++i) { - if ((*i)->name.cmp(key) == 0 && (*i)->value.cmp(value) == 0) + for (auto e: entries) + if (e->name() == key && e->value() == value) return true; - } return false; } void NotePairs::append(const NotePairs *src) { - for (std::vector::const_iterator i = src->entries.begin(); i != src->entries.end(); ++i) { - entries.push_back(new NotePairs::Entry((*i)->name.termedBuf(), (*i)->value.termedBuf())); - } + for (auto e: src->entries) + entries.push_back(new NotePairs::Entry(e->name(), e->value())); } void NotePairs::appendNewOnly(const NotePairs *src) { - for (std::vector::const_iterator i = src->entries.begin(); i != src->entries.end(); ++i) { - if (!hasPair((*i)->name.termedBuf(), (*i)->value.termedBuf())) - entries.push_back(new NotePairs::Entry((*i)->name.termedBuf(), (*i)->value.termedBuf())); + for (auto e: src->entries) { + if (!hasPair(e->name(), e->value())) + entries.push_back(new NotePairs::Entry(e->name(), e->value())); } } void NotePairs::replaceOrAdd(const NotePairs *src) { - for (std::vector::const_iterator i = src->entries.begin(); i != src->entries.end(); ++i) { - remove((*i)->name.termedBuf()); - } + for (auto e: src->entries) + remove(e->name()); append(src); } -NotePairs & -SyncNotes(AccessLogEntry &ale, HttpRequest &request) -{ - // XXX: auth code only has access to HttpRequest being authenticated - // so we must handle the case where HttpRequest is set without ALE being set. - - if (!ale.notes) { - if (!request.notes) - request.notes = new NotePairs; - ale.notes = request.notes; - } else { - assert(ale.notes == request.notes); - } - return *ale.notes; -} - -void -UpdateRequestNotes(ConnStateData *csd, HttpRequest &request, NotePairs const &helperNotes) -{ - // Tag client connection if the helper responded with clt_conn_tag=tag. - if (const char *connTag = helperNotes.findFirst("clt_conn_tag")) { - if (csd) - csd->connectionTag(connTag); - } - if (!request.notes) - request.notes = new NotePairs; - request.notes->replaceOrAdd(&helperNotes); -} - diff --git a/src/Notes.h b/src/Notes.h index b4d97bc044..2e5908e5c6 100644 --- a/src/Notes.h +++ b/src/Notes.h @@ -20,7 +20,11 @@ class HttpRequest; class HttpReply; +class AccessLogEntry; +class NotePairs; + typedef RefCount AccessLogEntryPointer; +typedef RefCount NotePairsPointer; /** * Used to store a note configuration. The notes are custom key:value @@ -32,85 +36,129 @@ class Note: public RefCountable { public: typedef RefCount Pointer; + /// Stores a value for the note. class Value: public RefCountable { public: typedef RefCount Pointer; - String value; ///< Configured annotation value, possibly with %macros - ACLList *aclList; ///< The access list used to determine if this value is valid for a request - /// Compiled annotation value format - Format::Format *valueFormat; - explicit Value(const String &aVal) : value(aVal), aclList(NULL), valueFormat(NULL) {} + friend class Note; + + enum Method { mhReplace, mhAppend }; + + Value(const char *aVal, const bool quoted, const char *descr, const Method method = mhReplace); ~Value(); - }; - typedef std::vector Values; + Value(const Value&) = delete; + Value &operator=(const Value&) = delete; + + Method method() const { return theMethod; } + const SBuf &value() const { return theValue; } - explicit Note(const String &aKey): key(aKey) {} + ACLList *aclList; ///< The access list used to determine if this value is valid for a request - /** - * Adds a value to the note and returns a pointer to the - * related Value object. - */ - Value::Pointer addValue(const String &value); + private: + /// \return the formatted value with expanded logformat %macros (quoted values). + /// \return the original value (non-quoted values). + const SBuf &format(const AccessLogEntryPointer &al); + + Format::Format *valueFormat; ///< Compiled annotation value format. + SBuf theValue; ///< Configured annotation value, possibly with %macros. + /// The expanded value produced by format(), empty for non-quoted values. + SBuf theFormattedValue; + /// Specifies how theValue will be applied to the existing annotation + /// with the same key: it either replaces the existing value or is appended + /// to the list of existing values. + Method theMethod; + }; + typedef std::vector Values; - /** - * Walks through the possible values list of the note and selects - * the first value which matches the given HttpRequest and HttpReply - * or NULL if none matches. - * If an AccessLogEntry given and Value::valueFormat is not null, the - * formatted value returned. - */ - const char *match(HttpRequest *request, HttpReply *reply, const AccessLogEntryPointer &al); + Note(const char *aKey, const size_t keyLen): theKey(aKey, keyLen) {} + explicit Note(const SBuf aKey): theKey(aKey) {} + Note(const Note&) = delete; + Note &operator=(const Note&) = delete; + + /// Adds a value to the note and returns a pointer to the + /// related Value object. + Value::Pointer addValue(const char *value, const bool quoted, const char *descr, + const Value::Method m = Value::mhAppend); + + /// Walks through the possible values list of the note, selects + /// the first value, matching the given HttpRequest and HttpReply + /// and assignes the given 'matched' to it. + /// \return true if matched, false otherwise + bool match(HttpRequest *request, HttpReply *reply, const AccessLogEntryPointer &al, SBuf &matched); + const SBuf &key() const { return theKey; } + void updateNotePairs(NotePairsPointer pairs, const CharacterSet *delimiters, const AccessLogEntryPointer &al); + /// Dump the single Note to the given StoreEntry object. + void dump(StoreEntry *entry, const char *key); + /// For the key and all its Values compile a string of + /// "Key: Value" pairs separated by sep string. + SBuf toString(const char *sep) const; - String key; ///< The note key +private: + SBuf theKey; ///< The note key Values values; ///< The possible values list for the note }; class ConfigParser; + /** * Used to store a notes configuration list. */ -class Notes +class Notes : public RefCountable { public: + typedef RefCount Pointer; typedef std::vector NotesList; typedef NotesList::iterator iterator; ///< iterates over the notes list typedef NotesList::const_iterator const_iterator; ///< iterates over the notes list - Notes(const char *aDescr, const char **metasBlacklist, bool allowFormatted = false): descr(aDescr), blacklisted(metasBlacklist), formattedValues(allowFormatted) {} - Notes(): descr(NULL), blacklisted(NULL), formattedValues(false) {} + Notes(const char *aDescr, const char **metasBlacklist, bool allowFormatted = true): descr(aDescr), blacklisted(metasBlacklist), formattedValues(allowFormatted) {} + Notes(): descr(nullptr), blacklisted(nullptr), formattedValues(false) {} ~Notes() { notes.clear(); } - /** - * Parse a notes line and returns a pointer to the - * parsed Note object. - */ + Notes(const Notes&) = delete; + Notes &operator=(const Notes&) = delete; + + /// Parses a notes line and returns a pointer to the parsed Note object. Note::Pointer parse(ConfigParser &parser); - /** - * Dump the notes list to the given StoreEntry object. - */ + + /// Parses an annotate line with "key=value" or "key+=value" formats. + void parseKvPair(); + + /// Dump the notes list to the given StoreEntry object. void dump(StoreEntry *entry, const char *name); - void clean(); /// clean the notes list + /// clean the notes list + void clean() { notes.clear(); } /// points to the first argument iterator begin() { return notes.begin(); } /// points to the end of list iterator end() { return notes.end(); } - /// return true if the notes list is empty - bool empty() { return notes.empty(); } + /// \returns true if the notes list is empty + bool empty() const { return notes.empty(); } + /// Convert Notes list to a string consist of "Key: Value" + /// entries separated by sep string. + const char *toString(const char *sep = "\r\n") const; + void updateNotePairs(NotePairsPointer pairs, const CharacterSet *delimiters, + const AccessLogEntryPointer &al); +private: + + /// Verifies that the key is not blacklisted (fatal error) and + /// does not contain special characters (non-fatal error). + /// If keyLen is not provided, the key is assumed null-terminated. + void validateKey(const SBuf &key) const; + + /// Adds a note to the notes list and returns a pointer to the + /// related Note object. If the note key already exists in list, + /// returns a pointer to the existing object. + /// If keyLen is not provided, the noteKey is assumed null-terminated. + Note::Pointer add(const SBuf ¬eKey); + Note::Pointer find(const SBuf ¬eKey); NotesList notes; ///< The Note::Pointer objects array list const char *descr; ///< A short description for notes list const char **blacklisted; ///< Null terminated list of blacklisted note keys bool formattedValues; ///< Whether the formatted values are supported - -private: - /** - * Adds a note to the notes list and returns a pointer to the - * related Note object. If the note key already exists in list, - * returns a pointer to the existing object. - */ - Note::Pointer add(const String ¬eKey); }; /** @@ -121,101 +169,87 @@ class NotePairs: public RefCountable public: typedef RefCount Pointer; - /** - * Used to store a note key/value pair. - */ - class Entry + /// Used to store a note key/value pair. + class Entry : public RefCountable { MEMPROXY_CLASS(Entry); public: - Entry(const char *aKey, const char *aValue): name(aKey), value(aValue) {} - String name; - String value; + typedef RefCount Pointer; + + Entry(const SBuf &aKey, const SBuf &aValue) + : theName(aKey), theValue(aValue) {} + Entry(const char *aKey, const char *aValue) + : theName(aKey), theValue(aValue) {} + Entry(const Entry &) = delete; + Entry &operator=(const Entry &) = delete; + + const SBuf &name() const { return theName; } + const SBuf &value() const { return theValue; } + + private: + SBuf theName; + SBuf theValue; }; + typedef std::vector Entries; ///< The key/value pair entries NotePairs() {} - ~NotePairs(); + NotePairs &operator=(NotePairs const &) = delete; + NotePairs(NotePairs const &) = delete; - /** - * Append the entries of the src NotePairs list to our list. - */ + /// Append the entries of the src NotePairs list to our list. void append(const NotePairs *src); - /** - * Replace existing list entries with the src NotePairs entries. - * Entries which do not exist in the destination set are added. - */ + /// Replace existing list entries with the src NotePairs entries. + /// Entries which do not exist in the destination set are added. void replaceOrAdd(const NotePairs *src); - /** - * Append any new entries of the src NotePairs list to our list. - * Entries which already exist in the destination set are ignored. - */ + /// Append any new entries of the src NotePairs list to our list. + /// Entries which already exist in the destination set are ignored. void appendNewOnly(const NotePairs *src); - /** - * Returns a comma separated list of notes with key 'noteKey'. - * Use findFirst instead when a unique kv-pair is needed. - */ - const char *find(const char *noteKey, const char *sep = ",") const; + /// \param resultNote a comma separated list of notes with key 'noteKey'. + /// \returns true if there are entries with the given 'noteKey'. + /// Use findFirst() instead when a unique kv-pair is needed. + bool find(SBuf &resultNote, const char *noteKey, const char *sep = ",") const; - /** - * Returns the first note value for this key or an empty string. - */ + /// \returns the first note value for this key or an empty string. const char *findFirst(const char *noteKey) const; - /** - * Adds a note key and value to the notes list. - * If the key name already exists in list, add the given value to its set - * of values. - */ + /// Adds a note key and value to the notes list. + /// If the key name already exists in the list, add the given value to its set + /// of values. + void add(const SBuf &key, const SBuf &value); void add(const char *key, const char *value); - /** - * Remove all notes with a given key. - */ + /// Remove all notes with a given key. If keyLen is not + /// provided, the key is assumed null-terminated. void remove(const char *key); + void remove(const SBuf &key); + + /// Adds a note key and values strList to the notes list. + /// If the key name already exists in the list, add the new values to its set + /// of values. + void addStrList(const SBuf &key, const SBuf &values, const CharacterSet &delimiters); - /** - * Adds a note key and values strList to the notes list. - * If the key name already exists in list, add the new values to its set - * of values. - */ - void addStrList(const char *key, const char *values); - - /** - * Return true if the key/value pair is already stored - */ - bool hasPair(const char *key, const char *value) const; - - /** - * Convert NotePairs list to a string consist of "Key: Value" - * entries separated by sep string. - */ + /// \returns true if the key/value pair is already stored + bool hasPair(const SBuf &key, const SBuf &value) const; + + /// Convert NotePairs list to a string consist of "Key: Value" + /// entries separated by sep string. const char *toString(const char *sep = "\r\n") const; - /** - * True if there are not entries in the list - */ + /// \returns true if there are not entries in the list bool empty() const {return entries.empty();} - std::vector entries; ///< The key/value pair entries + void clear() { entries.clear(); } + + /// If delimiters are provided, returns another Entries, converting each single multi-token + /// pair to multiple single-token pairs; returns existing entries otherwise. + const Entries &expandListEntries(const CharacterSet *delimiters) const; private: - NotePairs &operator = (NotePairs const &); // Not implemented - NotePairs(NotePairs const &); // Not implemented + Entries entries; ///< The key/value pair entries }; -class AccessLogEntry; -/** - * Keep in sync HttpRequest and the corresponding AccessLogEntry objects - */ -NotePairs &SyncNotes(AccessLogEntry &ale, HttpRequest &request); - -class ConnStateData; -/** - * Updates ConnStateData ids and HttpRequest notes from helpers received notes. - */ -void UpdateRequestNotes(ConnStateData *csd, HttpRequest &request, NotePairs const ¬es); #endif diff --git a/src/acl/Acl.cc b/src/acl/Acl.cc index 826dd230be..78111e352b 100644 --- a/src/acl/Acl.cc +++ b/src/acl/Acl.cc @@ -34,7 +34,8 @@ const ACLFlag ACLFlags::NoFlags[1] = {ACL_F_END}; const char *AclMatchedName = NULL; -ACLFlags::FlagsTokenizer::FlagsTokenizer(): tokPos(NULL) { } +ACLFlags::FlagsTokenizer::FlagsTokenizer() + : tokPos(nullptr), Parameter(nullptr) {} ACLFlag ACLFlags::FlagsTokenizer::nextFlag() @@ -50,19 +51,19 @@ ACLFlags::FlagsTokenizer::nextFlag() bool ACLFlags::FlagsTokenizer::hasParameter() const { - return tokPos && tokPos[0] && tokPos[1] == '=' && tokPos[2]; + return tokPos && tokPos[0] && !tokPos[1] && Parameter; } SBuf ACLFlags::FlagsTokenizer::getParameter() const { - return hasParameter() ? SBuf(&tokPos[2]) : SBuf(); + return hasParameter() ? SBuf(Parameter) : SBuf(); } bool ACLFlags::FlagsTokenizer::needNextToken() const { - return !tokPos || !tokPos[0] || !tokPos[1] || tokPos[1] == '='; + return !tokPos || !tokPos[0] || !tokPos[1]; } bool @@ -71,10 +72,17 @@ ACLFlags::FlagsTokenizer::nextToken() char *t = ConfigParser::PeekAtToken(); if (t == NULL || t[0] != '-' || !t[1]) return false; - (void)ConfigParser::NextQuotedToken(); - if (strcmp(t, "--") == 0) - return false; - tokPos = t + 1; + if (strchr(t, '=')) { + if(!ConfigParser::NextKvPair(tokPos, Parameter)) + abortFlags("Invalid formatting for flag '" << t << "'"); + assert(tokPos[0] == '-'); + tokPos++; + } else { + (void)ConfigParser::NextToken(); + if (!strcmp(t, "--")) + return false; + tokPos = t + 1; + } return true; } diff --git a/src/acl/Acl.h b/src/acl/Acl.h index e01ccbe77d..c837089ca4 100644 --- a/src/acl/Acl.h +++ b/src/acl/Acl.h @@ -93,6 +93,7 @@ public: bool nextToken(); char *tokPos; + char *Parameter; }; private: diff --git a/src/acl/Makefile.am b/src/acl/Makefile.am index df3a9c5338..51b6c4eab9 100644 --- a/src/acl/Makefile.am +++ b/src/acl/Makefile.am @@ -52,6 +52,12 @@ libacls_la_SOURCES = \ TimeData.h \ AllOf.cc \ AllOf.h \ + AnnotateClient.cc \ + AnnotateClient.h \ + AnnotateTransaction.cc \ + AnnotateTransaction.h \ + AnnotationData.cc \ + AnnotationData.h \ AnyOf.cc \ AnyOf.h \ Asn.cc \ diff --git a/src/acl/Note.cc b/src/acl/Note.cc index f1c8986bd3..a58ed9504a 100644 --- a/src/acl/Note.cc +++ b/src/acl/Note.cc @@ -20,7 +20,7 @@ int ACLNoteStrategy::match(ACLData * &data, ACLFilledChecklist *checklist, ACLFlags &flags) { if (const auto request = checklist->request) { - if (request->notes != NULL && matchNotes(data, request->notes.getRaw(), flags.delimiters())) + if (request->hasNotes() && matchNotes(data, request->notes().getRaw(), flags.delimiters())) return 1; #if USE_ADAPTATION const Adaptation::History::Pointer ah = request->adaptLogHistory(); @@ -34,24 +34,10 @@ ACLNoteStrategy::match(ACLData * &data, ACLFilledChecklist *checklist bool ACLNoteStrategy::matchNotes(ACLData *noteData, const NotePairs *note, const CharacterSet *delimiters) const { - for (auto &entry: note->entries) { - if (delimiters) { - NotePairs::Entry e(entry->name.termedBuf(), ""); - Parser::Tokenizer t(StringToSBuf(entry->value)); - SBuf s; - while (t.token(s, *delimiters)) { - e.value = s.c_str(); - if (noteData->match(&e)) - return true; - } - s = t.remaining(); - e.value = s.c_str(); - if (noteData->match(&e)) - return true; - } - if (noteData->match(entry)) + const NotePairs::Entries &entries = note->expandListEntries(delimiters); + for (auto e: entries) + if (noteData->match(e.getRaw())) return true; - } return false; } diff --git a/src/acl/NoteData.cc b/src/acl/NoteData.cc index f34ac5bf12..d7f1f41d85 100644 --- a/src/acl/NoteData.cc +++ b/src/acl/NoteData.cc @@ -27,19 +27,19 @@ ACLNoteData::~ACLNoteData() bool ACLNoteData::match(NotePairs::Entry *entry) { - if (entry->name.cmp(name.termedBuf()) != 0) + if (entry->name().cmp(name) != 0) return false; // name mismatch // a name-only note ACL matches any value; others require a values match return values->empty() || - values->match(entry->value.termedBuf()); + values->match(entry->value()); } SBufList ACLNoteData::dump() const { SBufList sl; - sl.push_back(StringToSBuf(name)); + sl.push_back(name); #if __cplusplus >= 201103L sl.splice(sl.end(), values->dump()); #else @@ -62,7 +62,7 @@ ACLNoteData::parse() bool ACLNoteData::empty() const { - return name.size() == 0; + return name.isEmpty(); } ACLData * diff --git a/src/acl/NoteData.h b/src/acl/NoteData.h index f38ea98dc8..d1d442dab9 100644 --- a/src/acl/NoteData.h +++ b/src/acl/NoteData.h @@ -30,7 +30,7 @@ public: virtual ACLData *clone() const; private: - String name; ///< Note name to check. It is always set + SBuf name; ///< Note name to check. It is always set ACLStringData *values; ///< if set, at least one value must match }; diff --git a/src/adaptation/Config.cc b/src/adaptation/Config.cc index 390add3cf1..536b5a3881 100644 --- a/src/adaptation/Config.cc +++ b/src/adaptation/Config.cc @@ -45,7 +45,7 @@ const char *metasBlacklist[] = { "Transfer-Complete", NULL }; -Notes Adaptation::Config::metaHeaders("ICAP header", metasBlacklist, true); +Notes Adaptation::Config::metaHeaders("ICAP header", metasBlacklist); bool Adaptation::Config::needHistory = false; Adaptation::ServiceConfig* diff --git a/src/adaptation/ecap/XactionRep.cc b/src/adaptation/ecap/XactionRep.cc index c330a9832b..5189f50e9a 100644 --- a/src/adaptation/ecap/XactionRep.cc +++ b/src/adaptation/ecap/XactionRep.cc @@ -188,10 +188,11 @@ Adaptation::Ecap::XactionRep::metaValue(const libecap::Name &name) const if (name.known()) { // must check to avoid empty names matching unset cfg typedef Notes::iterator ACAMLI; - for (ACAMLI i = Adaptation::Config::metaHeaders.begin(); i != Adaptation::Config::metaHeaders.end(); ++i) { - if (name == (*i)->key.termedBuf()) { - if (const char *value = (*i)->match(request, reply, al)) - return libecap::Area::FromTempString(value); + for (auto h: Adaptation::Config::metaHeaders) { + if (name == h->key().toStdString()) { + SBuf matched; + if (h->match(request, reply, al, matched)) + return libecap::Area::FromTempString(matched.toStdString()); else return libecap::Area(); } @@ -209,12 +210,11 @@ Adaptation::Ecap::XactionRep::visitEachMetaHeader(libecap::NamedValueVisitor &vi Must(request); HttpReply *reply = dynamic_cast(theVirginRep.raw().header); - typedef Notes::iterator ACAMLI; - for (ACAMLI i = Adaptation::Config::metaHeaders.begin(); i != Adaptation::Config::metaHeaders.end(); ++i) { - const char *v = (*i)->match(request, reply, al); - if (v) { - const libecap::Name name((*i)->key.termedBuf()); - const libecap::Area value = libecap::Area::FromTempString(v); + for (auto h: Adaptation::Config::metaHeaders) { + SBuf matched; + if (h->match(request, reply, al, matched)) { + const libecap::Name name(h->key().toStdString()); + const libecap::Area value = libecap::Area::FromTempString(matched.toStdString()); visitor.visit(name, value); } } @@ -238,14 +238,13 @@ Adaptation::Ecap::XactionRep::start() if (ah != NULL) { // retrying=false because ecap never retries transactions adaptHistoryId = ah->recordXactStart(service().cfg().key, current_time, false); - typedef Notes::iterator ACAMLI; - for (ACAMLI i = Adaptation::Config::metaHeaders.begin(); i != Adaptation::Config::metaHeaders.end(); ++i) { - const char *v = (*i)->match(request, reply, al); - if (v) { + SBuf matched; + for (auto h: Adaptation::Config::metaHeaders) { + if (h->match(request, reply, al, matched)) { if (ah->metaHeaders == NULL) ah->metaHeaders = new NotePairs(); - if (!ah->metaHeaders->hasPair((*i)->key.termedBuf(), v)) - ah->metaHeaders->add((*i)->key.termedBuf(), v); + if (!ah->metaHeaders->hasPair(h->key(), matched)) + ah->metaHeaders->add(h->key(), matched); } } } diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index 5e4a8358ae..e7c4fe8bbf 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -1486,22 +1486,25 @@ void Adaptation::Icap::ModXact::makeRequestHeaders(MemBuf &buf) makeUsernameHeader(request, buf); // Adaptation::Config::metaHeaders - typedef Notes::iterator ACAMLI; - for (ACAMLI i = Adaptation::Config::metaHeaders.begin(); i != Adaptation::Config::metaHeaders.end(); ++i) { + for (auto h: Adaptation::Config::metaHeaders) { HttpRequest *r = virgin.cause ? virgin.cause : dynamic_cast(virgin.header); Must(r); HttpReply *reply = dynamic_cast(virgin.header); - if (const char *value = (*i)->match(r, reply, alMaster)) { - buf.appendf("%s: %s\r\n", (*i)->key.termedBuf(), value); + SBuf matched; + if (h->match(r, reply, alMaster, matched)) { + buf.append(h->key().rawContent(), h->key().length()); + buf.append(": ", 2); + buf.append(matched.rawContent(), matched.length()); + buf.append("\r\n", 2); Adaptation::History::Pointer ah = request->adaptHistory(false); if (ah != NULL) { if (ah->metaHeaders == NULL) ah->metaHeaders = new NotePairs; - if (!ah->metaHeaders->hasPair((*i)->key.termedBuf(), value)) - ah->metaHeaders->add((*i)->key.termedBuf(), value); + if (!ah->metaHeaders->hasPair(h->key(), matched)) + ah->metaHeaders->add(h->key(), matched); } } } diff --git a/src/auth/digest/UserRequest.cc b/src/auth/digest/UserRequest.cc index 5d98c8150c..1411195639 100644 --- a/src/auth/digest/UserRequest.cc +++ b/src/auth/digest/UserRequest.cc @@ -355,8 +355,7 @@ Auth::Digest::UserRequest::HandleReply(void *data, const Helper::Reply &reply) Auth::Digest::User *digest_user = dynamic_cast(auth_user_request->user().getRaw()); assert(digest_user != NULL); - const char *ha1Note = reply.notes.findFirst("ha1"); - if (ha1Note != NULL) { + if (const char *ha1Note = reply.notes.findFirst("ha1")) { CvtBin(ha1Note, digest_user->HA1); digest_user->HA1created = 1; } else { @@ -381,9 +380,9 @@ Auth::Digest::UserRequest::HandleReply(void *data, const Helper::Reply &reply) digest_request->user()->credentials(Auth::Failed); digest_request->flags.invalid_password = true; - const char *msgNote = reply.notes.find("message"); - if (msgNote != NULL) { - digest_request->setDenyMessage(msgNote); + SBuf msgNote; + if (reply.notes.find(msgNote, "message")) { + digest_request->setDenyMessage(msgNote.c_str()); } else if (reply.other().hasContent()) { // old helpers did send ERR result but a bare message string instead of message= key name. digest_request->setDenyMessage(reply.other().content()); diff --git a/src/auth/negotiate/UserRequest.cc b/src/auth/negotiate/UserRequest.cc index f7a2b30555..0f675080ee 100644 --- a/src/auth/negotiate/UserRequest.cc +++ b/src/auth/negotiate/UserRequest.cc @@ -357,12 +357,12 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const Helper::Reply &reply break; case Helper::Error: { - const char *messageNote = reply.notes.find("message"); const char *tokenNote = reply.notes.findFirst("token"); + SBuf messageNote; /* authentication failure (wrong password, etc.) */ - if (messageNote != NULL) - auth_user_request->denyMessage(messageNote); + if (reply.notes.find(messageNote, "message")) + auth_user_request->denyMessage(messageNote.c_str()); else auth_user_request->denyMessage("Negotiate Authentication denied with no reason given"); auth_user_request->user()->credentials(Auth::Failed); @@ -385,11 +385,11 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const Helper::Reply &reply * Authenticate Negotiate start. * If after a KK deny the user's request w/ 407 and mark the helper as * Needing YR. */ - const char *errNote = reply.notes.find("message"); + SBuf errNote; if (reply.result == Helper::Unknown) auth_user_request->denyMessage("Internal Error"); - else if (errNote != NULL) - auth_user_request->denyMessage(errNote); + else if (reply.notes.find(errNote, "message")) + auth_user_request->denyMessage(errNote.c_str()); else auth_user_request->denyMessage("Negotiate Authentication failed with no reason given"); auth_user_request->user()->credentials(Auth::Failed); diff --git a/src/auth/ntlm/UserRequest.cc b/src/auth/ntlm/UserRequest.cc index a25f8a1a52..4670a11908 100644 --- a/src/auth/ntlm/UserRequest.cc +++ b/src/auth/ntlm/UserRequest.cc @@ -353,9 +353,9 @@ Auth::Ntlm::UserRequest::HandleReply(void *data, const Helper::Reply &reply) case Helper::Error: { /* authentication failure (wrong password, etc.) */ - const char *errNote = reply.notes.find("message"); - if (errNote != NULL) - auth_user_request->denyMessage(errNote); + SBuf errNote; + if (reply.notes.find(errNote, "message")) + auth_user_request->denyMessage(errNote.c_str()); else auth_user_request->denyMessage("NTLM Authentication denied with no reason given"); auth_user_request->user()->credentials(Auth::Failed); @@ -376,11 +376,11 @@ Auth::Ntlm::UserRequest::HandleReply(void *data, const Helper::Reply &reply) * Authenticate NTLM start. * If after a KK deny the user's request w/ 407 and mark the helper as * Needing YR. */ - const char *errNote = reply.notes.find("message"); + SBuf errNote; if (reply.result == Helper::Unknown) auth_user_request->denyMessage("Internal Error"); - else if (errNote != NULL) - auth_user_request->denyMessage(errNote); + else if (reply.notes.find(errNote, "message")) + auth_user_request->denyMessage(errNote.c_str()); else auth_user_request->denyMessage("NTLM Authentication failed with no reason given"); auth_user_request->user()->credentials(Auth::Failed); diff --git a/src/cf.data.pre b/src/cf.data.pre index 7be7ebd603..4200d338fa 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -1217,6 +1217,78 @@ DOC_START # Annotation sources include note and adaptation_meta directives # as well as helper and eCAP responses. + acl aclname annotate_transaction [-m[=delimiters]] key=value ... # [fast] + acl aclname annotate_transaction [-m[=delimiters]] key+=value ... # [fast] + # Always matches. [fast] + # Used for its side effect: This ACL immediately adds a + # key=value annotation to the current master transaction. + # The added annotation can then be tested using note ACL and + # logged (or sent to helpers) using %note format code. + # + # Annotations can be specified using replacement and addition + # formats. The key=value form replaces old same-key annotation + # value(s). The key+=value form appends a new value to the old + # same-key annotation. Both forms create a new key=value + # annotation if no same-key annotation exists already. If + # -m flag is used, then the value is interpreted as a list + # and the annotation will contain key=token pair(s) instead of the + # whole key=value pair. + # + # This ACL is especially useful for recording complex multi-step + # ACL-driven decisions. For example, the following configuration + # avoids logging transactions accepted after aclX matched: + # + # # First, mark transactions accepted after aclX matched + # acl markSpecial annotate_transaction special=true + # http_access allow acl001 + # ... + # http_access deny acl100 + # http_access allow aclX markSpecial + # + # # Second, do not log marked transactions: + # acl markedSpecial note special true + # access_log ... deny markedSpecial + # + # # Note that the following would not have worked because aclX + # # alone does not determine whether the transaction was allowed: + # access_log ... deny aclX # Wrong! + # + # Warning: This ACL annotates the transaction even when negated + # and even if subsequent ACLs fail to match. For example, the + # following three rules will have exactly the same effect as far + # as annotations set by the "mark" ACL are concerned: + # + # some_directive acl1 ... mark # rule matches if mark is reached + # some_directive acl1 ... !mark # rule never matches + # some_directive acl1 ... mark !all # rule never matches + + acl aclname annotate_client [-m[=delimiters]] key=value ... # [fast] + acl aclname annotate_client [-m[=delimiters]] key+=value ... # [fast] + # + # Always matches. Used for its side effect: This ACL immediately + # adds a key=value annotation to the current client-to-Squid + # connection. Connection annotations are propagated to the current + # and all future master transactions on the annotated connection. + # See the annotate_transaction ACL for details. + # + # For example, the following configuration avoids rewriting URLs + # of transactions bumped by SslBump: + # + # # First, mark bumped connections: + # acl markBumped annotate_client bumped=true + # ssl_bump peek acl1 + # ssl_bump stare acl2 + # ssl_bump bump acl3 markBumped + # ssl_bump splice all + # + # # Second, do not send marked transactions to the redirector: + # acl markedBumped note bumped true + # url_rewrite_access deny markedBumped + # + # # Note that the following would not have worked because acl3 alone + # # does not determine whether the connection is going to be bumped: + # url_rewrite_access deny acl3 # Wrong! + acl aclname adaptation_service service ... # Matches the name of any icap_service, ecap_service, # adaptation_service_set, or adaptation_service_chain that Squid diff --git a/src/client_side.cc b/src/client_side.cc index ef6b59d9d7..26d5516b66 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -430,15 +430,15 @@ ClientHttpRequest::logRequest() /* Add notes (if we have a request to annotate) */ if (request) { - // The al->notes and request->notes must point to the same object. - (void)SyncNotes(*al, *request); - for (auto i = Config.notes.begin(); i != Config.notes.end(); ++i) { - if (const char *value = (*i)->match(request, al->reply, NULL)) { - NotePairs ¬es = SyncNotes(*al, *request); - notes.add((*i)->key.termedBuf(), value); - debugs(33, 3, (*i)->key.termedBuf() << " " << value); + SBuf matched; + for (auto h: Config.notes) { + if (h->match(request, al->reply, NULL, matched)) { + request->notes()->add(h->key(), matched); + debugs(33, 3, h->key() << " " << matched); } } + // The al->notes and request->notes must point to the same object. + al->syncNotes(request); } ACLFilledChecklist checklist(NULL, request, NULL); @@ -4122,3 +4122,11 @@ ConnStateData::mayTunnelUnsupportedProto() ; } +NotePairs::Pointer +ConnStateData::notes() +{ + if (!theNotes) + theNotes = new NotePairs; + return theNotes; +} + diff --git a/src/client_side.h b/src/client_side.h index 74797884d8..8b0c1dd035 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -259,10 +259,6 @@ public: bool switchedToHttps() const { return false; } #endif - /* clt_conn_tag=tag annotation access */ - const SBuf &connectionTag() const { return connectionTag_; } - void connectionTag(const char *aTag) { connectionTag_ = aTag; } - /// handle a control message received by context from a peer and call back virtual bool writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call) = 0; @@ -299,6 +295,11 @@ public: virtual void startShutdown(); virtual void endingShutdown(); + /// \returns existing non-empty connection annotations, + /// creates and returns empty annotations otherwise + NotePairs::Pointer notes(); + bool hasNotes() const { return bool(theNotes) && !theNotes->empty(); } + protected: void startDechunkingRequest(); void finishDechunkingRequest(bool withSuccess); @@ -376,8 +377,10 @@ private: const char *stoppedSending_; /// the reason why we no longer read the request or nil const char *stoppedReceiving_; - - SBuf connectionTag_; ///< clt_conn_tag=Tag annotation for client connection + /// Connection annotations, clt_conn_tag and other tags are stored here. + /// If set, are propagated to the current and all future master transactions + /// on the connection. + NotePairs::Pointer theNotes; }; void setLogUri(ClientHttpRequest * http, char const *uri, bool cleanUrl = false); diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 81e193fd48..b05e1c2e96 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -894,7 +894,7 @@ void ClientRequestContext::clientRedirectStart() { debugs(33, 5, HERE << "'" << http->uri << "'"); - (void)SyncNotes(*http->al, *http->request); + http->al->syncNotes(http->request); if (Config.accessList.redirector) { acl_checklist = clientAclChecklistCreate(Config.accessList.redirector, http); acl_checklist->nonBlockingCheck(clientRedirectAccessCheckDone, this); @@ -1206,8 +1206,8 @@ ClientRequestContext::clientRedirectDone(const Helper::Reply &reply) // Put helper response Notes into the transaction state record (ALE) eventually // do it early to ensure that no matter what the outcome the notes are present. - if (http->al != NULL) - (void)SyncNotes(*http->al, *old_request); + if (http->al) + http->al->syncNotes(old_request); UpdateRequestNotes(http->getConn(), *old_request, reply.notes); @@ -1329,8 +1329,8 @@ ClientRequestContext::clientStoreIdDone(const Helper::Reply &reply) // Put helper response Notes into the transaction state record (ALE) eventually // do it early to ensure that no matter what the outcome the notes are present. - if (http->al != NULL) - (void)SyncNotes(*http->al, *old_request); + if (http->al) + http->al->syncNotes(old_request); UpdateRequestNotes(http->getConn(), *old_request, reply.notes); @@ -1677,17 +1677,18 @@ ClientHttpRequest::doCallouts() { assert(calloutContext); + auto &ale = calloutContext->http->al; /*Save the original request for logging purposes*/ - if (!calloutContext->http->al->request) { - calloutContext->http->al->request = request; - HTTPMSGLOCK(calloutContext->http->al->request); + if (!ale->request) { + ale->request = request; + HTTPMSGLOCK(ale->request); - NotePairs ¬es = SyncNotes(*calloutContext->http->al, *calloutContext->http->request); // Make the previously set client connection ID available as annotation. if (ConnStateData *csd = calloutContext->http->getConn()) { - if (!csd->connectionTag().isEmpty()) - notes.add("clt_conn_tag", SBuf(csd->connectionTag()).c_str()); + if (!csd->notes()->empty()) + calloutContext->http->request->notes()->appendNewOnly(csd->notes().getRaw()); } + ale->syncNotes(calloutContext->http->request); } if (!calloutContext->error) { diff --git a/src/format/Format.cc b/src/format/Format.cc index 7c18b0b855..8cf59dc37d 100644 --- a/src/format/Format.cc +++ b/src/format/Format.cc @@ -1390,18 +1390,19 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS tmp[1] = '\0'; if (fmt->data.header.header && *fmt->data.header.header) { const char *separator = tmp; + static SBuf note; #if USE_ADAPTATION Adaptation::History::Pointer ah = al->request ? al->request->adaptHistory() : Adaptation::History::Pointer(); if (ah != NULL && ah->metaHeaders != NULL) { - if (const char *meta = ah->metaHeaders->find(fmt->data.header.header, separator)) - sb.append(meta); + if (ah->metaHeaders->find(note, fmt->data.header.header, separator)) + sb.append(note.c_str()); } #endif if (al->notes != NULL) { - if (const char *note = al->notes->find(fmt->data.header.header, separator)) { + if (al->notes->find(note, fmt->data.header.header, separator)) { if (sb.size()) sb.append(separator); - sb.append(note); + sb.append(note.c_str()); } } out = sb.termedBuf(); diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc index 9066ff33a4..4eba62af9d 100644 --- a/src/servers/Http1Server.cc +++ b/src/servers/Http1Server.cc @@ -187,6 +187,13 @@ Http::One::Server::buildHttpRequest(Http::StreamPointer &context) request->header.putStr(Http::HOST, tmp.c_str()); } + // TODO: We fill request notes here until we find a way to verify whether + // no ACL checking is performed before ClientHttpRequest::doCallouts(). + if (hasNotes()) { + assert(!request->hasNotes()); + request->notes()->append(notes().getRaw()); + } + http->request = request.getRaw(); HTTPMSGLOCK(http->request); diff --git a/src/tests/stub_HttpRequest.cc b/src/tests/stub_HttpRequest.cc index 7670dc7d1c..1afc08c247 100644 --- a/src/tests/stub_HttpRequest.cc +++ b/src/tests/stub_HttpRequest.cc @@ -56,4 +56,5 @@ void HttpRequest::packFirstLineInto(Packable *, bool) const STUB bool HttpRequest::sanityCheckStartLine(const char *, const size_t, Http::StatusCode *) STUB_RETVAL(false) void HttpRequest::hdrCacheInit() STUB bool HttpRequest::inheritProperties(const HttpMsg *) STUB_RETVAL(false) +NotePairs::Pointer HttpRequest::notes() STUB_RETVAL(NotePairs::Pointer()) diff --git a/src/tests/stub_client_side.cc b/src/tests/stub_client_side.cc index 0dc49d9f5f..c83cddb465 100644 --- a/src/tests/stub_client_side.cc +++ b/src/tests/stub_client_side.cc @@ -39,6 +39,7 @@ void ConnStateData::connStateClosed(const CommCloseCbParams &) STUB void ConnStateData::requestTimeout(const CommTimeoutCbParams &) STUB void ConnStateData::swanSong() STUB void ConnStateData::quitAfterError(HttpRequest *) STUB +NotePairs::Pointer ConnStateData::notes() STUB_RETVAL(NotePairs::Pointer()) #if USE_OPENSSL void ConnStateData::httpsPeeked(Comm::ConnectionPointer) STUB void ConnStateData::getSslContextStart() STUB