HttpRequest::helperNotes to NotePairs This patch try to fix current current Notes interface and usage. The changes done having in mind that we need: 1) to add multiple notes with the same key 2) to support 3 different note types: adaptation meta headers, helper notes and custom notes added by the system administrator 3) to log notes using the %note formating code 4) to use the %note formating code everywhere the formating API is used. For example use the %note with the request_header_add configuration parameter. 5) to use notes with ACLs. Details: - The NotePairs class is not a kid of HttpHeader class anymore. It is implemented from scratch to cover Helper/adaptation and custom notes needs. * The new class stores key:value pairs in list. It allow multiple entries with the same key. * Includes a find method which return a coma separated list of values for a given key - The HttpRequest::helperNotes is now a Refcount of a HttpPairs object - The HelperReply::notes is now a HttpPairs object - The AccessLogEntry::notes now is a RefCount of a HttpPairs object, and stores only the custom notes add by the "note" configuration parameter - Add the AccessLogEntry::helperNotes which is a RefCount of a HttpPairs object to store notes added by helpers. Now the notes added by adaptation or helpers are accessible to format/* code imediatelly after added. Before this patch are accessible only for logging. Future work: - Posible merge AccessLogEntry::notes and AccessLogEntry::helperNotes - Performance fixes === modified file 'src/AccessLogEntry.h' --- src/AccessLogEntry.h 2013-03-17 12:19:16 +0000 +++ src/AccessLogEntry.h 2013-04-16 15:05:32 +0000 @@ -215,43 +215,45 @@ char *last_meta; } adapt; #endif // Why is this a sub-class and not a set of real "private:" fields? // It looks like its duplicating HTTPRequestMethod anyway! // TODO: shuffle this to the relevant protocol section OR replace with request->method class Private { public: Private() : method_str(NULL) {} const char *method_str; } _private; HierarchyLogEntry hier; HttpReply *reply; HttpRequest *request; //< virgin HTTP request HttpRequest *adapted_request; //< HTTP request after adaptation and redirection - /// key:value pairs set by note and adaptation_meta directives - /// plus key=value pairs returned from URL rewrite/redirect helper - NotePairs notes; + // TODO: merge configNotes and helperNotes + /// key:value pairs set by note. + NotePairs::Pointer configNotes; + /// key=value pairs returned from URL rewrite/redirect helper + NotePairs::Pointer helperNotes; #if ICAP_CLIENT /** \brief This subclass holds log info for ICAP part of request * \todo Inner class declarations should be moved outside */ class IcapLogEntry { public: IcapLogEntry() : reqMethod(Adaptation::methodNone), bytesSent(0), bytesRead(0), bodyBytesRead(-1), request(NULL), reply(NULL), outcome(Adaptation::Icap::xoUnknown), trTime(0), ioTime(0), resStatus(Http::scNone), processingTime(0) {} Ip::Address hostAddr; ///< ICAP server IP address String serviceName; ///< ICAP service name String reqUri; ///< ICAP Request-URI Adaptation::Icap::ICAP::Method reqMethod; ///< ICAP request method int64_t bytesSent; ///< number of bytes sent to ICAP server so far int64_t bytesRead; ///< number of bytes read from ICAP server so far /** === modified file 'src/HelperReply.cc' --- src/HelperReply.cc 2013-03-20 04:48:17 +0000 +++ src/HelperReply.cc 2013-04-16 14:45:54 +0000 @@ -128,76 +128,71 @@ } void HelperReply::parseResponseKeys() { // parse a "key=value" pair off the 'other()' buffer. while (other().hasContent()) { char *p = modifiableOther().content(); while (*p && *p != '=' && *p != ' ') ++p; if (*p != '=') return; // done. Not a key. // whitespace between key and value is prohibited. // workaround strwordtok() which skips whitespace prefix. if (xisspace(*(p+1))) return; // done. Not a key. *p = '\0'; ++p; - const String key(other().content()); + const char *key = other().content(); // the value may be a quoted string or a token const bool urlDecode = (*p != '"'); // check before moving p. char *v = strwordtok(NULL, &p); if (v != NULL && urlDecode && (p-v) > 2) // 1-octet %-escaped requires 3 bytes rfc1738_unescape(v); - const String value(v?v:""); // value can be empty, but must not be NULL - notes.add(key, value); + notes.add(key, v ? v : ""); // value can be empty, but must not be NULL modifiableOther().consume(p - other().content()); modifiableOther().consumeWhitespacePrefix(); } } std::ostream & operator <<(std::ostream &os, const HelperReply &r) { os << "{result="; switch (r.result) { case HelperReply::Okay: os << "OK"; break; case HelperReply::Error: os << "ERR"; break; case HelperReply::BrokenHelper: os << "BH"; break; case HelperReply::TT: os << "TT"; break; case HelperReply::Unknown: os << "Unknown"; break; } // dump the helper key=pair "notes" list - if (r.notes.notes.size() > 0) { + if (!r.notes.empty()) { os << ", notes={"; - for (Notes::NotesList::const_iterator m = r.notes.notes.begin(); m != r.notes.notes.end(); ++m) { - for (Note::Values::iterator v = (*m)->values.begin(); v != (*m)->values.end(); ++v) { - os << ',' << (*m)->key << '=' << ConfigParser::QuoteString((*v)->value); - } - } + os << r.notes.toString("; "); os << "}"; } if (r.other().hasContent()) os << ", other: \"" << r.other().content() << '\"'; os << '}'; return os; } === modified file 'src/HelperReply.h' --- src/HelperReply.h 2012-11-27 21:19:46 +0000 +++ src/HelperReply.h 2013-04-05 10:42:55 +0000 @@ -48,35 +48,35 @@ * token are URL-decoded. * quoted-string are \-escape decoded and the quotes are stripped. */ // XXX: buf should be const but we may need strwordtok() and rfc1738_unescape() void parse(char *buf, size_t len); public: /// The helper response 'result' field. enum Result_ { Unknown, // no result code received, or unknown result code Okay, // "OK" indicating success/positive result Error, // "ERR" indicating success/negative result BrokenHelper, // "BH" indicating failure due to helper internal problems. // result codes for backward compatibility with NTLM/Negotiate // TODO: migrate to a variant of the above results with kv-pair parameters TT } result; // list of key=value pairs the helper produced - Notes notes; + NotePairs notes; /// for stateful replies the responding helper 'server' needs to be preserved across callbacks CbcPointer whichServer; private: void parseResponseKeys(); /// the remainder of the line MemBuf other_; }; std::ostream &operator <<(std::ostream &os, const HelperReply &r); #endif /* _SQUID_SRC_HELPERREPLY_H */ === modified file 'src/HttpHeader.h' --- src/HttpHeader.h 2013-03-20 22:39:26 +0000 +++ src/HttpHeader.h 2013-04-04 15:47:57 +0000 @@ -159,41 +159,40 @@ ftDate_1123, ftETag, ftPCc, ftPContRange, ftPRange, ftPSc, ftDate_1123_or_ETag } field_type; /** Possible owners of http header */ typedef enum { hoNone =0, #if USE_HTCP hoHtcpReply, #endif hoRequest, hoReply, #if USE_SSL hoErrorDetail, #endif - hoNote, hoEnd } http_hdr_owner_type; // currently a POD class HttpHeaderFieldAttrs { public: const char *name; http_hdr_type id; field_type type; }; /** Iteration for headers; use HttpHeaderPos as opaque type, do not interpret */ typedef ssize_t HttpHeaderPos; /* use this and only this to initialize HttpHeaderPos */ #define HttpHeaderInitPos (-1) class HttpHeaderEntry { === modified file 'src/HttpRequest.cc' --- src/HttpRequest.cc 2013-03-17 12:19:16 +0000 +++ src/HttpRequest.cc 2013-04-16 14:48:35 +0000 @@ -151,44 +151,41 @@ safe_free(canonical); safe_free(vary_headers); urlpath.clean(); header.clean(); if (cache_control) { delete cache_control; cache_control = NULL; } if (range) { delete range; range = NULL; } myportname.clean(); - if (helperNotes) { - delete helperNotes; - helperNotes = NULL; - } + helperNotes = NULL; tag.clean(); #if USE_AUTH extacl_user.clean(); extacl_passwd.clean(); #endif extacl_log.clean(); extacl_message.clean(); #if USE_ADAPTATION adaptHistory_ = NULL; #endif #if ICAP_CLIENT icapHistory_ = NULL; #endif } void HttpRequest::reset() @@ -214,44 +211,41 @@ copy->host_addr = host_addr; copy->port = port; // urlPath handled in ctor copy->canonical = canonical ? xstrdup(canonical) : NULL; // range handled in hdrCacheInit() copy->ims = ims; copy->imslen = imslen; copy->hier = hier; // Is it safe to copy? Should we? copy->errType = errType; // XXX: what to do with copy->peer_login? copy->lastmod = lastmod; copy->vary_headers = vary_headers ? xstrdup(vary_headers) : NULL; // XXX: what to do with copy->peer_domain? copy->myportname = myportname; - if (helperNotes) { - copy->helperNotes = new Notes; - copy->helperNotes->notes = helperNotes->notes; - } + copy->helperNotes = helperNotes; copy->tag = tag; #if USE_AUTH copy->extacl_user = extacl_user; copy->extacl_passwd = extacl_passwd; #endif copy->extacl_log = extacl_log; copy->extacl_message = extacl_message; assert(copy->inheritProperties(this)); return copy; } bool HttpRequest::inheritProperties(const HttpMsg *aMsg) { const HttpRequest* aReq = dynamic_cast(aMsg); if (!aReq) return false; === modified file 'src/HttpRequest.h' --- src/HttpRequest.h 2013-03-16 04:57:43 +0000 +++ src/HttpRequest.h 2013-04-05 12:50:59 +0000 @@ -186,41 +186,41 @@ HierarchyLogEntry hier; int dnsWait; ///< sum of DNS lookup delays in milliseconds, for %dt err_type errType; int errDetail; ///< errType-specific detail about the transaction error char *peer_login; /* Configured peer login:password */ char *peer_host; /* Selected peer host*/ time_t lastmod; /* Used on refreshes */ const char *vary_headers; /* Used when varying entities are detected. Changes how the store key is calculated */ char *peer_domain; /* Configured peer forceddomain */ String myportname; // Internal tag name= value from port this requests arrived in. - Notes *helperNotes; ///< collection of meta notes associated with this request by helper lookups. + NotePairs::Pointer helperNotes; ///< collection of meta notes associated with this request by helper lookups. String tag; /* Internal tag for this request */ String extacl_user; /* User name returned by extacl lookup */ String extacl_passwd; /* Password returned by extacl lookup */ String extacl_log; /* String to be used for access.log purposes */ String extacl_message; /* String to be used for error page purposes */ #if FOLLOW_X_FORWARDED_FOR String x_forwarded_for_iterator; /* XXX a list of IP addresses */ #endif /* FOLLOW_X_FORWARDED_FOR */ public: bool multipartRangeRequest() const; bool parseFirstLine(const char *start, const char *end); === modified file 'src/Notes.cc' --- src/Notes.cc 2013-02-12 11:34:35 +0000 +++ src/Notes.cc 2013-04-12 09:42:17 +0000 @@ -19,131 +19,92 @@ * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. * */ #include "squid.h" #include "globals.h" #include "acl/FilledChecklist.h" #include "acl/Gadgets.h" #include "ConfigParser.h" #include "HttpRequest.h" #include "HttpReply.h" #include "SquidConfig.h" #include "Store.h" +#include "StrList.h" #include #include Note::Value::~Value() { aclDestroyAclList(&aclList); } Note::Value::Pointer Note::addValue(const String &value) { Value::Pointer v = new Value(value); values.push_back(v); return v; } const char * Note::match(HttpRequest *request, HttpReply *reply) { typedef Values::iterator VLI; ACLFilledChecklist ch(NULL, request, NULL); 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); if (ret == ACCESS_ALLOWED) return (*i)->value.termedBuf(); } return NULL; } Note::Pointer -Notes::find(const String ¬eKey) const +Notes::add(const String ¬eKey) { - typedef Notes::NotesList::const_iterator AMLI; + typedef Notes::NotesList::iterator AMLI; for (AMLI i = notes.begin(); i != notes.end(); ++i) { if ((*i)->key == noteKey) return (*i); } - return Note::Pointer(); -} - -void -Notes::add(const String ¬eKey, const String ¬eValue) -{ - Note::Pointer key = add(noteKey); - key->addValue(noteValue); -} - -Note::Pointer -Notes::add(const String ¬eKey) -{ - Note::Pointer note = find(noteKey); - if (note == NULL) { - note = new Note(noteKey); - notes.push_back(note); - } + Note::Pointer note = new Note(noteKey); + notes.push_back(note); return note; } -void -Notes::add(const Notes &src) -{ - typedef Notes::NotesList::const_iterator AMLI; - typedef Note::Values::iterator VLI; - - for (AMLI i = src.notes.begin(); i != src.notes.end(); ++i) { - - // ensure we have a key by that name to fill out values for... - // NP: not sharing pointers at the key level since merging other helpers - // details later would affect this src objects keys, which is a bad idea. - Note::Pointer ourKey = add((*i)->key); - - // known key names, merge the values lists... - for (VLI v = (*i)->values.begin(); v != (*i)->values.end(); ++v ) { - // 2012-11-29: values are read-only and Pointer can safely be shared - // for now we share pointers to save memory and gain speed. - // If that ever ceases to be true, convert this to a full copy. - ourKey->values.push_back(*v); - // TODO: prune/skip duplicates ? - } - } -} - Note::Pointer Notes::parse(ConfigParser &parser) { String key, value; ConfigParser::ParseString(&key); ConfigParser::ParseQuotedString(&value); Note::Pointer note = add(key); Note::Value::Pointer noteValue = note->addValue(value); aclParseAclList(parser, ¬eValue->aclList); 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(), descr ? descr : ""); } } } @@ -153,20 +114,97 @@ 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"); } } } void Notes::clean() { notes.clean(); } + +const char * +NotePairs::find(const char *noteKey) const +{ + static String value; + value.clean(); + for (Vector::const_iterator i = entries.begin(); i != entries.end(); ++i) { + if ((*i)->name.cmp(noteKey) == 0) { + if (value.size()) + value.append(", "); + value.append(ConfigParser::QuoteString((*i)->value)); + } + } + return value.size() ? value.termedBuf() : NULL; +} + +const char * +NotePairs::toString(const char *sep) const +{ + static String value; + value.clean(); + for (Vector::const_iterator i = entries.begin(); i != entries.end(); ++i) { + value.append((*i)->name); + value.append(": "); + value.append(ConfigParser::QuoteString((*i)->value)); + value.append(sep); + } + return value.size() ? value.termedBuf() : NULL; +} + +const char * +NotePairs::findFirst(const char *noteKey) const +{ + for (Vector::const_iterator i = entries.begin(); i != entries.end(); ++i) { + if ((*i)->name.cmp(noteKey) == 0) + return (*i)->value.termedBuf(); + } + return NULL; +} + +void +NotePairs::add(const char *key, const char *note) +{ + entries.push_back(new NotePairs::Entry(key, note)); +} + +void +NotePairs::addStrList(const char *key, const char *values) +{ + 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())); + } +} + +bool +NotePairs::hasPair(const char *key, const char *value) const +{ + for (Vector::const_iterator i = entries.begin(); i != entries.end(); ++i) { + if ((*i)->name.cmp(key) == 0 || (*i)->value.cmp(value) == 0) + return true; + } + return false; +} + +void +NotePairs::append(const NotePairs *src) +{ + for (Vector::const_iterator i = src->entries.begin(); i != src->entries.end(); ++i) { + entries.push_back(new NotePairs::Entry((*i)->name.termedBuf(), (*i)->value.termedBuf())); + } +} === modified file 'src/Notes.h' --- src/Notes.h 2012-11-30 11:08:47 +0000 +++ src/Notes.h 2013-04-05 09:12:23 +0000 @@ -1,147 +1,177 @@ #ifndef SQUID_NOTES_H #define SQUID_NOTES_H -#include "HttpHeader.h" -#include "HttpHeaderTools.h" +#include "Array.h" +#include "base/RefCount.h" +#include "CbDataList.h" +#include "MemPool.h" +#include "SquidString.h" #include "typedefs.h" #if HAVE_STRING #include #endif class HttpRequest; class HttpReply; +class ACLList; /** - * Used to store notes. The notes are custom key:value pairs - * ICAP request headers or ECAP options used to pass + * Used to store a note configuration. The notes are custom key:value + * pairs ICAP request headers or ECAP options used to pass * custom transaction-state related meta information to squid - * internal subsystems or to addaptation services. + * internal subsystems or to adaptation services. */ class Note: public RefCountable { public: typedef RefCount Pointer; /// Stores a value for the note. class Value: public RefCountable { public: typedef RefCount Pointer; String value; ///< a note value ACLList *aclList; ///< The access list used to determine if this value is valid for a request explicit Value(const String &aVal) : value(aVal), aclList(NULL) {} ~Value(); }; typedef Vector Values; explicit Note(const String &aKey): key(aKey) {} /** * Adds a value to the note and returns a pointer to the * related Value object. */ Value::Pointer addValue(const String &value); /** * 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. */ const char *match(HttpRequest *request, HttpReply *reply); - /** - * Returns the first value for this key or an empty string. - */ - const char *firstValue() const { return (values.size()>0&&values[0]->value.defined()?values[0]->value.termedBuf():""); } - String key; ///< The note key Values values; ///< The possible values list for the note }; class ConfigParser; /** - * Used to store a notes list. + * Used to store a notes configuration list. */ class Notes { public: typedef 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): descr(aDescr), blacklisted(metasBlacklist) {} Notes(): descr(NULL), blacklisted(NULL) {} ~Notes() { notes.clean(); } /** * Parse 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. */ void dump(StoreEntry *entry, const char *name); void clean(); /// clean the notes list /// 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(); } - /** - * 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. - */ - void add(const String ¬eKey, const String ¬eValue); - - /** - * Adds a set of notes from another notes list to this set. - * Creating entries for any new keys needed. - * If the key name already exists in list, add the given value to its set of values. - * - * WARNING: - * The list entries are all of shared Pointer type. Altering the src object(s) after - * using this function will update both Notes lists. Likewise, altering this - * destination NotesList will affect any relevant copies of src still in use. - */ - void add(const Notes &src); - - /** - * Returns a pointer to an existing Note with given key name or nil. - */ - Note::Pointer find(const String ¬eKey) const; - 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 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); }; -class NotePairs : public HttpHeader +/** + * Used to store list of notes + */ +class NotePairs: public RefCountable { public: - NotePairs() : HttpHeader(hoNote) {} + typedef RefCount Pointer; + + /** + * Used to store a note key/value pair. + */ + class Entry { + public: + Entry(const char *aKey, const char *aValue): name(aKey), value(aValue) {} + String name; + String value; + MEMPROXY_CLASS(Entry); + }; + + NotePairs() {} + + /** + * Append the entries of the src NotePairs list to our list. + */ + void append(const NotePairs *src); - /// convert a NotesList into a NotesPairs - /// appending to any existing entries already present - void append(const Notes::NotesList &src) { - for (Notes::NotesList::const_iterator m = src.begin(); m != src.end(); ++m) - for (Note::Values::iterator v =(*m)->values.begin(); v != (*m)->values.end(); ++v) - putExt((*m)->key.termedBuf(), (*v)->value.termedBuf()); - } - - void append(const NotePairs *src) { - HttpHeader::append(dynamic_cast(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; + + /** + * 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. + */ + void add(const char *key, const char *value); + + /** + * 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. + */ + const char *toString(const char *sep = "\r\n") const; + + /** + * True if there are not entries in the list + */ + bool empty() const {return entries.empty();} + + Vector entries; ///< The key/value pair entries }; +MEMPROXY_CLASS_INLINE(NotePairs::Entry); + #endif === modified file 'src/adaptation/History.h' --- src/adaptation/History.h 2012-10-29 04:59:58 +0000 +++ src/adaptation/History.h 2013-04-05 09:14:31 +0000 @@ -36,41 +36,41 @@ /// returns true and fills the record fields iff there is a db record bool getXxRecord(String &name, String &value) const; /// sets or resets next services for the Adaptation::Iterator to notice void updateNextServices(const String &services); /// returns true, fills the value, and resets iff next services were set bool extractNextServices(String &value); /// store the last meta header fields received from the adaptation service void recordMeta(const HttpHeader *lm); public: /// Last received meta header (REQMOD or RESPMOD, whichever comes last). HttpHeader lastMeta; /// All REQMOD and RESPMOD meta headers merged. Last field wins conflicts. HttpHeader allMeta; /// key:value pairs set by adaptation_meta, to be added to /// AccessLogEntry::notes when ALE becomes available - NotePairs metaHeaders; + NotePairs::Pointer metaHeaders; /// sets future services for the Adaptation::AccessCheck to notice void setFutureServices(const DynamicGroupCfg &services); /// returns true, fills the value, and resets iff future services were set bool extractFutureServices(DynamicGroupCfg &services); private: /// single Xaction stats (i.e., a historical record entry) class Entry { public: Entry(const String &serviceId, const timeval &when); Entry(); // required by Vector<> void stop(); ///< updates stats on transaction end int rptm(); ///< returns response time [msec], calculates it if needed String service; ///< adaptation service ID timeval start; ///< when the xaction was started === modified file 'src/adaptation/ecap/XactionRep.cc' --- src/adaptation/ecap/XactionRep.cc 2012-10-27 00:13:19 +0000 +++ src/adaptation/ecap/XactionRep.cc 2013-04-05 09:19:22 +0000 @@ -214,42 +214,45 @@ Adaptation::Ecap::XactionRep::start() { Must(theMaster); if (!theVirginRep.raw().body_pipe) makingVb = opNever; // there is nothing to deliver HttpRequest *request = dynamic_cast (theCauseRep ? theCauseRep->raw().header : theVirginRep.raw().header); Must(request); HttpReply *reply = dynamic_cast(theVirginRep.raw().header); Adaptation::History::Pointer ah = request->adaptLogHistory(); 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); - if (v && !ah->metaHeaders.hasByNameListMember((*i)->key.termedBuf(), v, ',')) { - ah->metaHeaders.addEntry(new HttpHeaderEntry(HDR_OTHER, (*i)->key.termedBuf(), v)); + if (v) { + if (ah->metaHeaders == NULL) + ah->metaHeaders = new NotePairs(); + if (!ah->metaHeaders->hasPair((*i)->key.termedBuf(), v)) + ah->metaHeaders->add((*i)->key.termedBuf(), v); } } } theMaster->start(); } void Adaptation::Ecap::XactionRep::swanSong() { // clear body_pipes, if any // this code does not maintain proxying* and canAccessVb states; should it? if (theAnswerRep != NULL) { BodyPipe::Pointer body_pipe = answer().body_pipe; if (body_pipe != NULL) { Must(body_pipe->stillProducing(this)); stopProducingFor(body_pipe, false); } } === modified file 'src/adaptation/icap/ModXact.cc' --- src/adaptation/icap/ModXact.cc 2013-03-18 04:55:51 +0000 +++ src/adaptation/icap/ModXact.cc 2013-04-05 13:12:03 +0000 @@ -1415,42 +1415,46 @@ client_addr = request->client_addr; if (!client_addr.IsAnyAddr() && !client_addr.IsNoAddr()) buf.Printf("X-Client-IP: %s\r\n", client_addr.NtoA(ntoabuf,MAX_IPSTRLEN)); } if (TheConfig.send_username && request) makeUsernameHeader(request, buf); // Adaptation::Config::metaHeaders typedef Notes::iterator ACAMLI; for (ACAMLI i = Adaptation::Config::metaHeaders.begin(); i != Adaptation::Config::metaHeaders.end(); ++i) { 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)) { buf.Printf("%s: %s\r\n", (*i)->key.termedBuf(), value); Adaptation::History::Pointer ah = request->adaptHistory(false); - if (ah != NULL && !ah->metaHeaders.hasByNameListMember((*i)->key.termedBuf(), value, ',')) - ah->metaHeaders.addEntry(new HttpHeaderEntry(HDR_OTHER, (*i)->key.termedBuf(), value)); + 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); + } } } // fprintf(stderr, "%s\n", buf.content()); buf.append(ICAP::crlf, 2); // terminate ICAP header // fill icapRequest for logging Must(icapRequest->parseCharBuf(buf.content(), buf.contentSize())); // start ICAP request body with encapsulated HTTP headers buf.append(httpBuf.content(), httpBuf.contentSize()); httpBuf.clean(); } // decides which Allow values to write and updates the request buffer void Adaptation::Icap::ModXact::makeAllowHeader(MemBuf &buf) { const bool allow204in = preview.enabled(); // TODO: add shouldAllow204in() === modified file 'src/auth/digest/UserRequest.cc' --- src/auth/digest/UserRequest.cc 2013-03-18 04:55:51 +0000 +++ src/auth/digest/UserRequest.cc 2013-03-22 17:01:34 +0000 @@ -289,67 +289,67 @@ // the HA1 will be found in content() for these responses. if (!oldHelperWarningDone) { debugs(29, DBG_IMPORTANT, "WARNING: Digest auth helper returned old format HA1 response. It needs to be upgraded."); oldHelperWarningDone=true; } /* allow this because the digest_request pointer is purely local */ Auth::Digest::User *digest_user = dynamic_cast(auth_user_request->user().getRaw()); assert(digest_user != NULL); CvtBin(reply.other().content(), digest_user->HA1); digest_user->HA1created = 1; } break; case HelperReply::Okay: { /* allow this because the digest_request pointer is purely local */ Auth::Digest::User *digest_user = dynamic_cast(auth_user_request->user().getRaw()); assert(digest_user != NULL); - Note::Pointer ha1Note = reply.notes.find("ha1"); + const char *ha1Note = reply.notes.findFirst("ha1"); if (ha1Note != NULL) { - CvtBin(ha1Note->firstValue(), digest_user->HA1); + CvtBin(ha1Note, digest_user->HA1); digest_user->HA1created = 1; } else { debugs(29, DBG_IMPORTANT, "ERROR: Digest auth helper did not produce a HA1. Using the wrong helper program? received: " << reply); } } break; case HelperReply::TT: debugs(29, DBG_IMPORTANT, "ERROR: Digest auth does not support the result code received. Using the wrong helper program? received: " << reply); // fall through to next case. Handle this as an ERR response. case HelperReply::BrokenHelper: // TODO retry the broken lookup on another helper? // fall through to next case for now. Handle this as an ERR response silently. case HelperReply::Error: { /* allow this because the digest_request pointer is purely local */ Auth::Digest::UserRequest *digest_request = dynamic_cast(auth_user_request.getRaw()); assert(digest_request); digest_request->user()->credentials(Auth::Failed); digest_request->flags.invalid_password = true; - Note::Pointer msgNote = reply.notes.find("message"); + const char *msgNote = reply.notes.find("message"); if (msgNote != NULL) { - digest_request->setDenyMessage(msgNote->firstValue()); + digest_request->setDenyMessage(msgNote); } 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()); if (!oldHelperWarningDone) { debugs(29, DBG_IMPORTANT, "WARNING: Digest auth helper returned old format ERR response. It needs to be upgraded."); oldHelperWarningDone=true; } } } break; } void *cbdata = NULL; if (cbdataReferenceValidDone(replyData->data, &cbdata)) replyData->handler(cbdata); delete replyData; } === modified file 'src/auth/negotiate/UserRequest.cc' --- src/auth/negotiate/UserRequest.cc 2013-04-04 06:15:00 +0000 +++ src/auth/negotiate/UserRequest.cc 2013-04-16 15:09:05 +0000 @@ -230,131 +230,131 @@ assert(lm_request != NULL); assert(lm_request->waiting); lm_request->waiting = 0; safe_free(lm_request->client_blob); assert(auth_user_request->user() != NULL); assert(auth_user_request->user()->auth_type == Auth::AUTH_NEGOTIATE); if (lm_request->authserver == NULL) lm_request->authserver = reply.whichServer.get(); // XXX: no locking? else assert(reply.whichServer == lm_request->authserver); switch (reply.result) { case HelperReply::TT: /* we have been given a blob to send to the client */ safe_free(lm_request->server_blob); lm_request->request->flags.mustKeepalive = true; if (lm_request->request->flags.proxyKeepalive) { - Note::Pointer tokenNote = reply.notes.find("token"); - lm_request->server_blob = xstrdup(tokenNote->firstValue()); + const char *tokenNote = reply.notes.findFirst("token"); + lm_request->server_blob = xstrdup(tokenNote); auth_user_request->user()->credentials(Auth::Handshake); auth_user_request->denyMessage("Authentication in progress"); - debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << tokenNote->firstValue() << "'"); + debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << tokenNote << "'"); } else { auth_user_request->user()->credentials(Auth::Failed); auth_user_request->denyMessage("Negotiate authentication requires a persistent connection"); } break; case HelperReply::Okay: { - Note::Pointer userNote = reply.notes.find("user"); - Note::Pointer tokenNote = reply.notes.find("token"); + const char *userNote = reply.notes.findFirst("user"); + const char *tokenNote = reply.notes.findFirst("token"); if (userNote == NULL || tokenNote == NULL) { // XXX: handle a success with no username better /* protocol error */ fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply.other().content()); break; } /* we're finished, release the helper */ - auth_user_request->user()->username(userNote->firstValue()); + auth_user_request->user()->username(userNote); auth_user_request->denyMessage("Login successful"); safe_free(lm_request->server_blob); - lm_request->server_blob = xstrdup(tokenNote->firstValue()); + lm_request->server_blob = xstrdup(tokenNote); lm_request->releaseAuthServer(); /* connection is authenticated */ debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username()); /* see if this is an existing user with a different proxy_auth * string */ AuthUserHashPointer *usernamehash = static_cast(hash_lookup(proxy_auth_username_cache, auth_user_request->user()->username())); Auth::User::Pointer local_auth_user = lm_request->user(); while (usernamehash && (usernamehash->user()->auth_type != Auth::AUTH_NEGOTIATE || strcmp(usernamehash->user()->username(), auth_user_request->user()->username()) != 0)) usernamehash = static_cast(usernamehash->next); if (usernamehash) { /* we can't seamlessly recheck the username due to the * challenge-response nature of the protocol. * Just free the temporary auth_user after merging as * much of it new state into the existing one as possible */ usernamehash->user()->absorb(local_auth_user); /* from here on we are working with the original cached credentials. */ local_auth_user = usernamehash->user(); auth_user_request->user(local_auth_user); } else { /* store user in hash's */ local_auth_user->addToNameCache(); } /* set these to now because this is either a new login from an * existing user or a new user */ local_auth_user->expiretime = current_time.tv_sec; auth_user_request->user()->credentials(Auth::Ok); debugs(29, 4, HERE << "Successfully validated user via Negotiate. Username '" << auth_user_request->user()->username() << "'"); } break; case HelperReply::Error: { - Note::Pointer messageNote = reply.notes.find("message"); - Note::Pointer tokenNote = reply.notes.find("token"); + const char *messageNote = reply.notes.find("message"); + const char *tokenNote = reply.notes.findFirst("token"); /* authentication failure (wrong password, etc.) */ if (messageNote != NULL) - auth_user_request->denyMessage(messageNote->firstValue()); + auth_user_request->denyMessage(messageNote); else auth_user_request->denyMessage("Negotiate Authentication denied with no reason given"); auth_user_request->user()->credentials(Auth::Failed); safe_free(lm_request->server_blob); if (tokenNote != NULL) - lm_request->server_blob = xstrdup(tokenNote->firstValue()); + lm_request->server_blob = xstrdup(tokenNote); lm_request->releaseAuthServer(); debugs(29, 4, HERE << "Failed validating user via Negotiate. Error returned '" << reply << "'"); } break; case HelperReply::Unknown: debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication Helper '" << reply.whichServer << "' crashed!."); /* continue to the next case */ case HelperReply::BrokenHelper: { /* TODO kick off a refresh process. This can occur after a YR or after * a KK. If after a YR release the helper and resubmit the request via * Authenticate Negotiate start. * If after a KK deny the user's request w/ 407 and mark the helper as * Needing YR. */ - Note::Pointer errNote = reply.notes.find("message"); + const char *errNote = reply.notes.find("message"); if (reply.result == HelperReply::Unknown) auth_user_request->denyMessage("Internal Error"); else if (errNote != NULL) - auth_user_request->denyMessage(errNote->firstValue()); + auth_user_request->denyMessage(errNote); else auth_user_request->denyMessage("Negotiate Authentication failed with no reason given"); auth_user_request->user()->credentials(Auth::Failed); safe_free(lm_request->server_blob); lm_request->releaseAuthServer(); debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication validating user. Error returned " << reply); } // break; } if (lm_request->request) { HTTPMSGUNLOCK(lm_request->request); lm_request->request = NULL; } r->handler(r->data); delete r; } void Auth::Negotiate::UserRequest::addAuthenticationInfoHeader(HttpReply * rep, int accel) { === modified file 'src/auth/ntlm/UserRequest.cc' --- src/auth/ntlm/UserRequest.cc 2013-04-04 06:15:00 +0000 +++ src/auth/ntlm/UserRequest.cc 2013-04-16 15:09:05 +0000 @@ -224,116 +224,116 @@ assert(lm_request != NULL); assert(lm_request->waiting); lm_request->waiting = 0; safe_free(lm_request->client_blob); assert(auth_user_request->user() != NULL); assert(auth_user_request->user()->auth_type == Auth::AUTH_NTLM); if (lm_request->authserver == NULL) lm_request->authserver = reply.whichServer.get(); // XXX: no locking? else assert(reply.whichServer == lm_request->authserver); switch (reply.result) { case HelperReply::TT: /* we have been given a blob to send to the client */ safe_free(lm_request->server_blob); lm_request->request->flags.mustKeepalive = true; if (lm_request->request->flags.proxyKeepalive) { - Note::Pointer serverBlob = reply.notes.find("token"); - lm_request->server_blob = xstrdup(serverBlob->firstValue()); + const char *serverBlob = reply.notes.findFirst("token"); + lm_request->server_blob = xstrdup(serverBlob); auth_user_request->user()->credentials(Auth::Handshake); auth_user_request->denyMessage("Authentication in progress"); - debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << serverBlob->firstValue() << "'"); + debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << serverBlob << "'"); } else { auth_user_request->user()->credentials(Auth::Failed); auth_user_request->denyMessage("NTLM authentication requires a persistent connection"); } break; case HelperReply::Okay: { /* we're finished, release the helper */ - Note::Pointer userLabel = reply.notes.find("user"); - auth_user_request->user()->username(userLabel->firstValue()); + const char *userLabel = reply.notes.findFirst("user"); + auth_user_request->user()->username(userLabel); auth_user_request->denyMessage("Login successful"); safe_free(lm_request->server_blob); lm_request->releaseAuthServer(); - debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << userLabel->firstValue() << "'"); + debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << userLabel << "'"); /* connection is authenticated */ debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username()); /* see if this is an existing user with a different proxy_auth * string */ AuthUserHashPointer *usernamehash = static_cast(hash_lookup(proxy_auth_username_cache, auth_user_request->user()->username())); Auth::User::Pointer local_auth_user = lm_request->user(); while (usernamehash && (usernamehash->user()->auth_type != Auth::AUTH_NTLM || strcmp(usernamehash->user()->username(), auth_user_request->user()->username()) != 0)) usernamehash = static_cast(usernamehash->next); if (usernamehash) { /* we can't seamlessly recheck the username due to the * challenge-response nature of the protocol. * Just free the temporary auth_user after merging as * much of it new state into the existing one as possible */ usernamehash->user()->absorb(local_auth_user); /* from here on we are working with the original cached credentials. */ local_auth_user = usernamehash->user(); auth_user_request->user(local_auth_user); } else { /* store user in hash's */ local_auth_user->addToNameCache(); } /* set these to now because this is either a new login from an * existing user or a new user */ local_auth_user->expiretime = current_time.tv_sec; auth_user_request->user()->credentials(Auth::Ok); debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << auth_user_request->user()->username() << "'"); } break; case HelperReply::Error: { /* authentication failure (wrong password, etc.) */ - Note::Pointer errNote = reply.notes.find("message"); + const char *errNote = reply.notes.find("message"); if (errNote != NULL) - auth_user_request->denyMessage(errNote->firstValue()); + auth_user_request->denyMessage(errNote); else auth_user_request->denyMessage("NTLM Authentication denied with no reason given"); auth_user_request->user()->credentials(Auth::Failed); safe_free(lm_request->server_blob); lm_request->releaseAuthServer(); - debugs(29, 4, HERE << "Failed validating user via NTLM. Error returned '" << errNote->firstValue() << "'"); + debugs(29, 4, HERE << "Failed validating user via NTLM. Error returned '" << errNote << "'"); } break; case HelperReply::Unknown: debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication Helper '" << reply.whichServer << "' crashed!."); /* continue to the next case */ case HelperReply::BrokenHelper: { /* TODO kick off a refresh process. This can occur after a YR or after * a KK. If after a YR release the helper and resubmit the request via * Authenticate NTLM start. * If after a KK deny the user's request w/ 407 and mark the helper as * Needing YR. */ - Note::Pointer errNote = reply.notes.find("message"); + const char *errNote = reply.notes.find("message"); if (reply.result == HelperReply::Unknown) auth_user_request->denyMessage("Internal Error"); else if (errNote != NULL) - auth_user_request->denyMessage(errNote->firstValue()); + auth_user_request->denyMessage(errNote); else auth_user_request->denyMessage("NTLM Authentication failed with no reason given"); auth_user_request->user()->credentials(Auth::Failed); safe_free(lm_request->server_blob); lm_request->releaseAuthServer(); debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication validating user. Error returned '" << reply << "'"); } break; } if (lm_request->request) { HTTPMSGUNLOCK(lm_request->request); lm_request->request = NULL; } r->handler(r->data); delete r; } === modified file 'src/client_side.cc' --- src/client_side.cc 2013-04-04 06:15:00 +0000 +++ src/client_side.cc 2013-04-16 15:09:05 +0000 @@ -581,59 +581,56 @@ //This is the request after adaptation or redirection aLogEntry->headers.adapted_request = xstrdup(mb.buf); // the virgin request is saved to aLogEntry->request if (aLogEntry->request) { packerClean(&p); mb.reset(); packerToMemInit(&p, &mb); aLogEntry->request->header.packInto(&p); aLogEntry->headers.request = xstrdup(mb.buf); } #if USE_ADAPTATION const Adaptation::History::Pointer ah = request->adaptLogHistory(); if (ah != NULL) { packerClean(&p); mb.reset(); packerToMemInit(&p, &mb); ah->lastMeta.packInto(&p); aLogEntry->adapt.last_meta = xstrdup(mb.buf); - aLogEntry->notes.append(&ah->metaHeaders); } #endif packerClean(&p); mb.clean(); } #if ICAP_CLIENT const Adaptation::Icap::History::Pointer ih = request->icapHistory(); if (ih != NULL) aLogEntry->icap.processingTime = ih->processingTime(); #endif aLogEntry->http.method = request->method; aLogEntry->http.version = request->http_ver; aLogEntry->hier = request->hier; - if (request->helperNotes) - aLogEntry->notes.append(request->helperNotes->notes); if (request->content_length > 0) // negative when no body or unknown length aLogEntry->cache.requestSize += request->content_length; aLogEntry->cache.extuser = request->extacl_user.termedBuf(); #if USE_AUTH if (request->auth_user_request != NULL) { if (request->auth_user_request->username()) aLogEntry->cache.authuser = xstrdup(request->auth_user_request->username()); } #endif // Adapted request, if any, inherits and then collects all the stats, but // the virgin request gets logged instead; copy the stats to log them. // TODO: avoid losses by keeping these stats in a shared history object? if (aLogEntry->request) { aLogEntry->request->dnsWait = request->dnsWait; aLogEntry->request->errType = request->errType; aLogEntry->request->errDetail = request->errDetail; } } @@ -683,41 +680,43 @@ if (request) prepareLogWithRequestDetails(request, al); if (getConn() != NULL && getConn()->clientConnection != NULL && getConn()->clientConnection->rfc931[0]) al->cache.rfc931 = getConn()->clientConnection->rfc931; #if USE_SSL && 0 /* This is broken. Fails if the connection has been closed. Needs * to snarf the ssl details some place earlier.. */ if (getConn() != NULL) al->cache.ssluser = sslGetUserEmail(fd_table[getConn()->fd].ssl); #endif /*Add meta headers*/ typedef Notes::iterator ACAMLI; for (ACAMLI i = Config.notes.begin(); i != Config.notes.end(); ++i) { if (const char *value = (*i)->match(request, al->reply)) { - al->notes.addEntry(new HttpHeaderEntry(HDR_OTHER, (*i)->key.termedBuf(), value)); + if (al->configNotes == NULL) + al->configNotes = new NotePairs; + al->configNotes->add((*i)->key.termedBuf(), value); debugs(33, 3, HERE << (*i)->key.termedBuf() << " " << value); } } ACLFilledChecklist *checklist = clientAclChecklistCreate(Config.accessList.log, this); if (al->reply) { checklist->reply = al->reply; HTTPMSGLOCK(checklist->reply); } if (!Config.accessList.log || checklist->fastCheck() == ACCESS_ALLOWED) { if (request) { al->adapted_request = request; HTTPMSGLOCK(al->adapted_request); } accessLogLog(al, checklist); if (request) updateCounters(); === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2013-04-04 06:15:00 +0000 +++ src/client_side_request.cc 2013-04-16 15:09:05 +0000 @@ -1239,198 +1239,202 @@ void clientStoreIdDoneWrapper(void *data, const HelperReply &result) { ClientRequestContext *calloutContext = (ClientRequestContext *)data; if (!calloutContext->httpStateIsValid()) return; calloutContext->clientStoreIdDone(result); } void ClientRequestContext::clientRedirectDone(const HelperReply &reply) { HttpRequest *old_request = http->request; debugs(85, 5, HERE << "'" << http->uri << "' result=" << reply); assert(redirect_state == REDIRECT_PENDING); redirect_state = REDIRECT_DONE; - // copy the URL rewriter response Notes to the HTTP request for logging + // 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. - // TODO put them straight into the transaction state record (ALE?) eventually - if (!old_request->helperNotes) - old_request->helperNotes = new Notes; - old_request->helperNotes->add(reply.notes); + if (http->al != NULL) { + if (!http->al->helperNotes) + http->al->helperNotes = new NotePairs; + http->al->helperNotes->append(&reply.notes); + old_request->helperNotes = http->al->helperNotes; + } switch (reply.result) { case HelperReply::Unknown: case HelperReply::TT: // Handler in redirect.cc should have already mapped Unknown // IF it contained valid entry for the old URL-rewrite helper protocol debugs(85, DBG_IMPORTANT, "ERROR: URL rewrite helper returned invalid result code. Wrong helper? " << reply); break; case HelperReply::BrokenHelper: debugs(85, DBG_IMPORTANT, "ERROR: URL rewrite helper: " << reply << ", attempt #" << (redirect_fail_count+1) << " of 2"); if (redirect_fail_count < 2) { // XXX: make this configurable ? ++redirect_fail_count; // reset state flag to try redirector again from scratch. redirect_done = false; } break; case HelperReply::Error: // no change to be done. break; case HelperReply::Okay: { // #1: redirect with a specific status code OK status=NNN url="..." // #2: redirect with a default status code OK url="..." // #3: re-write the URL OK rewrite-url="..." - Note::Pointer statusNote = reply.notes.find("status"); - Note::Pointer urlNote = reply.notes.find("url"); + const char *statusNote = reply.notes.findFirst("status"); + const char *urlNote = reply.notes.findFirst("url"); if (urlNote != NULL) { // HTTP protocol redirect to be done. // TODO: change default redirect status for appropriate requests // Squid defaults to 302 status for now for better compatibility with old clients. // HTTP/1.0 client should get 302 (Http::scMovedTemporarily) // HTTP/1.1 client contacting reverse-proxy should get 307 (Http::scTemporaryRedirect) // HTTP/1.1 client being diverted by forward-proxy should get 303 (Http::scSeeOther) Http::StatusCode status = Http::scMovedTemporarily; if (statusNote != NULL) { - const char * result = statusNote->firstValue(); + const char * result = statusNote; status = static_cast(atoi(result)); } if (status == Http::scMovedPermanently || status == Http::scMovedTemporarily || status == Http::scSeeOther || status == Http::scPermanentRedirect || status == Http::scTemporaryRedirect) { http->redirect.status = status; - http->redirect.location = xstrdup(urlNote->firstValue()); + http->redirect.location = xstrdup(urlNote); // TODO: validate the URL produced here is RFC 2616 compliant absolute URI } else { - debugs(85, DBG_CRITICAL, "ERROR: URL-rewrite produces invalid " << status << " redirect Location: " << urlNote->firstValue()); + debugs(85, DBG_CRITICAL, "ERROR: URL-rewrite produces invalid " << status << " redirect Location: " << urlNote); } } else { // URL-rewrite wanted. Ew. - urlNote = reply.notes.find("rewrite-url"); + urlNote = reply.notes.findFirst("rewrite-url"); // prevent broken helpers causing too much damage. If old URL == new URL skip the re-write. - if (urlNote != NULL && strcmp(urlNote->firstValue(), http->uri)) { + if (urlNote != NULL && strcmp(urlNote, http->uri)) { // XXX: validate the URL properly *without* generating a whole new request object right here. // XXX: the clone() should be done only AFTER we know the new URL is valid. HttpRequest *new_request = old_request->clone(); - if (urlParse(old_request->method, const_cast(urlNote->firstValue()), new_request)) { + if (urlParse(old_request->method, const_cast(urlNote), new_request)) { debugs(61,2, HERE << "URL-rewriter diverts URL from " << urlCanonical(old_request) << " to " << urlCanonical(new_request)); // update the new request to flag the re-writing was done on it new_request->flags.redirected = true; // unlink bodypipe from the old request. Not needed there any longer. if (old_request->body_pipe != NULL) { old_request->body_pipe = NULL; debugs(61,2, HERE << "URL-rewriter diverts body_pipe " << new_request->body_pipe << " from request " << old_request << " to " << new_request); } // update the current working ClientHttpRequest fields safe_free(http->uri); http->uri = xstrdup(urlCanonical(new_request)); HTTPMSGUNLOCK(old_request); http->request = new_request; HTTPMSGLOCK(http->request); } else { debugs(85, DBG_CRITICAL, "ERROR: URL-rewrite produces invalid request: " << - old_request->method << " " << urlNote->firstValue() << " " << old_request->http_ver); + old_request->method << " " << urlNote << " " << old_request->http_ver); delete new_request; } } } } break; } /* FIXME PIPELINE: This is innacurate during pipelining */ if (http->getConn() != NULL && Comm::IsConnOpen(http->getConn()->clientConnection)) fd_note(http->getConn()->clientConnection->fd, http->uri); assert(http->uri); http->doCallouts(); } /** * This method handles the different replies from StoreID helper. */ void ClientRequestContext::clientStoreIdDone(const HelperReply &reply) { HttpRequest *old_request = http->request; debugs(85, 5, "'" << http->uri << "' result=" << reply); assert(store_id_state == REDIRECT_PENDING); store_id_state = REDIRECT_DONE; - // copy the helper response Notes to the HTTP request for logging + // 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. - // TODO put them straight into the transaction state record (ALE?) eventually - if (!old_request->helperNotes) - old_request->helperNotes = new Notes; - old_request->helperNotes->add(reply.notes); + if (http->al != NULL) { + if (!http->al->helperNotes) + http->al->helperNotes = new NotePairs; + http->al->helperNotes->append(&reply.notes); + old_request->helperNotes = http->al->helperNotes; + } switch (reply.result) { case HelperReply::Unknown: case HelperReply::TT: // Handler in redirect.cc should have already mapped Unknown // IF it contained valid entry for the old helper protocol debugs(85, DBG_IMPORTANT, "ERROR: storeID helper returned invalid result code. Wrong helper? " << reply); break; case HelperReply::BrokenHelper: debugs(85, DBG_IMPORTANT, "ERROR: storeID helper: " << reply << ", attempt #" << (store_id_fail_count+1) << " of 2"); if (store_id_fail_count < 2) { // XXX: make this configurable ? ++store_id_fail_count; // reset state flag to try StoreID again from scratch. store_id_done = false; } break; case HelperReply::Error: // no change to be done. break; case HelperReply::Okay: { - Note::Pointer urlNote = reply.notes.find("store-id"); + const char *urlNote = reply.notes.findFirst("store-id"); // prevent broken helpers causing too much damage. If old URL == new URL skip the re-write. - if (urlNote != NULL && strcmp(urlNote->firstValue(), http->uri) ) { + if (urlNote != NULL && strcmp(urlNote, http->uri) ) { // Debug section required for some very specific cases. - debugs(85, 9, "Setting storeID with: " << urlNote->firstValue() ); - http->request->store_id = urlNote->firstValue(); - http->store_id = urlNote->firstValue(); + debugs(85, 9, "Setting storeID with: " << urlNote ); + http->request->store_id = urlNote; + http->store_id = urlNote; } } break; } http->doCallouts(); } /** Test cache allow/deny configuration * Sets flags.cachable=1 if caching is not denied. */ void ClientRequestContext::checkNoCache() { if (Config.accessList.noCache) { acl_checklist = clientAclChecklistCreate(Config.accessList.noCache, http); acl_checklist->nonBlockingCheck(checkNoCacheDoneWrapper, this); } else { /* unless otherwise specified, we try to cache. */ checkNoCacheDone(ACCESS_ALLOWED); === modified file 'src/external_acl.cc' --- src/external_acl.cc 2013-01-27 17:35:07 +0000 +++ src/external_acl.cc 2013-03-08 16:33:26 +0000 @@ -1311,60 +1311,60 @@ * value needs to be URL-encoded or enclosed in double quotes (") * with \-escaping on any whitespace, quotes, or slashes (\). */ static void externalAclHandleReply(void *data, const HelperReply &reply) { externalAclState *state = static_cast(data); externalAclState *next; ExternalACLEntryData entryData; entryData.result = ACCESS_DENIED; external_acl_entry *entry = NULL; debugs(82, 2, HERE << "reply=" << reply); if (reply.result == HelperReply::Okay) entryData.result = ACCESS_ALLOWED; // XXX: handle other non-DENIED results better // XXX: make entryData store a proper HelperReply object instead of copying. - Note::Pointer label = reply.notes.find("tag"); - if (label != NULL && label->values[0]->value.size() > 0) - entryData.tag = label->values[0]->value; - - label = reply.notes.find("message"); - if (label != NULL && label->values[0]->value.size() > 0) - entryData.message = label->values[0]->value; - - label = reply.notes.find("log"); - if (label != NULL && label->values[0]->value.size() > 0) - entryData.log = label->values[0]->value; + const char *label = reply.notes.findFirst("tag"); + if (label != NULL && *label != '\0') + entryData.tag = label; + + label = reply.notes.findFirst("message"); + if (label != NULL && *label != '\0') + entryData.message = label; + + label = reply.notes.findFirst("log"); + if (label != NULL && *label != '\0') + entryData.log = label; #if USE_AUTH - label = reply.notes.find("user"); - if (label != NULL && label->values[0]->value.size() > 0) - entryData.user = label->values[0]->value; - - label = reply.notes.find("password"); - if (label != NULL && label->values[0]->value.size() > 0) - entryData.password = label->values[0]->value; + label = reply.notes.findFirst("user"); + if (label != NULL && *label != '\0') + entryData.user = label; + + label = reply.notes.findFirst("password"); + if (label != NULL && *label != '\0') + entryData.password = label; #endif dlinkDelete(&state->list, &state->def->queue); if (cbdataReferenceValid(state->def)) { // only cache OK and ERR results. if (reply.result == HelperReply::Okay || reply.result == HelperReply::Error) entry = external_acl_cache_add(state->def, state->key, entryData); else { external_acl_entry *oldentry = (external_acl_entry *)hash_lookup(state->def->cache, state->key); if (oldentry) external_acl_cache_delete(state->def, oldentry); } } do { void *cbdata; cbdataReferenceDone(state->def); === modified file 'src/format/Format.cc' --- src/format/Format.cc 2013-03-16 04:57:43 +0000 +++ src/format/Format.cc 2013-04-16 14:52:56 +0000 @@ -1025,51 +1025,73 @@ case LFT_SSL_USER_CERT_SUBJECT: if (X509 *cert = al->cache.sslClientCert.get()) { if (X509_NAME *subject = X509_get_subject_name(cert)) { X509_NAME_oneline(subject, tmp, sizeof(tmp)); out = tmp; } } break; case LFT_SSL_USER_CERT_ISSUER: if (X509 *cert = al->cache.sslClientCert.get()) { if (X509_NAME *issuer = X509_get_issuer_name(cert)) { X509_NAME_oneline(issuer, tmp, sizeof(tmp)); out = tmp; } } break; #endif case LFT_NOTE: if (fmt->data.string) { - sb = al->notes.getByName(fmt->data.string); +#if USE_ADAPTATION + Adaptation::History::Pointer ah = al->request->adaptHistory(); + if (ah != NULL && ah->metaHeaders != NULL) { + if (const char *meta = ah->metaHeaders->find(fmt->data.string)) + sb.append(meta); + } +#endif + if (al->helperNotes != NULL) { + if (const char *note = al->helperNotes->find(fmt->data.string)) { + if (sb.size()) + sb.append(", "); + sb.append(note); + } + } + if (al->configNotes != NULL) { + if (const char *note = al->configNotes->find(fmt->data.string)) { + if (sb.size()) + sb.append(", "); + sb.append(note); + } + } out = sb.termedBuf(); quote = 1; } else { - HttpHeaderPos pos = HttpHeaderInitPos; - while (const HttpHeaderEntry *e = al->notes.getEntry(&pos)) { - sb.append(e->name); - sb.append(": "); - sb.append(e->value); - sb.append("\r\n"); - } +#if USE_ADAPTATION + Adaptation::History::Pointer ah = al->request->adaptHistory(); + if (ah != NULL && ah->metaHeaders != NULL && !ah->metaHeaders->empty()) + sb.append(ah->metaHeaders->toString()); +#endif + if (al->helperNotes != NULL && !al->helperNotes->empty()) + sb.append(al->helperNotes->toString()); + if (al->configNotes != NULL && !al->configNotes->empty()) + sb.append(al->configNotes->toString()); out = sb.termedBuf(); quote = 1; } break; case LFT_PERCENT: out = "%"; break; } if (dooff) { snprintf(tmp, sizeof(tmp), "%0*" PRId64, fmt->zero && fmt->widthMin >= 0 ? fmt->widthMin : 0, outoff); out = tmp; } else if (doint) { snprintf(tmp, sizeof(tmp), "%0*ld", fmt->zero && fmt->widthMin >= 0 ? fmt->widthMin : 0, outint); out = tmp; }