=== modified file 'src/ClientRequestContext.h' --- src/ClientRequestContext.h 2012-06-19 21:51:49 +0000 +++ src/ClientRequestContext.h 2012-07-22 06:47:29 +0000 @@ -14,6 +14,8 @@ #include "adaptation/forward.h" #endif +class HelperReply; + class ClientRequestContext : public RefCountable { @@ -32,7 +34,7 @@ void clientAccessCheck2(); void clientAccessCheckDone(const allow_t &answer); void clientRedirectStart(); - void clientRedirectDone(char *result); + void clientRedirectDone(const HelperReply &reply); void checkNoCache(); void checkNoCacheDone(const allow_t &answer); #if USE_ADAPTATION === added file 'src/HelperReply.cc' --- src/HelperReply.cc 1970-01-01 00:00:00 +0000 +++ src/HelperReply.cc 2012-07-10 03:55:13 +0000 @@ -0,0 +1,96 @@ +#include "squid.h" +#include "HelperReply.h" +#include "helper.h" + +#if 0 +HelperReply::HelperReply(const HelperReply &r) : + result(r.result) + other_(r.other()), +{ +} +#endif + +HelperReply::HelperReply(const char *buf, size_t len) : + result(HelperReply::Unknown), + whichServer(NULL) +{ + // check we have something to parse + if (!buf || len < 1) + return; + + const char *p = buf; + + if (len >= 2) { + // NOTE: only increment 'p' if a result code is found. + // some helper formats (digest auth, URL-rewriter) just send a data string + // we must also check for the ' ' character after the response token here + if (!strncmp(p,"OK ",3)) { + result = HelperReply::Okay; + p+=2; + } else if (!strncmp(p,"ERR ",4)) { + result = HelperReply::Error; + p+=3; + } else if (!strncmp(p,"BH ",3)) { + result = HelperReply::BrokenHelper; + p+=2; + } else if (!strncmp(p,"TT ",3)) { + // NTLM challenge token + result = HelperReply::TT; + p+=2; + } else if (!strncmp(p,"AF ",3)) { + // NTLM OK response + result = HelperReply::AF; + p+=2; + } else if (!strncmp(p,"NA ",3)) { + // NTLM fail-closed ERR response + result = HelperReply::NA; + p+=2; + } + + for(;xisspace(*p);p++); // skip whitespace + } + + const mb_size_t blobSize = (buf+len-p); + other_.init(blobSize, blobSize+1); + other_.append(p, blobSize); // remainders of the line. + + // NULL-terminate so the helper callback handlers do not buffer-overrun + other_.terminate(); +} + +std::ostream & +operator <<(std::ostream &os, const HelperReply &r) +{ + os << "{result="; + switch(r.result) + { + case HelperReply::Okay: + os << "OK"; + break; + case HelperReply::Error: + os << "ERR"; + break; + case HelperReply::BrokenHelper: + os << "BH"; + break; + case HelperReply::TT: + os << "TT"; + break; + case HelperReply::AF: + os << "AF"; + break; + case HelperReply::NA: + os << "NA"; + break; + case HelperReply::Unknown: + os << "Unknown"; + break; + } + + if (r.other().hasContent()) + os << ", other: \"" << r.other().content() << '\"'; + + os << '}'; + + return os; +} === added file 'src/HelperReply.h' --- src/HelperReply.h 1970-01-01 00:00:00 +0000 +++ src/HelperReply.h 2012-07-17 05:23:11 +0000 @@ -0,0 +1,67 @@ +#ifndef _SQUID_SRC_HELPERREPLY_H +#define _SQUID_SRC_HELPERREPLY_H + +#include "base/CbcPointer.h" +#include "MemBuf.h" + +#if HAVE_OSTREAM +#include +#endif + +class helper_stateful_server; + +/** + * This object stores the reply message from a helper lookup + * It provides parser routing to accept a raw buffer and process the + * helper reply into fields for easy access by callers + */ +class HelperReply +{ +private: + // implicit creation and copy are prohibited explicitly + HelperReply(); + HelperReply(const HelperReply &r); + HelperReply &operator =(const HelperReply &r); + +public: + // create/parse details from the msg buffer provided + HelperReply(const char *buf, size_t len); + ~HelperReply() {} + + const MemBuf &other() const { return other_; } + + /// backward compatibility: + /// access to modifiable blob, required by redirectHandleReply() + /// and by urlParse() in ClientRequestContext::clientRedirectDone() + /// and by token blob/arg parsing in Negotiate auth handler + MemBuf &modifiableOther() const { return *const_cast(&other_); } + +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 + 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; + +// TODO other key=pair values. when the callbacks actually use this object. +// for now they retain their own parsing routines handling other() + + /// for stateful replies the responding helper 'server' needs to be preserved across callbacks + CbcPointer whichServer; + +private: + /// the remainder of the line + MemBuf other_; +}; + +std::ostream &operator <<(std::ostream &os, const HelperReply &r); + +#endif /* _SQUID_SRC_HELPERREPLY_H */ === modified file 'src/Makefile.am' --- src/Makefile.am 2012-07-18 16:21:47 +0000 +++ src/Makefile.am 2012-07-22 06:47:29 +0000 @@ -340,6 +340,8 @@ helper.h \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ HierarchyLogEntry.h \ $(HTCPSOURCE) \ @@ -420,6 +422,7 @@ PingData.h \ protos.h \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ RemovalPolicy.h \ @@ -1323,6 +1326,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ $(HTCPSOURCE) \ http.cc \ HttpBody.h \ @@ -1362,6 +1367,7 @@ peer_sourcehash.cc \ peer_userhash.cc \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ Server.cc \ @@ -1648,6 +1654,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -1693,6 +1701,7 @@ peer_sourcehash.cc \ peer_userhash.cc \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ Server.cc \ @@ -1840,6 +1849,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -1886,6 +1897,7 @@ peer_userhash.cc \ RemovalPolicy.cc \ redirect.cc \ + redirect.h \ refresh.cc \ Server.cc \ $(SNMP_SOURCE) \ @@ -2028,6 +2040,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -2073,6 +2087,7 @@ peer_userhash.cc \ pconn.cc \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ Server.cc \ @@ -2260,6 +2275,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -2300,6 +2317,7 @@ peer_sourcehash.cc \ peer_userhash.cc \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ Server.cc \ @@ -3164,6 +3182,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -3209,6 +3229,7 @@ peer_sourcehash.cc \ peer_userhash.cc \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ Server.cc \ === modified file 'src/SquidDns.h' --- src/SquidDns.h 2012-01-04 01:22:23 +0000 +++ src/SquidDns.h 2012-07-17 05:21:11 +0000 @@ -1,6 +1,10 @@ #ifndef SQUID_DNS_H #define SQUID_DNS_H +#if USE_DNSHELPER +#include "helper.h" +#endif + namespace Ip { class Address; === modified file 'src/auth/State.h' --- src/auth/State.h 2012-06-19 23:16:13 +0000 +++ src/auth/State.h 2012-07-17 05:21:11 +0000 @@ -5,6 +5,7 @@ #include "auth/UserRequest.h" #include "cbdata.h" +#include "helper.h" namespace Auth { === modified file 'src/auth/UserRequest.h' --- src/auth/UserRequest.h 2012-06-19 23:16:13 +0000 +++ src/auth/UserRequest.h 2012-07-17 05:21:11 +0000 @@ -42,7 +42,7 @@ #include "auth/User.h" #include "dlink.h" #include "ip/Address.h" -#include "typedefs.h" +#include "helper.h" #include "HttpHeader.h" class ConnStateData; === modified file 'src/auth/basic/UserRequest.cc' --- src/auth/basic/UserRequest.cc 2012-06-19 23:16:13 +0000 +++ src/auth/basic/UserRequest.cc 2012-07-17 05:21:11 +0000 @@ -4,6 +4,7 @@ #include "auth/basic/UserRequest.h" #include "auth/State.h" #include "charset.h" +#include "HelperReply.h" #include "rfc1738.h" #include "SquidTime.h" @@ -134,21 +135,12 @@ } void -Auth::Basic::UserRequest::HandleReply(void *data, char *reply) +Auth::Basic::UserRequest::HandleReply(void *data, const HelperReply &reply) { Auth::StateData *r = static_cast(data); BasicAuthQueueNode *tmpnode; - char *t = NULL; void *cbdata; - debugs(29, 5, HERE << "{" << (reply ? reply : "") << "}"); - - if (reply) { - if ((t = strchr(reply, ' '))) - *t++ = '\0'; - - if (*reply == '\0') - reply = NULL; - } + debugs(29, 5, HERE "reply=" << reply); assert(r->auth_user_request != NULL); assert(r->auth_user_request->user()->auth_type == Auth::AUTH_BASIC); @@ -159,13 +151,13 @@ assert(basic_auth != NULL); - if (reply && (strncasecmp(reply, "OK", 2) == 0)) + if (reply.result == HelperReply::Okay) basic_auth->credentials(Auth::Ok); else { basic_auth->credentials(Auth::Failed); - if (t && *t) - r->auth_user_request->setDenyMessage(t); + if (reply.other().hasContent()) + r->auth_user_request->setDenyMessage(reply.other().content()); } basic_auth->expiretime = squid_curtime; === modified file 'src/auth/digest/UserRequest.cc' --- src/auth/digest/UserRequest.cc 2012-06-19 23:16:13 +0000 +++ src/auth/digest/UserRequest.cc 2012-07-17 05:21:11 +0000 @@ -271,25 +271,18 @@ } void -Auth::Digest::UserRequest::HandleReply(void *data, char *reply) +Auth::Digest::UserRequest::HandleReply(void *data, const HelperReply &reply) { Auth::StateData *replyData = static_cast(data); - char *t = NULL; - void *cbdata; - debugs(29, 9, HERE << "{" << (reply ? reply : "") << "}"); - - if (reply) { - if ((t = strchr(reply, ' '))) - *t++ = '\0'; - - if (*reply == '\0' || *reply == '\n') - reply = NULL; - } + debugs(29, 9, HERE << "reply=" << reply); assert(replyData->auth_user_request != NULL); Auth::UserRequest::Pointer auth_user_request = replyData->auth_user_request; - if (reply && (strncasecmp(reply, "ERR", 3) == 0)) { + switch (reply.result) + { + case HelperReply::Error: + { /* allow this because the digest_request pointer is purely local */ Auth::Digest::UserRequest *digest_request = dynamic_cast(auth_user_request.getRaw()); assert(digest_request); @@ -297,17 +290,28 @@ digest_request->user()->credentials(Auth::Failed); digest_request->flags.invalid_password = 1; - if (t && *t) - digest_request->setDenyMessage(t); - } else if (reply) { + if (reply.other().hasContent()) + 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, digest_user->HA1); + CvtBin(reply.other().content(), digest_user->HA1); digest_user->HA1created = 1; } - + break; + + default: + ; // XXX: handle other states properly. + } + + void *cbdata = NULL; if (cbdataReferenceValidDone(replyData->data, &cbdata)) replyData->handler(cbdata); === modified file 'src/auth/negotiate/UserRequest.cc' --- src/auth/negotiate/UserRequest.cc 2012-07-02 12:28:10 +0000 +++ src/auth/negotiate/UserRequest.cc 2012-07-17 05:21:11 +0000 @@ -229,25 +229,18 @@ } void -Auth::Negotiate::UserRequest::HandleReply(void *data, void *lastserver, char *reply) +Auth::Negotiate::UserRequest::HandleReply(void *data, const HelperReply &reply) { Auth::StateData *r = static_cast(data); - char *blob, *arg = NULL; - - debugs(29, 8, HERE << "helper: '" << lastserver << "' sent us '" << (reply ? reply : "") << "'"); + debugs(29, 8, HERE << "helper: '" << reply.whichServer << "' sent us reply=" << reply); if (!cbdataReferenceValid(r->data)) { - debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication invalid callback data. helper '" << lastserver << "'."); + debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication invalid callback data. helper '" << reply.whichServer << "'."); delete r; return; } - if (!reply) { - debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication Helper '" << lastserver << "' crashed!."); - reply = (char *)"BH Internal error"; - } - Auth::UserRequest::Pointer auth_user_request = r->auth_user_request; assert(auth_user_request != NULL); @@ -262,24 +255,24 @@ assert(auth_user_request->user()->auth_type == Auth::AUTH_NEGOTIATE); if (lm_request->authserver == NULL) - lm_request->authserver = static_cast(lastserver); + lm_request->authserver = reply.whichServer.get(); // XXX: no locking? else - assert(lm_request->authserver == lastserver); + assert(lm_request->authserver == reply.whichServer.raw()); /* seperate out the useful data */ - blob = strchr(reply, ' '); - - if (blob) { - blob++; - arg = strchr(blob + 1, ' '); - } else { - arg = NULL; + char *modifiableBlob = reply.modifiableOther().content(); + char *arg = NULL; + if (modifiableBlob && *modifiableBlob != '\0') { + arg = strchr(modifiableBlob + 1, ' '); + if (arg) + *arg++ = '\0'; } + const char *blob = modifiableBlob; - if (strncasecmp(reply, "TT ", 3) == 0) { + switch(reply.result) + { + case HelperReply::TT: /* we have been given a blob to send to the client */ - if (arg) - *arg++ = '\0'; safe_free(lm_request->server_blob); lm_request->request->flags.must_keepalive = 1; if (lm_request->request->flags.proxy_keepalive) { @@ -291,12 +284,18 @@ auth_user_request->user()->credentials(Auth::Failed); auth_user_request->denyMessage("NTLM authentication requires a persistent connection"); } - } else if (strncasecmp(reply, "AF ", 3) == 0 && arg != NULL) { + break; + + case HelperReply::AF: + case HelperReply::Okay: + { + if (arg == NULL) { + // XXX: handle a success with no username better + /* protocol error */ + fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply.other().content()); + break; + } /* we're finished, release the helper */ - - if (arg) - *arg++ = '\0'; - auth_user_request->user()->username(arg); auth_user_request->denyMessage("Login successful"); safe_free(lm_request->server_blob); @@ -329,21 +328,32 @@ * 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 '" << blob << "'"); + debugs(29, 4, HERE << "Successfully validated user via Negotiate. Username '" << arg << "'"); + } + break; - } else if (strncasecmp(reply, "NA ", 3) == 0 && arg != NULL) { + case HelperReply::NA: + case HelperReply::Error: + if (arg == NULL) { + /* protocol error */ + fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply.other().content()); + break; + } /* authentication failure (wrong password, etc.) */ - - if (arg) - *arg++ = '\0'; - auth_user_request->denyMessage(arg); auth_user_request->user()->credentials(Auth::Failed); safe_free(lm_request->server_blob); lm_request->server_blob = xstrdup(blob); lm_request->releaseAuthServer(); - debugs(29, 4, HERE << "Failed validating user via Negotiate. Error returned '" << blob << "'"); - } else if (strncasecmp(reply, "BH ", 3) == 0) { + debugs(29, 4, HERE << "Failed validating user via Negotiate. Error returned '" << reply << "'"); + break; + + case HelperReply::Unknown: + debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication Helper '" << reply.whichServer << "' crashed!."); + blob = "Internal error"; + /* continue to the next case */ + + case HelperReply::BrokenHelper: /* TODO kick off a refresh process. This can occur after a YR or after * a KK. If after a YR release the helper and resubmit the request via * Authenticate Negotiate start. @@ -353,12 +363,10 @@ 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 << "'"); - } else { - /* protocol error */ - fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply); + debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication validating user. Error returned " << reply); } + xfree(arg); lm_request->request = NULL; r->handler(r->data); delete r; === modified file 'src/auth/negotiate/UserRequest.h' --- src/auth/negotiate/UserRequest.h 2012-06-19 23:16:13 +0000 +++ src/auth/negotiate/UserRequest.h 2012-07-17 05:21:11 +0000 @@ -52,7 +52,7 @@ HttpRequest *request; private: - static HLPSCB HandleReply; + static HLPCB HandleReply; }; } // namespace Negotiate === modified file 'src/auth/ntlm/UserRequest.cc' --- src/auth/ntlm/UserRequest.cc 2012-07-02 12:28:10 +0000 +++ src/auth/ntlm/UserRequest.cc 2012-07-17 05:21:11 +0000 @@ -223,24 +223,18 @@ } void -Auth::Ntlm::UserRequest::HandleReply(void *data, void *lastserver, char *reply) +Auth::Ntlm::UserRequest::HandleReply(void *data, const HelperReply &reply) { Auth::StateData *r = static_cast(data); - char *blob; - debugs(29, 8, HERE << "helper: '" << lastserver << "' sent us '" << (reply ? reply : "") << "'"); + debugs(29, 8, HERE << "helper: '" << reply.whichServer << "' sent us reply=" << reply); if (!cbdataReferenceValid(r->data)) { - debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication invalid callback data. helper '" << lastserver << "'."); + debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication invalid callback data. helper '" << reply.whichServer << "'."); delete r; return; } - if (!reply) { - debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication Helper '" << lastserver << "' crashed!."); - reply = (char *)"BH Internal error"; - } - Auth::UserRequest::Pointer auth_user_request = r->auth_user_request; assert(auth_user_request != NULL); @@ -255,16 +249,16 @@ assert(auth_user_request->user()->auth_type == Auth::AUTH_NTLM); if (lm_request->authserver == NULL) - lm_request->authserver = static_cast(lastserver); + lm_request->authserver = reply.whichServer.get(); // XXX: no locking? else - assert(lm_request->authserver == lastserver); + assert(lm_request->authserver == reply.whichServer.raw()); /* seperate out the useful data */ - blob = strchr(reply, ' '); - if (blob) - ++blob; + const char *blob = reply.other().content(); - if (strncasecmp(reply, "TT ", 3) == 0) { + 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.must_keepalive = 1; @@ -277,7 +271,11 @@ auth_user_request->user()->credentials(Auth::Failed); auth_user_request->denyMessage("NTLM authentication requires a persistent connection"); } - } else if (strncasecmp(reply, "AF ", 3) == 0) { + break; + + case HelperReply::AF: + case HelperReply::Okay: + { /* we're finished, release the helper */ auth_user_request->user()->username(blob); auth_user_request->denyMessage("Login successful"); @@ -312,15 +310,25 @@ 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 << "'"); + } + break; - } else if (strncasecmp(reply, "NA ", 3) == 0) { + case HelperReply::NA: + case HelperReply::Error: /* authentication failure (wrong password, etc.) */ auth_user_request->denyMessage(blob); 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 << "'"); - } else if (strncasecmp(reply, "BH ", 3) == 0) { + 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: /* 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. @@ -331,9 +339,7 @@ safe_free(lm_request->server_blob); lm_request->releaseAuthServer(); debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication validating user. Error returned '" << reply << "'"); - } else { - /* protocol error */ - fatalf("authenticateNTLMHandleReply: *** Unsupported helper response ***, '%s'\n", reply); + break; } if (lm_request->request) { === modified file 'src/auth/ntlm/UserRequest.h' --- src/auth/ntlm/UserRequest.h 2012-06-19 23:16:13 +0000 +++ src/auth/ntlm/UserRequest.h 2012-07-17 05:21:11 +0000 @@ -48,7 +48,7 @@ HttpRequest *request; private: - static HLPSCB HandleReply; + static HLPCB HandleReply; }; } // namespace Ntlm === modified file 'src/client_side.cc' --- src/client_side.cc 2012-07-22 03:15:02 +0000 +++ src/client_side.cc 2012-07-22 06:56:18 +0000 @@ -3666,23 +3666,23 @@ } void -ConnStateData::sslCrtdHandleReplyWrapper(void *data, char *reply) +ConnStateData::sslCrtdHandleReplyWrapper(void *data, const HelperReply &reply) { ConnStateData * state_data = (ConnStateData *)(data); state_data->sslCrtdHandleReply(reply); } void -ConnStateData::sslCrtdHandleReply(const char * reply) +ConnStateData::sslCrtdHandleReply(const HelperReply &reply) { - if (!reply) { - debugs(1, 1, HERE << "\"ssl_crtd\" helper return reply"); + if (!reply.other().hasContent()) { + debugs(1, DBG_CRITICAL, "ssl_crtd: helper returned empty reply " << reply); } else { Ssl::CrtdMessage reply_message; - if (reply_message.parse(reply, strlen(reply)) != Ssl::CrtdMessage::OK) { + if (reply_message.parse(reply.other().content(), reply.other().contentSize()) != Ssl::CrtdMessage::OK) { debugs(33, 5, HERE << "Reply from ssl_crtd for " << sslConnectHostOrIp << " is incorrect"); } else { - if (reply_message.getCode() != "OK") { + if (reply.result != HelperReply::Okay) { debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " cannot be generated. ssl_crtd response: " << reply_message.getBody()); } else { debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " was successfully recieved from ssl_crtd"); === modified file 'src/client_side.h' --- src/client_side.h 2012-07-19 00:12:22 +0000 +++ src/client_side.h 2012-07-22 06:47:29 +0000 @@ -52,6 +52,7 @@ class ClientHttpRequest; class clientStreamNode; class ChunkedCodingParser; +class HelperReply; /** * Badly named. @@ -337,9 +338,9 @@ */ void getSslContextDone(SSL_CTX * sslContext, bool isNew = false); /// Callback function. It is called when squid receive message from ssl_crtd. - static void sslCrtdHandleReplyWrapper(void *data, char *reply); + static void sslCrtdHandleReplyWrapper(void *data, const HelperReply &reply); /// Proccess response from ssl_crtd. - void sslCrtdHandleReply(const char * reply); + void sslCrtdHandleReply(const HelperReply &reply); void switchToHttps(HttpRequest *request, Ssl::BumpMode bumpServerMode); bool switchedToHttps() const { return switchedToHttps_; } === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2012-07-19 00:12:22 +0000 +++ src/client_side_request.cc 2012-07-22 06:47:29 +0000 @@ -69,11 +69,13 @@ #include "errorpage.h" #include "fde.h" #include "format/Token.h" +#include "helper.h" #include "HttpHdrCc.h" #include "HttpReply.h" #include "HttpRequest.h" #include "ip/QosConfig.h" #include "MemObject.h" +#include "redirect.h" #include "Store.h" #include "SquidTime.h" #include "wordlist.h" @@ -122,7 +124,7 @@ #endif static int clientHierarchical(ClientHttpRequest * http); static void clientInterpretRequestHeaders(ClientHttpRequest * http); -static RH clientRedirectDoneWrapper; +static HLPCB clientRedirectDoneWrapper; static void checkNoCacheDoneWrapper(allow_t, void *); extern "C" CSR clientGetMoreData; extern "C" CSS clientReplyStatus; @@ -887,7 +889,7 @@ if (answer == ACCESS_ALLOWED) redirectStart(http, clientRedirectDoneWrapper, context); else - context->clientRedirectDone(NULL); + context->clientRedirectDone(HelperReply(NULL,0)); } void @@ -1173,7 +1175,7 @@ } void -clientRedirectDoneWrapper(void *data, char *result) +clientRedirectDoneWrapper(void *data, const HelperReply &result) { ClientRequestContext *calloutContext = (ClientRequestContext *)data; @@ -1184,14 +1186,19 @@ } void -ClientRequestContext::clientRedirectDone(char *result) +ClientRequestContext::clientRedirectDone(const HelperReply &reply) { HttpRequest *old_request = http->request; - debugs(85, 5, "clientRedirectDone: '" << http->uri << "' result=" << (result ? result : "NULL")); + debugs(85, 5, HERE << "'" << http->uri << "' result=" << reply); assert(redirect_state == REDIRECT_PENDING); redirect_state = REDIRECT_DONE; - if (result) { + if (reply.other().hasContent()) { + /* 2012-06-28: This cast is due to urlParse() truncating too-long URLs itself. + * At this point altering the helper buffer in that way is not harmful, but annoying. + * When Bug 1961 is resolved and urlParse has a const API, this needs to die. + */ + char * result = const_cast(reply.other().content()); http_status status = (http_status) atoi(result); if (status == HTTP_MOVED_PERMANENTLY @@ -1199,7 +1206,7 @@ || status == HTTP_SEE_OTHER || status == HTTP_PERMANENT_REDIRECT || status == HTTP_TEMPORARY_REDIRECT) { - char *t = result; + char *t = NULL; if ((t = strchr(result, ':')) != NULL) { http->redirect.status = status; === modified file 'src/client_side_request.h' --- src/client_side_request.h 2012-07-18 16:21:47 +0000 +++ src/client_side_request.h 2012-07-22 06:47:29 +0000 @@ -211,8 +211,6 @@ SQUIDCEXTERN void clientAccessCheck(ClientHttpRequest *); /* ones that should be elsewhere */ -SQUIDCEXTERN void redirectStart(ClientHttpRequest *, RH *, void *); - SQUIDCEXTERN void tunnelStart(ClientHttpRequest *, int64_t *, int *); #if _USE_INLINE_ === modified file 'src/dns.cc' --- src/dns.cc 2012-01-20 18:55:04 +0000 +++ src/dns.cc 2012-07-17 05:21:11 +0000 @@ -39,6 +39,7 @@ #include "SquidTime.h" #include "mgr/Registration.h" #include "helper.h" +#include "HelperReply.h" /* MS VisualStudio Projects are monolitich, so we need the following #if to include the external DNS code in compile process when @@ -128,9 +129,8 @@ if (squid_curtime - first_warn > 3 * 60) fatal("DNS servers not responding for 3 minutes"); - debugs(34, 1, "dnsSubmit: queue overload, rejecting " << lookup); - - callback(data, (char *)"$fail Temporary network problem, please retry later"); + const char *t = "$fail Temporary network problem, please retry later"; + callback(data, HelperReply(t, strlen(t))); return; } === modified file 'src/external_acl.cc' --- src/external_acl.cc 2012-06-19 16:08:52 +0000 +++ src/external_acl.cc 2012-07-22 20:29:08 +0000 @@ -1307,30 +1307,28 @@ * the whitespace escaped using \ (\ escaping obviously also applies to * any " characters) */ - static void -externalAclHandleReply(void *data, char *reply) +externalAclHandleReply(void *data, const HelperReply &reply) { externalAclState *state = static_cast(data); externalAclState *next; - char *status; - char *token; - char *value; char *t = NULL; ExternalACLEntryData entryData; entryData.result = ACCESS_DENIED; external_acl_entry *entry = NULL; - debugs(82, 2, "externalAclHandleReply: reply=\"" << reply << "\""); - - if (reply) { - status = strwordtok(reply, &t); - - if (status && strcmp(status, "OK") == 0) - entryData.result = ACCESS_ALLOWED; + debugs(82, 2, HERE << "reply=" << reply); + + if (reply.result == HelperReply::Okay) + 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))) { - value = strchr(token, '='); + char *value = strchr(token, '='); if (value) { *value++ = '\0'; /* terminate the token, and move up to the value */ @@ -1363,7 +1361,8 @@ dlinkDelete(&state->list, &state->def->queue); if (cbdataReferenceValid(state->def)) { - if (reply) + // only cache OK and ERR results. + if (reply.result == HelperReply::Okay || reply.result == HelperReply::Error) entry = external_acl_cache_add(state->def, state->key, entryData); else { external_acl_entry *oldentry = (external_acl_entry *)hash_lookup(state->def->cache, state->key); === modified file 'src/fqdncache.cc' --- src/fqdncache.cc 2012-03-04 22:24:58 +0000 +++ src/fqdncache.cc 2012-07-17 05:21:11 +0000 @@ -36,6 +36,7 @@ #include "cbdata.h" #include "DnsLookupDetails.h" #include "event.h" +#include "HelperReply.h" #include "mgr/Registration.h" #include "SquidDns.h" #include "SquidTime.h" @@ -494,7 +495,7 @@ */ static void #if USE_DNSHELPER -fqdncacheHandleReply(void *data, char *reply) +fqdncacheHandleReply(void *data, const HelperReply &reply) #else fqdncacheHandleReply(void *data, const rfc1035_rr * answers, int na, const char *error_message) #endif @@ -506,7 +507,7 @@ statCounter.dns.svcTime.count(age); #if USE_DNSHELPER - fqdncacheParse(f, reply); + fqdncacheParse(f, reply.other().content()); #else fqdncacheParse(f, answers, na, error_message); === modified file 'src/helper.cc' --- src/helper.cc 2012-03-07 23:33:53 +0000 +++ src/helper.cc 2012-07-17 05:21:11 +0000 @@ -383,7 +383,7 @@ { if (hlp == NULL) { debugs(84, 3, "helperSubmit: hlp == NULL"); - callback(data, NULL); + callback(data, HelperReply(NULL,0)); return; } @@ -404,11 +404,11 @@ /// lastserver = "server last used as part of a reserved request sequence" void -helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, void *data, helper_stateful_server * lastserver) +helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPCB * callback, void *data, helper_stateful_server * lastserver) { if (hlp == NULL) { debugs(84, 3, "helperStatefulSubmit: hlp == NULL"); - callback(data, 0, NULL); + callback(data, HelperReply(NULL,0)); return; } @@ -728,11 +728,12 @@ } for (i = 0; i < concurrency; i++) { + // XXX: re-schedule these on another helper? if ((r = srv->requests[i])) { void *cbdata; if (cbdataReferenceValidDone(r->data, &cbdata)) - r->callback(cbdata, NULL); + r->callback(cbdata, HelperReply(NULL,0)); helperRequestFree(r); @@ -791,8 +792,11 @@ if ((r = srv->request)) { void *cbdata; - if (cbdataReferenceValidDone(r->data, &cbdata)) - r->callback(cbdata, srv, NULL); + if (cbdataReferenceValidDone(r->data, &cbdata)) { + HelperReply nilReply(NULL,0); + nilReply.whichServer = srv; + r->callback(cbdata, nilReply); + } helperStatefulRequestFree(r); @@ -808,10 +812,14 @@ } /// Calls back with a pointer to the buffer with the helper output -static void helperReturnBuffer(int request_number, helper_server * srv, helper * hlp, char * msg, char * msg_end) +static void +helperReturnBuffer(int request_number, helper_server * srv, helper * hlp, char * msg, char * msg_end) { helper_request *r = srv->requests[request_number]; if (r) { +// TODO: parse the reply into new helper reply object +// pass that to the callback instead of msg + HLPCB *callback = r->callback; srv->requests[request_number] = NULL; @@ -820,7 +828,7 @@ void *cbdata = NULL; if (cbdataReferenceValidDone(r->data, &cbdata)) - callback(cbdata, msg); + callback(cbdata, HelperReply(msg, (msg_end-msg))); srv->stats.pending--; @@ -989,7 +997,9 @@ *t = '\0'; if (r && cbdataReferenceValid(r->data)) { - r->callback(r->data, srv, srv->rbuf); + HelperReply res(srv->rbuf, (t - srv->rbuf)); + res.whichServer = srv; + r->callback(r->data, res); } else { debugs(84, 1, "StatefulHandleRead: no callback data registered"); called = 0; @@ -1320,11 +1330,13 @@ /* 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 */ - r->callback(r->data, srv, NULL); + HelperReply nilReply(NULL,0); + nilReply.whichServer = srv; + r->callback(r->data, nilReply); /* throw away the placeholder */ helperStatefulRequestFree(r); /* and push the queue. Note that the callback may have submitted a new - * request to the helper which is why we test for the request*/ + * request to the helper which is why we test for the request */ if (srv->request == NULL) helperStatefulServerDone(srv); === modified file 'src/helper.h' --- src/helper.h 2012-01-20 18:55:04 +0000 +++ src/helper.h 2012-07-17 05:21:11 +0000 @@ -39,10 +39,11 @@ #include "comm/forward.h" #include "ip/Address.h" #include "HelperChildConfig.h" +#include "HelperReply.h" class helper_request; -typedef void HLPSCB(void *, void *lastserver, char *buf); +typedef void HLPCB(void *, const HelperReply &reply); class helper { @@ -192,7 +193,7 @@ public: MEMPROXY_CLASS(helper_stateful_request); char *buf; - HLPSCB *callback; + HLPCB *callback; int placeholder; /* if 1, this is a dummy request waiting for a stateful helper to become available */ void *data; }; @@ -203,7 +204,7 @@ SQUIDCEXTERN void helperOpenServers(helper * hlp); SQUIDCEXTERN void helperStatefulOpenServers(statefulhelper * hlp); SQUIDCEXTERN void helperSubmit(helper * hlp, const char *buf, HLPCB * callback, void *data); -SQUIDCEXTERN void helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, void *data, helper_stateful_server * lastserver); +SQUIDCEXTERN void helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPCB * callback, void *data, helper_stateful_server * lastserver); SQUIDCEXTERN void helperStats(StoreEntry * sentry, helper * hlp, const char *label = NULL); SQUIDCEXTERN void helperStatefulStats(StoreEntry * sentry, statefulhelper * hlp, const char *label = NULL); SQUIDCEXTERN void helperShutdown(helper * hlp); === modified file 'src/ipcache.cc' --- src/ipcache.cc 2012-03-04 22:24:58 +0000 +++ src/ipcache.cc 2012-07-17 05:21:11 +0000 @@ -590,7 +590,7 @@ /// \ingroup IPCacheInternal static void #if USE_DNSHELPER -ipcacheHandleReply(void *data, char *reply) +ipcacheHandleReply(void *data, const HelperReply &reply) #else ipcacheHandleReply(void *data, const rfc1035_rr * answers, int na, const char *error_message) #endif @@ -602,7 +602,7 @@ statCounter.dns.svcTime.count(age); #if USE_DNSHELPER - ipcacheParse(i, reply); + ipcacheParse(i, reply.other().content()); #else int done = ipcacheParse(i, answers, na, error_message); === modified file 'src/redirect.cc' --- src/redirect.cc 2012-01-20 18:55:04 +0000 +++ src/redirect.cc 2012-07-17 05:21:11 +0000 @@ -46,7 +46,7 @@ #include "HttpRequest.h" #include "client_side.h" #include "client_side_reply.h" -#include "helper.h" +#include "redirect.h" #include "rfc1738.h" #if USE_SSL #include "ssl/support.h" @@ -62,7 +62,7 @@ Ip::Address client_addr; const char *client_ident; const char *method_s; - RH *handler; + HLPCB *handler; } redirectStateData; static HLPCB redirectHandleReply; @@ -73,21 +73,36 @@ CBDATA_TYPE(redirectStateData); static void -redirectHandleReply(void *data, char *reply) +redirectHandleReply(void *data, const HelperReply &reply) { redirectStateData *r = static_cast(data); - char *t; + debugs(61, 5, HERE << "reply=" << reply); + + // XXX: This funtion is now kept only to check for and display this garbage use-case + // it can be removed when the helpers are all updated to the normalized "OK/ERR key-pairs" format + + if (reply.result == HelperReply::Unknown) { + // BACKWARD COMPATIBILITY 2012-06-15: + // Some nasty old helpers send back the entire input line including extra format keys. + // This is especially bad for simple perl search-replace filter scripts. + // + // * trim all but the first word off the response. + // * warn once every 50 responses that this will stop being fixed-up soon. + // + if (const char * res = reply.other().content()) { + if (const char *t = strchr(res, ' ')) { + static int warn = 0; + debugs(61, (!(warn++%50)? DBG_CRITICAL:2), "UPGRADE WARNING: URL rewriter reponded with garbage '" << t << + "'. Future Squid will treat this as part of the URL."); + const mb_size_t garbageLength = reply.other().contentSize() - (t-res); + reply.modifiableOther().truncate(garbageLength); + } + if (reply.other().hasContent() && *res == '\0') + reply.modifiableOther().clean(); // drop the whole buffer of garbage. + } + } + void *cbdata; - debugs(61, 5, "redirectHandleRead: {" << (reply && *reply != '\0' ? reply : "") << "}"); - - if (reply) { - if ((t = strchr(reply, ' '))) - *t = '\0'; - - if (*reply == '\0') - reply = NULL; - } - if (cbdataReferenceValidDone(r->data, &cbdata)) r->handler(cbdata, reply); @@ -119,7 +134,7 @@ /**** PUBLIC FUNCTIONS ****/ void -redirectStart(ClientHttpRequest * http, RH * handler, void *data) +redirectStart(ClientHttpRequest * http, HLPCB * handler, void *data) { ConnStateData * conn = http->getConn(); redirectStateData *r = NULL; @@ -136,7 +151,7 @@ if (Config.onoff.redirector_bypass && redirectors->stats.queue_size) { /* Skip redirector if there is one request queued */ n_bypassed++; - handler(data, NULL); + handler(data, HelperReply(NULL,0)); return; } === added file 'src/redirect.h' --- src/redirect.h 1970-01-01 00:00:00 +0000 +++ src/redirect.h 2012-07-10 03:55:14 +0000 @@ -0,0 +1,10 @@ +#ifndef _SQUID_SRC_REDIRECT_H +#define _SQUID_SRC_REDIRECT_H + +#include "helper.h" + +class ClientHttpRequest; + +extern void redirectStart(ClientHttpRequest *, HLPCB *, void *); + +#endif /* _SQUID_SRC_REDIRECT_H */ === modified file 'src/tests/stub_helper.cc' --- src/tests/stub_helper.cc 2012-01-20 18:55:04 +0000 +++ src/tests/stub_helper.cc 2012-07-17 05:21:14 +0000 @@ -5,7 +5,7 @@ #include "tests/STUB.h" void helperSubmit(helper * hlp, const char *buf, HLPCB * callback, void *data) STUB -void helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPSCB * callback, void *data, helper_stateful_server * lastserver) STUB +void helperStatefulSubmit(statefulhelper * hlp, const char *buf, HLPCB * callback, void *data, helper_stateful_server * lastserver) STUB helper::~helper() STUB CBDATA_CLASS_INIT(helper); === modified file 'src/typedefs.h' --- src/typedefs.h 2012-03-07 23:33:53 +0000 +++ src/typedefs.h 2012-07-22 06:47:29 +0000 @@ -124,7 +124,6 @@ #include "anyp/ProtocolType.h" typedef void IRCB(struct peer *, peer_t, AnyP::ProtocolType, void *, void *data); -typedef void RH(void *data, char *); /* in wordlist.h */ class wordlist; @@ -139,7 +138,6 @@ typedef void OBJH(StoreEntry *); typedef void SIGHDLR(int sig); typedef void STVLDCB(void *, int, int); -typedef void HLPCB(void *, char *buf); typedef int HLPSAVAIL(void *); typedef void HLPSONEQ(void *); typedef void HLPCMDOPTS(int *argc, char **argv);