=== 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= */ @@ -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= */ === 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 @@ -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(&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 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 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 ¬eKey) +Notes::findByName(const String ¬eKey) 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 ¬eKey, const String ¬eValue) +{ + Note::Pointer key = add(noteKey); + key->addValue(noteValue); +} + +Note::Pointer +Notes::add(const String ¬eKey) +{ + 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 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 ¬eKey); + +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 ¬eKey, const String ¬eValue); + + /** + * Looks up a note by key name and returns a Note::Pointer to it + */ + Note::Pointer findByName(const String ¬eKey) 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_user_request->user().getRaw()); + assert(digest_user != NULL); + + CvtBin(reply.other().content(), digest_user->HA1); + digest_user->HA1created = 1; + } + break; + + case HelperReply::Okay: { + /* allow this because the digest_request pointer is purely local */ + Auth::Digest::User *digest_user = dynamic_cast(auth_user_request->user().getRaw()); + assert(digest_user != NULL); + + Note::Pointer ha1Note = reply.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_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_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 client_ip "/" fqdn user method [ kvpairs] - - In the future, the rewriter interface will be extended with - key=value pairs ("kvpairs" shown above). Rewriter programs + [channel-ID ] URL client_ip "/" fqdn user method [ kv-pairs] + + + After processing the request the helper must reply using the following format: + + [channel-ID ] result [ 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 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 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(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; }