=== modified file 'helpers/digest_auth/LDAP/digest_pw_auth.cc'
--- helpers/digest_auth/LDAP/digest_pw_auth.cc	2012-08-28 13:00:30 +0000
+++ helpers/digest_auth/LDAP/digest_pw_auth.cc	2012-11-09 04:01:31 +0000
@@ -64,10 +64,10 @@
     requestData->error = 0;
     GetHHA1(requestData);
     if (requestData->error) {
-        SEND_ERR("No such user");
+        SEND_ERR("message=\"No such user\"");
         return;
     }
-    printf("%s\n", requestData->HHA1);
+    printf("OK ha1=\"%s\"\n", requestData->HHA1);
 }
 
 static void
@@ -76,7 +76,7 @@
     RequestData requestData;
     ParseBuffer(buf, &requestData);
     if (!requestData.parsed) {
-        SEND_ERR("");
+        SEND_BH("message=\"Invalid line received\"");
         return;
     }
     OutputHHA1(&requestData);

=== modified file 'helpers/digest_auth/eDirectory/digest_pw_auth.cc'
--- helpers/digest_auth/eDirectory/digest_pw_auth.cc	2012-01-20 18:55:04 +0000
+++ helpers/digest_auth/eDirectory/digest_pw_auth.cc	2012-11-09 04:03:57 +0000
@@ -64,10 +64,10 @@
     requestData->error = 0;
     GetHHA1(requestData);
     if (requestData->error) {
-        SEND_ERR("No such user");
+        SEND_ERR("message=\"No such user\"");
         return;
     }
-    printf("%s\n", requestData->HHA1);
+    printf("OK ha1=\"%s\"\n", requestData->HHA1);
 }
 
 static void
@@ -76,7 +76,7 @@
     RequestData requestData;
     ParseBuffer(buf, &requestData);
     if (!requestData.parsed) {
-        SEND_ERR("");
+        SEND_BH("message=\"Invalid line received\"");
         return;
     }
     OutputHHA1(&requestData);

=== modified file 'helpers/digest_auth/file/digest_file_auth.cc'
--- helpers/digest_auth/file/digest_file_auth.cc	2012-01-20 18:55:04 +0000
+++ helpers/digest_auth/file/digest_file_auth.cc	2012-11-09 04:01:03 +0000
@@ -38,8 +38,6 @@
 #include "helpers/defines.h"
 #include "text_backend.h"
 
-#define PROGRAM_NAME "digest_file_auth"
-
 static void
 GetHHA1(RequestData * requestData)
 {
@@ -68,10 +66,10 @@
     requestData->error = 0;
     GetHHA1(requestData);
     if (requestData->error) {
-        SEND_ERR("No such user");
+        SEND_ERR("message=\"No such user\"");
         return;
     }
-    printf("%s\n", requestData->HHA1);
+    printf("OK ha1=\"%s\"\n", requestData->HHA1);
 }
 
 static void
@@ -80,7 +78,7 @@
     RequestData requestData;
     ParseBuffer(buf, &requestData);
     if (!requestData.parsed) {
-        SEND_ERR("");
+        SEND_BH("message=\"Invalid line received\"");
         return;
     }
     OutputHHA1(&requestData);

=== modified file 'helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in'
--- helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in	2012-06-12 08:50:53 +0000
+++ helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in	2012-09-22 09:04:27 +0000
@@ -151,10 +151,10 @@
 
     print(stderr "Received: Channel=".$cid.", UID='".$uid."'\n") if ($debug);
 
-    $status = $cid . " ERR database error";
+    $status = $cid . " ERR message=\"database error\"";
     my $sth = query_db($uid) || next;
     print(stderr "Rows: ". $sth->rows()."\n") if ($debug);
-    $status = $cid . " ERR unknown UID '".$uid."'";
+    $status = $cid . " ERR message=\"unknown UID '".$uid."'\"";
     my $row = $sth->fetchrow_hashref() || next;
     $status = $cid . " OK" . ($row->{'user'} ne "" ? " user=" . $row->{'user'} : "" ) . ($row->{'tag'} ne "" ? " tag=" . $row->{'tag'} : "" );
     $sth->finish();

=== modified file 'helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc'
--- helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc	2012-11-14 05:34:55 +0000
+++ helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc	2012-11-20 00:02:19 +0000
@@ -1765,7 +1765,7 @@
         /* No space given, but group string is required --> ERR */
         if ((edui_conf.mode & EDUI_MODE_GROUP) && (p == NULL)) {
             debug("while() -> Search group is missing. (required)\n");
-            local_printfx("ERR (Search Group Required)\n");
+            local_printfx("ERR message=\"(Search Group Required)\"\n");
             continue;
         }
         x = 0;
