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-05 12:29:23 +0000
@@ -215,43 +215,44 @@
         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;
+    /// key:value pairs set by note
+    NotePairs::Pointer notes;
+    /// 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/ConfigParser.cc'
--- src/ConfigParser.cc	2013-01-27 17:35:07 +0000
+++ src/ConfigParser.cc	2013-04-03 16:11:36 +0000
@@ -171,41 +171,41 @@
     /* scan until the end of the quoted string, unescaping " and \  */
     while (*s && *s != '"') {
         if (*s == '\\') {
             const char * next = s+1; // may point to 0
             memmove(s, next, strlen(next) + 1);
         }
         ++s;
     }
 
     if (*s != '"') {
         debugs(3, DBG_CRITICAL, "ParseQuotedString: missing '\"' at the end of quoted string" );
         self_destruct();
     }
     strtok(s-1, "\""); /*Reset the strtok to point after the "  */
     *s = '\0';
 
     var->reset(token+1);
 }
 
 const char *
-ConfigParser::QuoteString(String &var)
+ConfigParser::QuoteString(const String &var)
 {
     static String quotedStr;
     const char *s = var.termedBuf();
     bool  needQuote = false;
 
     for (const char *l = s; !needQuote &&  *l != '\0'; ++l  )
         needQuote = !isalnum(*l);
 
     if (!needQuote)
         return s;
 
     quotedStr.clean();
     quotedStr.append('"');
     for (; *s != '\0'; ++s) {
         if (*s == '"' || *s == '\\')
             quotedStr.append('\\');
         quotedStr.append(*s);
     }
     quotedStr.append('"');
     return quotedStr.termedBuf();

=== modified file 'src/ConfigParser.h'
--- src/ConfigParser.h	2013-01-27 17:35:07 +0000
+++ src/ConfigParser.h	2013-04-03 16:11:25 +0000
@@ -60,33 +60,33 @@
  * using modules without linking cache_cf.o in - because that drags
  * in all of squid by reference. Instead the tokeniser only is
  * brought in.
  */
 class ConfigParser
 {
 
 public:
     void destruct();
     static void ParseUShort(unsigned short *var);
     static void ParseBool(bool *var);
     static void ParseString(char **var);
     static void ParseString(String *var);
     /// Parse an unquoted token (no spaces) or a "quoted string" that
     /// may include spaces. In some contexts, quotes strings may also
     /// include macros. Quoted strings may escape any character with
     /// a backslash (\), which is currently only useful for inner
     /// quotes. TODO: support quoted strings anywhere a token is accepted.
     static void ParseQuotedString(char **var, bool *wasQuoted = NULL);
     static void ParseQuotedString(String *var, bool *wasQuoted = NULL);
-    static const char *QuoteString(String &var);
+    static const char *QuoteString(const String &var);
     static void ParseWordList(wordlist **list);
     static char * strtokFile();
     static void strtokFileUndo();
     static void strtokFilePutBack(const char *);
 private:
     static char *lastToken;
     static std::queue<std::string> undo;
 };
 
 int parseConfigFile(const char *file_name);
 
 #endif /* SQUID_CONFIGPARSER_H */

=== modified file 'src/HelperReply.cc'
--- src/HelperReply.cc	2013-03-20 04:48:17 +0000
+++ src/HelperReply.cc	2013-04-05 10:42:28 +0000
@@ -128,76 +128,72 @@
 }
 
 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() > 0) {
         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<helper_stateful_server> 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-05 12:55:25 +0000
@@ -151,44 +151,42 @@
     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;
+    if (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 +212,42 @@
     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;
-    }
+    if (helperNotes != NULL)
+        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<const HttpRequest*>(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-05 10:33:08 +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 <algorithm>
 #include <string>
 
 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 &noteKey) const
+Notes::add(const String &noteKey)
 {
-    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 &noteKey, const String &noteValue)
-{
-    Note::Pointer key = add(noteKey);
-    key->addValue(noteValue);
-}
-
-Note::Pointer
-Notes::add(const String &noteKey)
-{
-    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, &noteValue->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<NotePairs::Entry *>::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<NotePairs::Entry *>::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<NotePairs::Entry *>::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<NotePairs::Entry *>::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<NotePairs::Entry *>::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 <string>
 #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<Note> Pointer;
     /// Stores a value for the note.
     class Value: public RefCountable
     {
     public:
         typedef RefCount<Value> 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<Value::Pointer> 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<Note::Pointer> 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 &noteKey, const String &noteValue);
-
-    /**
-     * 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 &noteKey) 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 &noteKey);
 };
 
-class NotePairs : public HttpHeader
+/**
+ * Used to store list of notes
+ */
+class NotePairs: public RefCountable
 {
 public:
-    NotePairs() : HttpHeader(hoNote) {}
+    typedef RefCount<NotePairs> 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<const HttpHeader*>(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<NotePairs::Entry *> 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<HttpRequest*> (theCauseRep ?
                            theCauseRep->raw().header : theVirginRep.raw().header);
     Must(request);
 
     HttpReply *reply = dynamic_cast<HttpReply*>(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<HttpRequest*>(virgin.header);
         Must(r);
 
         HttpReply *reply = dynamic_cast<HttpReply*>(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::Digest::User *>(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::Digest::User *>(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::Digest::UserRequest *>(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-03-18 04:55:51 +0000
+++ src/auth/negotiate/UserRequest.cc	2013-03-22 17:01:34 +0000
@@ -251,131 +251,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<AuthUserHashPointer *>(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<AuthUserHashPointer *>(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-01-28 16:56:05 +0000
+++ src/auth/ntlm/UserRequest.cc	2013-03-08 15:45:40 +0000
@@ -244,116 +244,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<AuthUserHashPointer *>(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<AuthUserHashPointer *>(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-03-28 03:16:05 +0000
+++ src/client_side.cc	2013-04-05 09:32:43 +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->notes == NULL)
+                al->notes = new NotePairs;
+            al->notes->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-03-26 10:33:10 +0000
+++ src/client_side_request.cc	2013-04-05 12:53:33 +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<Http::StatusCode>(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<char*>(urlNote->firstValue()), new_request)) {
+                if (urlParse(old_request->method, const_cast<char*>(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<externalAclState *>(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-05 12:34:32 +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->notes != NULL) {
+                    if (const char *note = al->notes->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->notes != NULL && !al->notes->empty())
+                    sb.append(al->notes->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;
         }
 