@@ -1808,7 +1808,7 @@
             if (x != LDAP_ERR_SUCCESS) {
                 /* Unable to bind */
                 debug("BindLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
-                local_printfx("ERR (BindLDAP: %s - %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
+                local_printfx("ERR message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
                 continue;
             } else
                 debug("BindLDAP(-, %s, %s, (LDAP_AUTH_TLS)) -> %s\n", edui_conf.dn, edui_conf.passwd, ErrLDAP(x));
@@ -1819,7 +1819,7 @@
                 if (x != LDAP_ERR_SUCCESS) {
                     /* Unable to bind */
                     debug("BindLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
-                    local_printfx("ERR (BindLDAP: %s - %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
+                    local_printfx("ERR message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
                     continue;
                 } else
                     debug("BindLDAP(-, %s, %s, (LDAP_AUTH_SIMPLE)) -> %s\n", edui_conf.dn, edui_conf.passwd, ErrLDAP(x));
@@ -1829,7 +1829,7 @@
                 if (x != LDAP_ERR_SUCCESS) {
                     /* Unable to bind */
                     debug("BindLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
-                    local_printfx("ERR (BindLDAP: %s - %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
+                    local_printfx("ERR message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
                     continue;
                 } else
                     debug("BindLDAP(-, -, -, (LDAP_AUTH_NONE)) -> %s\n", ErrLDAP(x));
@@ -1848,7 +1848,7 @@
             /* Everything failed --> ERR */
             debug("while() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
             CloseLDAP(&edui_ldap);
-            local_printfx("ERR (General Failure: %s)\n", ErrLDAP(x));
+            local_printfx("ERR message=\"(General Failure: %s)\"\n", ErrLDAP(x));
             continue;
         }
         edui_ldap.err = -1;
@@ -1863,14 +1863,14 @@
                 x = ConvertIP(&edui_ldap, bufb);
                 if (x < 0) {
                     debug("ConvertIP() -> %s\n", ErrLDAP(x));
-                    local_printfx("ERR (ConvertIP: %s)\n", ErrLDAP(x));
+                    local_printfx("ERR message=\"(ConvertIP: %s)\"\n", ErrLDAP(x));
                 } else {
                     edui_ldap.err = -1;
                     debug("ConvertIP(-, %s) -> Result[%d]: %s\n", bufb, x, edui_ldap.search_ip);
                     x = SearchFilterLDAP(&edui_ldap, bufa);
                     if (x < 0) {
                         debug("SearchFilterLDAP() -> %s\n", ErrLDAP(x));
-                        local_printfx("ERR (SearchFilterLDAP: %s)\n", ErrLDAP(x));
+                        local_printfx("ERR message=\"(SearchFilterLDAP: %s)\"\n", ErrLDAP(x));
                     } else {
                         /* Do Search */
                         edui_ldap.err = -1;
@@ -1878,14 +1878,14 @@
                         x = SearchLDAP(&edui_ldap, edui_ldap.scope, edui_ldap.search_filter, (char **) &search_attrib);
                         if (x != LDAP_ERR_SUCCESS) {
                             debug("SearchLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
-                            local_printfx("ERR (SearchLDAP: %s)\n", ErrLDAP(x));
+                            local_printfx("ERR message=\"(SearchLDAP: %s)\"\n", ErrLDAP(x));
                         } else {
                             edui_ldap.err = -1;
                             debug("SearchLDAP(-, %d, %s, -) -> %s\n", edui_conf.scope, edui_ldap.search_filter, ErrLDAP(x));
                             x = SearchIPLDAP(&edui_ldap);
                             if (x != LDAP_ERR_SUCCESS) {
                                 debug("SearchIPLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
-                                local_printfx("ERR (SearchIPLDAP: %s)\n", ErrLDAP(x));
+                                local_printfx("ERR message=\"(SearchIPLDAP: %s)\"\n", ErrLDAP(x));
                             } else {
                                 debug("SearchIPLDAP(-, %s) -> %s\n", edui_ldap.userid, ErrLDAP(x));
                                 local_printfx("OK user=%s\n", edui_ldap.userid);			/* Got userid --> OK user=<userid> */
@@ -1897,35 +1897,35 @@
                 }
             } else {
                 debug("StringSplit() -> Error: %" PRIuSIZE "\n", i);
-                local_printfx("ERR (StringSplit Error %" PRIuSIZE ")\n", i);
+                local_printfx("ERR message=\"(StringSplit Error %" PRIuSIZE ")\"\n", i);
             }
         } else {
             /* No group to match against, only an IP */
             x = ConvertIP(&edui_ldap, bufa);
             if (x < 0) {
                 debug("ConvertIP() -> %s\n", ErrLDAP(x));
-                local_printfx("ERR (ConvertIP: %s)\n", ErrLDAP(x));
+                local_printfx("ERR message=\"(ConvertIP: %s)\"\n", ErrLDAP(x));
             } else {
                 debug("ConvertIP(-, %s) -> Result[%d]: %s\n", bufa, x, edui_ldap.search_ip);
                 /* Do search */
                 x = SearchFilterLDAP(&edui_ldap, NULL);
                 if (x < 0) {
                     debug("SearchFilterLDAP() -> %s\n", ErrLDAP(x));
-                    local_printfx("ERR (SearchFilterLDAP: %s)\n", ErrLDAP(x));
+                    local_printfx("ERR message=\"(SearchFilterLDAP: %s)\"\n", ErrLDAP(x));
                 } else {
                     edui_ldap.err = -1;
                     debug("SearchFilterLDAP(-, NULL) -> Length: %u\n", x);
                     x = SearchLDAP(&edui_ldap, edui_ldap.scope, edui_ldap.search_filter, (char **) &search_attrib);
                     if (x != LDAP_ERR_SUCCESS) {
                         debug("SearchLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(x));
-                        local_printfx("ERR (SearchLDAP: %s)\n", ErrLDAP(x));
+                        local_printfx("ERR message=\"(SearchLDAP: %s)\"\n", ErrLDAP(x));
                     } else {
                         edui_ldap.err = -1;
                         debug("SearchLDAP(-, %d, %s, -) -> %s\n", edui_conf.scope, edui_ldap.search_filter, ErrLDAP(x));
                         x = SearchIPLDAP(&edui_ldap);
                         if (x != LDAP_ERR_SUCCESS) {
                             debug("SearchIPLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
-                            local_printfx("ERR (SearchIPLDAP: %s)\n", ErrLDAP(x));
+                            local_printfx("ERR message=\"(SearchIPLDAP: %s)\"\n", ErrLDAP(x));
                         } else {
                             debug("SearchIPLDAP(-, %s) -> %s\n", edui_ldap.userid, ErrLDAP(x));
                             local_printfx("OK user=%s\n", edui_ldap.userid);				/* Got a userid --> OK user=<userid> */

=== modified file 'src/HelperReply.cc'
--- src/HelperReply.cc	2012-10-30 00:13:18 +0000
+++ src/HelperReply.cc	2012-11-24 14:28:54 +0000
@@ -1,11 +1,24 @@
+/*
+ * DEBUG: section 84    Helper process maintenance
+ * AUTHOR: Amos Jeffries
+ */
 #include "squid.h"
+#include "ConfigParser.h"
 #include "HelperReply.h"
 #include "helper.h"
+#include "rfc1738.h"
+#include "SquidString.h"
 
-HelperReply::HelperReply(const char *buf, size_t len) :
+HelperReply::HelperReply(char *buf, size_t len) :
         result(HelperReply::Unknown),
         whichServer(NULL)
 {
+    parse(buf,len);
+}
+
+void
+HelperReply::parse(char *buf, size_t len)
+{
     // check we have something to parse
     if (!buf || len < 1) {
         // for now ensure that legacy handlers are not presented with NULL strings.
@@ -14,7 +27,7 @@
         return;
     }
 
-    const char *p = buf;
+    char *p = buf;
 
     // optimization: do not consider parsing result code if the response is short.
     // URL-rewriter may return relative URLs or empty response for a large portion
@@ -35,13 +48,50 @@
             // NTLM challenge token
             result = HelperReply::TT;
             p+=3;
+            // followed by an auth token
+            char *w1 = strwordtok(NULL, &p);
+            if (w1 != NULL) {
+                MemBuf authToken;
+                authToken.init();
+                authToken.append(w1, strlen(w1));
+                responseKeys.add("token",authToken.content());
+            } else {
+                // token field is mandatory on this response code
+                result = HelperReply::BrokenHelper;
+                responseKeys.add("message","Missing 'token' data");
+            }
+
         } else if (!strncmp(p,"AF ",3)) {
-            // NTLM OK response
-            result = HelperReply::AF;
+            // NTLM/Negotate OK response
+            result = HelperReply::Okay;
             p+=3;
+            // followed by:
+            //  an optional auth token and user field
+            // or, an optional username field
+            char *w1 = strwordtok(NULL, &p);
+            char *w2 = strwordtok(NULL, &p);
+            if (w2 != NULL) {
+                // Negotiate "token user"
+                MemBuf authToken;
+                authToken.init();
+                authToken.append(w1, strlen(w1));
+                responseKeys.add("token",authToken.content());
+
+                MemBuf user;
+                user.init();
+                user.append(w2,strlen(w2));
+                responseKeys.add("user",user.content());
+
+            } else if (w1 != NULL) {
+                // NTLM "user"
+                MemBuf user;
+                user.init();
+                user.append(w1,strlen(w1));
+                responseKeys.add("user",user.content());
+            }
         } else if (!strncmp(p,"NA ",3)) {
             // NTLM fail-closed ERR response
-            result = HelperReply::NA;
+            result = HelperReply::Error;
             p+=3;
         }
 
@@ -54,6 +104,48 @@
 
     // NULL-terminate so the helper callback handlers do not buffer-overrun
     other_.terminate();
+
+    parseResponseKeys();
+
+    // Hack for backward-compatibility: BH used to be a text message...
+    if (other().hasContent() && result == HelperReply::BrokenHelper) {
+        responseKeys.add("message",other().content());
+        modifiableOther().clean();
+    }
+}
+
+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());
+
+        // 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);
+        String value = v;
+
+        responseKeys.add(key, value);
+
+        modifiableOther().consume(p - other().content());
+        modifiableOther().consumeWhitespacePrefix();
+    }
 }
 
 std::ostream &
@@ -73,17 +165,22 @@
     case HelperReply::TT:
         os << "TT";
         break;
-    case HelperReply::AF:
-        os << "AF";
-        break;
-    case HelperReply::NA:
-        os << "NA";
-        break;
     case HelperReply::Unknown:
         os << "Unknown";
         break;
     }
 
+    // dump the helper key=pair "notes" list
+    if (r.responseKeys.notes.size() > 0) {
+        os << ", notes={";
+        for (Notes::NotesList::const_iterator m = r.responseKeys.notes.begin(); m != r.responseKeys.notes.end(); ++m) {
+            for (Note::Values::iterator v = (*m)->values.begin(); v != (*m)->values.end(); ++v) {
+                os << ',' << (*m)->key << '=' << ConfigParser::QuoteString((*v)->value);
+            }
+        }
+        os << "}";
+    }
+
     if (r.other().hasContent())
         os << ", other: \"" << r.other().content() << '\"';
 

=== modified file 'src/HelperReply.h'
--- src/HelperReply.h	2012-10-29 23:12:04 +0000
+++ src/HelperReply.h	2012-11-10 06:07:52 +0000
@@ -3,6 +3,7 @@
 
 #include "base/CbcPointer.h"
 #include "MemBuf.h"
+#include "Notes.h"
 
 #if HAVE_OSTREAM
 #include <ostream>
@@ -23,8 +24,14 @@
     HelperReply &operator =(const HelperReply &r);
 
 public:
+    HelperReply() : result(HelperReply::Unknown), responseKeys(), whichServer(NULL) {
+        other_.init(1,1);
+        other_.terminate();
+    }
+
     // create/parse details from the msg buffer provided
-    HelperReply(const char *buf, size_t len);
+    // XXX: buf should be const but parse() needs non-const for now
+    HelperReply(char *buf, size_t len);
 
     const MemBuf &other() const { return other_; }
 
@@ -34,28 +41,38 @@
     /// and by token blob/arg parsing in Negotiate auth handler
     MemBuf &modifiableOther() const { return *const_cast<MemBuf*>(&other_); }
 
+    /** parse a helper response line format:
+     *   line     := [ result ] *#( key-pair )
+     *   key-pair := OWS token '=' ( quoted-string | token )
+     *
+     * 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 failure/negative result
+        Error,        // "ERR" indicating success/negative result
         BrokenHelper, // "BH" indicating failure due to helper internal problems.
 
-        // some result codes for backward compatibility with NTLM/Negotiate
-        // TODO: migrate these into variants of the above results with key-pair parameters
-        TT,
-        AF,
-        NA
+        // result codes for backward compatibility with NTLM/Negotiate
+        // TODO: migrate to a variant of the above results with key-pair parameters
+        TT
     } result;
 
-// TODO other key=pair values. when the callbacks actually use this object.
-// for now they retain their own parsing routines handling other()
+    // list of key=value pairs the helper produced
+    Notes responseKeys;
 
     /// 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_;
 };

=== modified file 'src/MemBuf.cc'
--- src/MemBuf.cc	2012-09-01 14:38:36 +0000
+++ src/MemBuf.cc	2012-11-11 03:47:36 +0000
@@ -220,6 +220,20 @@
     PROF_stop(MemBuf_consume);
 }
 
+/// removes all whitespace prefix bytes and "packs" by moving content left
+void MemBuf::consumeWhitespacePrefix()
+{
+    PROF_start(MemBuf_consumeWhitespace);
+    if (contentSize() > 0) {
+        const char *end = buf + contentSize();
+        const char *p = buf;
+        for(; p<end && xisspace(*p); ++p);
+        if (p-buf > 0)
+            consume(p-buf);
+    }
+    PROF_stop(MemBuf_consumeWhitespace);
+}
+
 // removes last tailSize bytes
 void MemBuf::truncate(mb_size_t tailSize)
 {

=== modified file 'src/MemBuf.h'
--- src/MemBuf.h	2012-10-03 17:32:57 +0000
+++ src/MemBuf.h	2012-11-11 03:49:22 +0000
@@ -81,6 +81,8 @@
     /// \note there is currently no stretch() method to grow without appending
 
     void consume(mb_size_t sz);  // removes sz bytes, moving content left
+    void consumeWhitespacePrefix();    ///< removes all prefix whitespace, moving content left
+
     void append(const char *c, mb_size_t sz); // grows if needed and possible
     void appended(mb_size_t sz); // updates content size after external append
     void truncate(mb_size_t sz);  // removes sz last bytes

=== modified file 'src/Notes.cc'
--- src/Notes.cc	2012-10-26 19:43:58 +0000
+++ src/Notes.cc	2012-11-24 14:34:26 +0000
@@ -73,16 +73,32 @@
 }
 
 Note::Pointer
-Notes::add(const String &noteKey)
+Notes::findByName(const String &noteKey) const
 {
-    typedef Notes::NotesList::iterator AMLI;
+    typedef Notes::NotesList::const_iterator AMLI;
     for (AMLI i = notes.begin(); i != notes.end(); ++i) {
         if ((*i)->key == noteKey)
             return (*i);
     }
 
-    Note::Pointer note = new Note(noteKey);
-    notes.push_back(note);
+    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 = findByName(noteKey);
+    if (note == NULL) {
+        note = new Note(noteKey);
+        notes.push_back(note);
+    }
     return note;
 }
 

=== modified file 'src/Notes.h'
--- src/Notes.h	2012-10-27 00:13:19 +0000
+++ src/Notes.h	2012-11-24 14:34:26 +0000
@@ -61,6 +61,7 @@
 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) {}
@@ -93,6 +94,18 @@
      * returns a pointer to the existing object.
      */
     Note::Pointer add(const String &noteKey);
+
+public:
+    /**
+     * 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);
+
+    /**
+     * Looks up a note by key name and returns a Note::Pointer to it
+     */
+    Note::Pointer findByName(const String &noteKey) const;
 };
 
 class NotePairs : public HttpHeader

=== modified file 'src/auth/digest/UserRequest.cc'
--- src/auth/digest/UserRequest.cc	2012-10-30 00:13:18 +0000
+++ src/auth/digest/UserRequest.cc	2012-11-09 04:15:32 +0000
@@ -280,7 +280,48 @@
     assert(replyData->auth_user_request != NULL);
     Auth::UserRequest::Pointer auth_user_request = replyData->auth_user_request;
 
+    static bool oldHelperWarningDone = false;
     switch (reply.result) {
+    case HelperReply::Unknown: {
+        // Squid 3.3 and older the digest helper only returns a HA1 hash (no "OK")
+        // 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.responseKeys.findByName("ha1");
+        if (ha1Note != NULL) {
+            CvtBin(ha1Note->values[0]->value.termedBuf(), 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());
@@ -289,24 +330,19 @@
         digest_request->user()->credentials(Auth::Failed);
         digest_request->flags.invalid_password = 1;
 
-        if (reply.other().hasContent())
+        Note::Pointer msgNote = reply.responseKeys.findByName("message");
+        if (msgNote != NULL) {
+            digest_request->setDenyMessage(msgNote->values[0]->value.termedBuf());
+        } 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());
-    }
-    break;
-
-    case HelperReply::Unknown: // Squid 3.2 and older the digest helper only returns a HA1 hash (no "OK")
-    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);
-
-        CvtBin(reply.other().content(), digest_user->HA1);
-        digest_user->HA1created = 1;
-    }
-    break;
-
-    default:
-        ; // XXX: handle other states properly.
+            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;

=== modified file 'src/auth/negotiate/UserRequest.cc'
--- src/auth/negotiate/UserRequest.cc	2012-10-30 00:13:18 +0000
+++ src/auth/negotiate/UserRequest.cc	2012-11-06 09:14:41 +0000
@@ -262,37 +262,27 @@
     else
         assert(reply.whichServer == lm_request->authserver);
 
-    /* seperate out the useful data */
-    char *modifiableBlob = reply.modifiableOther().content();
-    char *arg = NULL;
-    if (modifiableBlob && *modifiableBlob != '\0') {
-        arg = strchr(modifiableBlob + 1, ' ');
-        if (arg) {
-            *arg = '\0';
-            ++arg;
-        }
-    }
-    const char *blob = modifiableBlob;
-
     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 = 1;
         if (lm_request->request->flags.proxyKeepalive) {
-            lm_request->server_blob = xstrdup(blob);
+            Note::Pointer tokenNote = reply.responseKeys.findByName("token");
+            lm_request->server_blob = xstrdup(tokenNote->values[0]->value.termedBuf());
             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 blob '" << blob << "'");
+            debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << tokenNote->values[0]->value << "'");
         } else {
             auth_user_request->user()->credentials(Auth::Failed);
-            auth_user_request->denyMessage("NTLM authentication requires a persistent connection");
+            auth_user_request->denyMessage("Negotiate authentication requires a persistent connection");
         }
         break;
 
-    case HelperReply::AF:
     case HelperReply::Okay: {
-        if (arg == NULL) {
+        Note::Pointer userNote = reply.responseKeys.findByName("user");
+        Note::Pointer tokenNote = reply.responseKeys.findByName("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());
@@ -300,10 +290,10 @@
         }
 
         /* we're finished, release the helper */
-        auth_user_request->user()->username(arg);
+        auth_user_request->user()->username(userNote->values[0]->value.termedBuf());
         auth_user_request->denyMessage("Login successful");
         safe_free(lm_request->server_blob);
-        lm_request->server_blob = xstrdup(blob);
+        lm_request->server_blob = xstrdup(tokenNote->values[0]->value.termedBuf());
         lm_request->releaseAuthServer();
 
         /* connection is authenticated */
@@ -332,45 +322,53 @@
          * 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 '" << arg << "'");
+        debugs(29, 4, HERE << "Successfully validated user via Negotiate. Username '" << auth_user_request->user()->username() << "'");
     }
     break;
 
-    case HelperReply::NA:
-    case HelperReply::Error:
-        if (arg == NULL) {
+    case HelperReply::Error: {
+        Note::Pointer messageNote = reply.responseKeys.findByName("message");
+        Note::Pointer tokenNote = reply.responseKeys.findByName("token");
+        if (tokenNote == NULL) {
             /* protocol error */
             fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply.other().content());
             break;
         }
+
         /* authentication failure (wrong password, etc.) */
-        auth_user_request->denyMessage(arg);
+        auth_user_request->denyMessage(messageNote->values[0]->value.termedBuf());
         auth_user_request->user()->credentials(Auth::Failed);
         safe_free(lm_request->server_blob);
-        lm_request->server_blob = xstrdup(blob);
+        lm_request->server_blob = xstrdup(tokenNote->values[0]->value.termedBuf());
         lm_request->releaseAuthServer();
         debugs(29, 4, HERE << "Failed validating user via Negotiate. Error returned '" << reply << "'");
-        break;
+    }
+    break;
 
     case HelperReply::Unknown:
         debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication Helper '" << reply.whichServer << "' crashed!.");
-        blob = "Internal error";
         /* continue to the next case */
 
-    case HelperReply::BrokenHelper:
+    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. */
-        auth_user_request->denyMessage(blob);
+        Note::Pointer errNote = reply.responseKeys.findByName("message");
+        if (reply.result == HelperReply::Unknown)
+            auth_user_request->denyMessage("Internal Error");
+        else if (errNote != NULL)
+            auth_user_request->denyMessage(errNote->values[0]->value.termedBuf());
+        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;
     }
 
-    xfree(arg);
     if (lm_request->request) {
         HTTPMSGUNLOCK(lm_request->request);
         lm_request->request = NULL;

=== modified file 'src/auth/ntlm/UserRequest.cc'
--- src/auth/ntlm/UserRequest.cc	2012-10-30 00:13:18 +0000
+++ src/auth/ntlm/UserRequest.cc	2012-11-06 08:21:31 +0000
@@ -255,34 +255,32 @@
     else
         assert(reply.whichServer == lm_request->authserver);
 
-    /* seperate out the useful data */
-    const char *blob = reply.other().content();
-
     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 = 1;
         if (lm_request->request->flags.proxyKeepalive) {
-            lm_request->server_blob = xstrdup(blob);
+            Note::Pointer serverBlob = reply.responseKeys.findByName("token");
+            lm_request->server_blob = xstrdup(serverBlob->values[0]->value.termedBuf());
             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 blob '" << blob << "'");
+            debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << serverBlob->values[0]->value << "'");
         } else {
             auth_user_request->user()->credentials(Auth::Failed);
             auth_user_request->denyMessage("NTLM authentication requires a persistent connection");
         }
         break;
 
-    case HelperReply::AF:
     case HelperReply::Okay: {
         /* we're finished, release the helper */
-        auth_user_request->user()->username(blob);
+        Note::Pointer userLabel = reply.responseKeys.findByName("user");
+        auth_user_request->user()->username(userLabel->values[0]->value.termedBuf());
         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 '" << blob << "'");
+        debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << userLabel->values[0]->value << "'");
         /* 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
@@ -309,37 +307,47 @@
          * 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 '" << blob << "'");
+        debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << auth_user_request->user()->username() << "'");
     }
     break;
 
-    case HelperReply::NA:
-    case HelperReply::Error:
+    case HelperReply::Error: {
         /* authentication failure (wrong password, etc.) */
-        auth_user_request->denyMessage(blob);
+        Note::Pointer errNote = reply.responseKeys.findByName("message");
+        if (errNote != NULL)
+            auth_user_request->denyMessage(errNote->values[0]->value.termedBuf());
+        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 '" << blob << "'");
-        break;
+        debugs(29, 4, HERE << "Failed validating user via NTLM. Error returned '" << errNote->values[0]->value << "'");
+    }
+    break;
 
     case HelperReply::Unknown:
         debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication Helper '" << reply.whichServer << "' crashed!.");
-        blob = "Internal error";
         /* continue to the next case */
 
-    case HelperReply::BrokenHelper:
+    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. */
-        auth_user_request->denyMessage(blob);
+        Note::Pointer errNote = reply.responseKeys.findByName("message");
+        if (reply.result == HelperReply::Unknown)
+            auth_user_request->denyMessage("Internal Error");
+        else if (errNote != NULL)
+            auth_user_request->denyMessage(errNote->values[0]->value.termedBuf());
+        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;
+    }
+    break;
     }
 
     if (lm_request->request) {

=== modified file 'src/cf.data.pre'
--- src/cf.data.pre	2012-10-29 01:31:29 +0000
+++ src/cf.data.pre	2012-11-11 04:08:33 +0000
@@ -242,9 +242,22 @@
 
 	"program" cmdline
 	Specify the command for the external authenticator.  Such a program
-	reads a line containing "username password" and replies "OK" or
-	"ERR" in an endless loop. "ERR" responses may optionally be followed
-	by a error description available as %m in the returned error page.
+	reads a line containing "username password" and	replies with one of
+	three results:
+
+	  OK
+		the user exists.
+
+	  ERR
+		the user does not exist.
+
+	  BH
+		An internal error occured in the helper, preventing
+		a result being identified.
+
+	"ERR" and "BH" results may optionally be followed by message="..."
+	containing a description available as %m in the returned error page.
+
 	If you use an authenticator, make sure you have 1 acl of type
 	proxy_auth.
 
@@ -315,11 +328,22 @@
 	"program" cmdline
 	Specify the command for the external authenticator.  Such
 	a program reads a line containing "username":"realm" and
-	replies with the appropriate H(A1) value hex encoded or
-	ERR if the user (or his H(A1) hash) does not exists.
-	See rfc 2616 for the definition of H(A1).
-	"ERR" responses may optionally be followed by a error description
-	available as %m in the returned error page.
+	replies with one of three results:
+
+	  OK ha1="..."
+		the user exists. The ha1= key is mandatory and
+		contains the appropriate H(A1) value, hex encoded.
+		See rfc 2616 for the definition of H(A1).
+
+	  ERR
+		the user does not exist.
+
+	  BH
+		An internal error occured in the helper, preventing
+		a result being identified.
+
+	"ERR" and "BH" results may optionally be followed by message="..."
+	containing a description available as %m in the returned error page.
 
 	By default, the digest authentication scheme is not used unless a
 	program is specified.
@@ -401,7 +425,7 @@
 	Such a program reads exchanged NTLMSSP packets with
 	the browser via Squid until authentication is completed.
 	If you use an NTLM authenticator, make sure you have 1 acl
-	of type proxy_auth.  By default, the NTLM authenticator_program
+	of type proxy_auth.  By default, the NTLM authenticator program
 	is not used.
 
 	auth_param ntlm program @DEFAULT_PREFIX@/bin/ntlm_auth
@@ -441,7 +465,7 @@
 	using the Kerberos mechanisms.
 	If you use a Negotiate authenticator, make sure you have at least
 	one acl of type proxy_auth active. By default, the negotiate
-	authenticator_program is not used.
+	authenticator program is not used.
 	The only supported program for this role is the ntlm_auth
 	program distributed as part of Samba, version 4 or later.
 
@@ -619,40 +643,87 @@
 	  %%		The percent sign. Useful for helpers which need
 			an unchanging input format.
 
-	In addition to the above, any string specified in the referencing
-	acl will also be included in the helper request line, after the
-	specified formats (see the "acl external" directive)
-
-	The helper receives lines per the above format specification,
-	and returns lines starting with OK or ERR indicating the validity
-	of the request and optionally followed by additional keywords with
-	more details.
+
+	General request syntax:
+
+	  [channel-ID] FORMAT-values [acl-values ...]
+
+
+	FORMAT-values consists of transaction details expanded with
+	whitespace separation per the config file FORMAT specification
+	using the FORMAT macros listed above.
+
+	acl-values consists of any string specified in the referencing
+	config 'acl ... external' line. see the "acl external" directive.
+
+	Request values sent to the helper are URL escaped to protect
+	each value in requests against whitespaces.
+
+	If using protocol=2.5 then the request sent to the helper is not
+	URL escaped to protect against whitespace.
+
+	NOTE: protocol=3.0 is deprecated as no longer necessary.
+
+	When using the concurrency= option the protocol is changed by
+	introducing a query channel tag in front of the request/response.
+	The query channel tag is a number between 0 and concurrency-1.
+	This value must be echoed back unchanged to Squid as the first part
+	of the response relating to its request.
+
+
+	The helper receives lines expanded per the above format specification
+	and for each input line returns 1 line starting with OK/ERR/BH result
+	code and optionally followed by additional keywords with more details.
+
 
 	General result syntax:
 
-	  OK/ERR keyword=value ...
+	  [channel-ID] result keyword=value ...
+
+	Result consists of one of the codes:
+
+	  OK
+		the ACL test produced a match.
+
+	  ERR
+		the ACL test does not produce a match.
+
+	  BH
+		An internal error occured in the helper, preventing
+		a result being identified.
+
+	The meaning of 'a match' is determined by your squid.conf
+	access control configuration. See the Squid wiki for details.
 
 	Defined keywords:
 
 	  user=		The users name (login)
+
 	  password=	The users password (for login= cache_peer option)
-	  message=	Message describing the reason. Available as %o
-	  		in error pages
-	  tag=		Apply a tag to a request (for both ERR and OK results)
-	  		Only sets a tag, does not alter existing tags.
+
+	  message=	Message describing the reason for this response.
+			Available as %o in error pages.
+			Useful on (ERR and BH results).
+
+	  tag=		Apply a tag to a request. Only sets a tag once,
+			does not alter existing tags.
+
 	  log=		String to be logged in access.log. Available as
-	  		%ea in logformat specifications
-
-	If protocol=3.0 (the default) then URL escaping is used to protect
-	each value in both requests and responses.
-
-	If using protocol=2.5 then all values need to be enclosed in quotes
-	if they may contain whitespace, or the whitespace escaped using \.
-	And quotes or \ characters within the keyword value must be \ escaped.
-
-	When using the concurrency= option the protocol is changed by
-	introducing a query channel tag infront of the request/response.
-	The query channel tag is a number between 0 and concurrency-1.
+	  		%ea in logformat specifications.
+
+	Any keywords may be sent on any response whether OK, ERR or BH.
+
+	All response keyword values need to be a single token with URL
+	escaping, or enclosed in double quotes (") and escaped using \ on
+	any double quotes or \ characters within the value. The wrapping
+	double quotes are removed before the value is interpreted by Squid.
+	\r and \n are also replace by CR and LF.
+
+	Some example key values:
+
+		user=John%20Smith
+		user="John Smith"
+		user="J. \"Bob\" Smith"
 DOC_END
 
 NAME: acl
@@ -4064,19 +4135,54 @@
 
 	For each requested URL, the rewriter will receive on line with the format
 
-	URL <SP> client_ip "/" fqdn <SP> user <SP> method [<SP> kvpairs]<NL>
-
-	In the future, the rewriter interface will be extended with
-	key=value pairs ("kvpairs" shown above).  Rewriter programs
+	  [channel-ID <SP>] URL <SP> client_ip "/" fqdn <SP> user <SP> method [<SP> kv-pairs]<NL>
+
+
+	After processing the request the helper must reply using the following format:
+
+	  [channel-ID <SP>] result [<SP> kv-pairs]
+
+	The result code can be:
+
+	  OK status=30N url="..."
+		Redirect the URL to the one supplied in 'url='.
+		'status=' is optional and contains the status code to send
+		the client in Squids HTTP response. It must be one of the
+		HTTP redirect status codes: 301, 302, 303, 307, 308.
+		When no status is given Squid will use 302.
+
+	  OK rewrite-url="..."
+		Rewrite the URL to the one supplied in 'rewrite-url='.
+		The new URL is fetched directly by Squid and returned to
+		the client as the response to its request.
+
+	  ERR
+		Do not change the URL.
+
+	  BH
+		An internal error occured in the helper, preventing
+		a result being identified.
+
+
+	In the future, the interface protocol will be extended with
+	key=value pairs ("kv-pairs" shown above).  Helper programs
 	should be prepared to receive and possibly ignore additional
 	whitespace-separated tokens on each input line.
 
-	And the rewriter may return a rewritten URL. The other components of
-	the request line does not need to be returned (ignored if they are).
-
-	The rewriter can also indicate that a client-side redirect should
-	be performed to the new URL. This is done by prefixing the returned
-	URL with "301:" (moved permanently) or 302: (moved temporarily), etc.
+	When using the concurrency= option the protocol is changed by
+	introducing a query channel tag in front of the request/response.
+	The query channel tag is a number between 0 and concurrency-1.
+	This value must be echoed back unchanged to Squid as the first part
+	of the response relating to its request.
+
+	WARNING: URL re-writing ability should be avoided whenever possible.
+		 Use the URL redirect form of response instead.
+
+	Re-write creates a difference in the state held by the client
+	and server. Possibly causing confusion when the server response
+	contains snippets of its view state. Embeded URLs, response
+	and content Location headers, etc. are not re-written by this
+	interface.
 
 	By default, a URL rewriter is not used.
 DOC_END

=== modified file 'src/client_side.cc'
--- src/client_side.cc	2012-11-07 19:26:45 +0000
+++ src/client_side.cc	2012-11-24 14:34:26 +0000
@@ -3698,8 +3698,10 @@
 void
 ConnStateData::sslCrtdHandleReply(const HelperReply &reply)
 {
-    if (!reply.other().hasContent()) {
-        debugs(1, DBG_IMPORTANT, HERE << "\"ssl_crtd\" helper return <NULL> reply");
+    if (reply.result == HelperReply::BrokenHelper) {
+        debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " cannot be generated. ssl_crtd response: " << reply);
+    } else if (!reply.other().hasContent()) {
+        debugs(1, DBG_IMPORTANT, HERE << "\"ssl_crtd\" helper returned <NULL> reply.");
     } else {
         Ssl::CrtdMessage reply_message(Ssl::CrtdMessage::REPLY);
         if (reply_message.parse(reply.other().content(), reply.other().contentSize()) != Ssl::CrtdMessage::OK) {

=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc	2012-10-30 00:36:45 +0000
+++ src/client_side_request.cc	2012-11-24 14:34:26 +0000
@@ -902,7 +902,7 @@
     if (answer == ACCESS_ALLOWED)
         redirectStart(http, clientRedirectDoneWrapper, context);
     else {
-        HelperReply nilReply(NULL,0);
+        HelperReply nilReply;
         context->clientRedirectDone(nilReply);
     }
 }

=== modified file 'src/dns.cc'
--- src/dns.cc	2012-10-30 09:16:33 +0000
+++ src/dns.cc	2012-11-08 06:00:57 +0000
@@ -131,7 +131,13 @@
         debugs(34, DBG_IMPORTANT, "dnsSubmit: queue overload, rejecting " << lookup);
 
         const char *t = "$fail Temporary network problem, please retry later";
-        HelperReply failReply(t, strlen(t));
+        HelperReply failReply;
+        /* XXX: upgrade the ipcache and fqdn cache handlers to new syntax
+        failReply.result= HelperReply::BrokenHelper;
+        failReply.responseKeys.add("message","Temporary network problem, please retry later");
+        failReply.responseKeys.add("message","DNS lookup queue overloaded");
+        */
+        failReply.modifiableOther().append(t, strlen(t));
         callback(data, failReply);
         return;
     }

=== modified file 'src/external_acl.cc'
--- src/external_acl.cc	2012-10-29 23:12:04 +0000
+++ src/external_acl.cc	2012-11-10 11:02:45 +0000
@@ -361,10 +361,13 @@
         } else if (strcmp(token, "protocol=2.5") == 0) {
             a->quote = external_acl::QUOTE_METHOD_SHELL;
         } else if (strcmp(token, "protocol=3.0") == 0) {
+            debugs(3, DBG_PARSE_NOTE(2), "WARNING: external_acl_type option protocol=3.0 is deprecated. Remove this from your config.");
             a->quote = external_acl::QUOTE_METHOD_URL;
         } else if (strcmp(token, "quote=url") == 0) {
+            debugs(3, DBG_PARSE_NOTE(2), "WARNING: external_acl_type option quote=url is deprecated. Remove this from your config.");
             a->quote = external_acl::QUOTE_METHOD_URL;
         } else if (strcmp(token, "quote=shell") == 0) {
+            debugs(3, DBG_PARSE_NOTE(2), "WARNING: external_acl_type option quote=shell is deprecated. Use protocol=2.5 if still needed.");
             a->quote = external_acl::QUOTE_METHOD_SHELL;
 
             /* INET6: allow admin to configure some helpers explicitly to
@@ -549,6 +552,9 @@
         if (node->cache)
             storeAppendPrintf(sentry, " cache=%d", node->cache_size);
 
+        if (node->quote == external_acl::QUOTE_METHOD_SHELL)
+            storeAppendPrintf(sentry, " protocol=2.5");
+
         for (format = node->format; format; format = format->next) {
             switch (format->type) {
 
@@ -1302,16 +1308,14 @@
  *
  * Other keywords may be added to the protocol later
  *
- * value needs to be enclosed in quotes if it may contain whitespace, or
- * the whitespace escaped using \ (\ escaping obviously also applies to
- * any " characters)
+ * 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;
-    char *t = NULL;
     ExternalACLEntryData entryData;
     entryData.result = ACCESS_DENIED;
     external_acl_entry *entry = NULL;
@@ -1322,41 +1326,29 @@
         entryData.result = ACCESS_ALLOWED;
     // XXX: handle other non-DENIED results better
 
-    if (reply.other().hasContent()) {
-        char *temp = reply.modifiableOther().content();
-        char *token = strwordtok(temp, &t);
-
-        while ((token = strwordtok(NULL, &t))) {
-            char *value = strchr(token, '=');
-
-            if (value) {
-                *value = '\0';	/* terminate the token, and move up to the value */
-                ++value;
-
-                if (state->def->quote == external_acl::QUOTE_METHOD_URL)
-                    rfc1738_unescape(value);
-
-                if (strcmp(token, "message") == 0)
-                    entryData.message = value;
-                else if (strcmp(token, "error") == 0)
-                    entryData.message = value;
-                else if (strcmp(token, "tag") == 0)
-                    entryData.tag = value;
-                else if (strcmp(token, "log") == 0)
-                    entryData.log = value;
+    // XXX: make entryData store a proper helperReply object.
+
+    Note::Pointer label = reply.responseKeys.findByName("tag");
+    if (label != NULL && label->values[0]->value.size() > 0)
+        entryData.tag = label->values[0]->value;
+
+    label = reply.responseKeys.findByName("message");
+    if (label != NULL && label->values[0]->value.size() > 0)
+        entryData.message = label->values[0]->value;
+
+    label = reply.responseKeys.findByName("log");
+    if (label != NULL && label->values[0]->value.size() > 0)
+        entryData.log = label->values[0]->value;
+
 #if USE_AUTH
-                else if (strcmp(token, "user") == 0)
-                    entryData.user = value;
-                else if (strcmp(token, "password") == 0)
-                    entryData.password = value;
-                else if (strcmp(token, "passwd") == 0)
-                    entryData.password = value;
-                else if (strcmp(token, "login") == 0)
-                    entryData.user = value;
+    label = reply.responseKeys.findByName("user");
+    if (label != NULL && label->values[0]->value.size() > 0)
+        entryData.user = label->values[0]->value;
+
+    label = reply.responseKeys.findByName("password");
+    if (label != NULL && label->values[0]->value.size() > 0)
+        entryData.password = label->values[0]->value;
 #endif
-            }
-        }
-    }
 
     dlinkDelete(&state->list, &state->def->queue);
 

=== modified file 'src/fqdncache.cc'
--- src/fqdncache.cc	2012-10-29 23:12:04 +0000
+++ src/fqdncache.cc	2012-11-10 03:39:05 +0000
@@ -34,6 +34,7 @@
 #include "cbdata.h"
 #include "DnsLookupDetails.h"
 #include "event.h"
+#include "helper.h"
 #include "HelperReply.h"
 #include "Mem.h"
 #include "mgr/Registration.h"

=== modified file 'src/helper.cc'
--- src/helper.cc	2012-10-30 09:16:33 +0000
+++ src/helper.cc	2012-11-24 13:28:50 +0000
@@ -398,7 +398,7 @@
 {
     if (hlp == NULL) {
         debugs(84, 3, "helperSubmit: hlp == NULL");
-        HelperReply nilReply(NULL, 0);
+        HelperReply nilReply;
         callback(data, nilReply);
         return;
     }
@@ -424,7 +424,7 @@
 {
     if (hlp == NULL) {
         debugs(84, 3, "helperStatefulSubmit: hlp == NULL");
-        HelperReply nilReply(NULL, 0);
+        HelperReply nilReply;
         callback(data, nilReply);
         return;
     }
@@ -758,7 +758,7 @@
             void *cbdata;
 
             if (cbdataReferenceValidDone(r->data, &cbdata)) {
-                HelperReply nilReply(NULL, 0);
+                HelperReply nilReply;
                 r->callback(cbdata, nilReply);
             }
 
@@ -824,7 +824,7 @@
         void *cbdata;
 
         if (cbdataReferenceValidDone(r->data, &cbdata)) {
-            HelperReply nilReply(NULL,0);
+            HelperReply nilReply;
             nilReply.whichServer = srv;
             r->callback(cbdata, nilReply);
         }
@@ -942,7 +942,6 @@
             t[-1] = '\0';
 
         *t = '\0';
-        ++t;
 
         if (hlp->childs.concurrency) {
             i = strtol(msg, &msg, 10);
@@ -952,6 +951,8 @@
         }
 
         helperReturnBuffer(i, srv, hlp, msg, t);
+        // only skip off the \0 _after_ passing its location to helperReturnBuffer
+        ++t;
     }
 
     if (Comm::IsConnOpen(srv->readPipe)) {
@@ -1023,10 +1024,16 @@
     if ((t = strchr(srv->rbuf, hlp->eom))) {
         /* end of reply found */
         int called = 1;
+        int skip = 1;
         debugs(84, 3, "helperStatefulHandleRead: end of reply found");
 
-        if (t > srv->rbuf && t[-1] == '\r' && hlp->eom == '\n')
-            t[-1] = '\0';
+        if (t > srv->rbuf && t[-1] == '\r' && hlp->eom == '\n') {
+            *t = '\0';
+            // rewind to the \r octet which is the real terminal now
+            // and remember that we have to skip forward 2 places now.
+            skip = 2;
+            --t;
+        }
 
         *t = '\0';
 
@@ -1038,6 +1045,8 @@
             debugs(84, DBG_IMPORTANT, "StatefulHandleRead: no callback data registered");
             called = 0;
         }
+        // only skip off the \0's _after_ passing its location in HelperReply above
+        t += skip;
 
         srv->flags.busy = 0;
         srv->roffset = 0;
@@ -1366,7 +1375,7 @@
         /* a callback is needed before this request can _use_ a helper. */
         /* we don't care about releasing this helper. The request NEVER
          * gets to the helper. So we throw away the return code */
-        HelperReply nilReply(NULL,0);
+        HelperReply nilReply;
         nilReply.whichServer = srv;
         r->callback(r->data, nilReply);
         /* throw away the placeholder */

=== modified file 'src/redirect.cc'
--- src/redirect.cc	2012-10-30 09:16:33 +0000
+++ src/redirect.cc	2012-11-24 14:34:26 +0000
@@ -151,8 +151,10 @@
     if (Config.onoff.redirector_bypass && redirectors->stats.queue_size) {
         /* Skip redirector if there is one request queued */
         ++n_bypassed;
-        HelperReply nilReply(NULL,0);
-        handler(data, nilReply);
+        HelperReply bypassReply;
+        bypassReply.result = HelperReply::Okay;
+        bypassReply.responseKeys.add("message","URL rewrite/redirect queue too long. Bypassed.");
+        handler(data, bypassReply);
         return;
     }
 

=== modified file 'src/ssl/helper.cc'
--- src/ssl/helper.cc	2012-10-30 09:16:33 +0000
+++ src/ssl/helper.cc	2012-11-08 08:49:10 +0000
@@ -93,8 +93,9 @@
         if (squid_curtime - first_warn > 3 * 60)
             fatal("SSL servers not responding for 3 minutes");
         debugs(34, DBG_IMPORTANT, HERE << "Queue overload, rejecting");
-        const char *errMsg = "BH error 45 Temporary network problem, please retry later"; // XXX: upgrade to message=""
-        HelperReply failReply(errMsg,strlen(errMsg));
+        HelperReply failReply;
+        failReply.result = HelperReply::BrokenHelper;
+        failReply.responseKeys.add("message", "error 45 Temporary network problem, please retry later");
         callback(data, failReply);
         return;
     }