=== modified file 'src/ClientRequestContext.h' --- src/ClientRequestContext.h 2012-05-08 01:13:51 +0000 +++ src/ClientRequestContext.h 2012-06-16 11:23:01 +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-04 09:13:33 +0000 @@ -0,0 +1,97 @@ +#include "squid.h" +#include "HelperReply.h" +#include "helper.h" + +CBDATA_CLASS_INIT(HelperReply); + +HelperReply::HelperReply(const char *buf, size_t len) : + status(HelperReply::Unknown), + lastserver(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)) { + status = HelperReply::OK; + p+=2; + } else if (!strncmp(p,"ERR ",4)) { + status = HelperReply::ERR; + p+=3; + } else if (!strncmp(p,"BH ",3)) { + status = HelperReply::BrokenHelper; + p+=2; + } else if (!strncmp(p,"TT ",3)) { + // NTLM challenge token + status = HelperReply::TT; + p+=2; + } else if (!strncmp(p,"AF ",3)) { + // NTLM OK response + status = HelperReply::AF; + p+=2; + } else if (!strncmp(p,"NA ",3)) { + // NTLM fail-closed ERR response + status = HelperReply::NA; + p+=2; + } else if (!strncmp(p,"LD ",3)) { + // NTLM fail-open OK response + status = HelperReply::LD; + 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 old helper callback handlers do not buffer-overrun + other_.terminate(); +} + +std::ostream & +operator <<(std::ostream &os, const HelperReply &r) +{ + os << "{status="; + switch(r.status) + { + case HelperReply::OK: + os << "OK"; + break; + case HelperReply::ERR: + os << "ERR"; + break; + case HelperReply::BrokenHelper: + os << "BH"; + break; + case HelperReply::TT: + os << "TT"; + break; + case HelperReply::AF: + os << "AF"; + break; + case HelperReply::LD: + os << "LD"; + 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-04 08:18:12 +0000 @@ -0,0 +1,60 @@ +#ifndef _SQUID_SRC_HELPERREPLY_H +#define _SQUID_SRC_HELPERREPLY_H + +#include "base/CbcPointer.h" +#include "cbdata.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 +{ +public: + // create/parse details from the msg buffer provided + HelperReply(const char *buf, size_t len); + HelperReply(); // not defined + HelperReply(const HelperReply &); // not defined + ~HelperReply() {} + + const MemBuf &other() const { return other_; }; + +public: + enum Result { + Unknown, + OK, + ERR, + BrokenHelper, + + // 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, + LD + } status; + +// 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 last server needs to be preserved across callbacks + CbcPointer lastserver; + +private: + /// the remainder of the line + MemBuf other_; + + CBDATA_CLASS2(HelperReply); +}; + +std::ostream &operator <<(std::ostream &os, const HelperReply &r); + +#endif /* _SQUID_SRC_HELPERREPLY_H */ === modified file 'src/Makefile.am' --- src/Makefile.am 2012-05-30 10:25:42 +0000 +++ src/Makefile.am 2012-07-04 10:19:24 +0000 @@ -340,6 +340,8 @@ helper.h \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ HierarchyLogEntry.h \ $(HTCPSOURCE) \ @@ -419,6 +421,7 @@ PingData.h \ protos.h \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ RemovalPolicy.h \ @@ -1309,6 +1312,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ $(HTCPSOURCE) \ http.cc \ HttpBody.h \ @@ -1348,6 +1353,7 @@ peer_sourcehash.cc \ peer_userhash.cc \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ Server.cc \ @@ -1633,6 +1639,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -1678,6 +1686,7 @@ peer_sourcehash.cc \ peer_userhash.cc \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ Server.cc \ @@ -1825,6 +1834,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -1871,6 +1882,7 @@ peer_userhash.cc \ RemovalPolicy.cc \ redirect.cc \ + redirect.h \ refresh.cc \ Server.cc \ $(SNMP_SOURCE) \ @@ -2013,6 +2025,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -2058,6 +2072,7 @@ peer_userhash.cc \ pconn.cc \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ Server.cc \ @@ -2245,6 +2260,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -2285,6 +2302,7 @@ peer_sourcehash.cc \ peer_userhash.cc \ redirect.cc \ + redirect.h \ refresh.cc \ RemovalPolicy.cc \ Server.cc \ @@ -3145,6 +3163,8 @@ helper.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ + HelperReply.cc \ + HelperReply.h \ hier_code.h \ $(HTCPSOURCE) \ http.cc \ @@ -3190,6 +3210,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-06-15 10:48:47 +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-06-23 03:32:16 +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-06-23 03:33:41 +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-04 08:03:02 +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.status == HelperReply::OK) 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-04 08:07:01 +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.status) + { + case HelperReply::ERR: + { /* 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::OK: + { /* 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-04 08:05:35 +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.lastserver << "' 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.lastserver << "'."); 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,22 @@ 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.lastserver.get(); // XXX: no locking? else - assert(lm_request->authserver == lastserver); + assert(lm_request->authserver == reply.lastserver.raw()); /* seperate out the useful data */ - blob = strchr(reply, ' '); - - if (blob) { - blob++; - arg = strchr(blob + 1, ' '); - } else { - arg = NULL; + const char *blob = reply.other().content(); + char *arg = NULL; + if (blob && *blob != '\0') { + const char *temp = strchr(blob + 1, ' '); + arg = xstrdup(++temp); } - if (strncasecmp(reply, "TT ", 3) == 0) { + switch(reply.status) + { + 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 +282,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::OK: + { + 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 +326,31 @@ * 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::ERR: + 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.lastserver << "' crashed!."); + blob = "Internal error"; + + 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 +360,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-06-23 03:32:16 +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-04 08:03:51 +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.lastserver << "' sent us {" << 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.lastserver << "'."); 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.lastserver.get(); // XXX: no locking? else - assert(lm_request->authserver == lastserver); + assert(lm_request->authserver == reply.lastserver.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.status) + { + 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::OK: + { /* we're finished, release the helper */ auth_user_request->user()->username(blob); auth_user_request->denyMessage("Login successful"); @@ -312,15 +310,26 @@ 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::ERR: /* 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: + if (reply.status == HelperReply::Unknown) { + debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication Helper '" << reply.lastserver << "' crashed!."); + blob = "Internal error"; + } + + 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 +340,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-06-23 03:32:16 +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-06-22 03:49:38 +0000 +++ src/client_side.cc 2012-07-04 10:17:04 +0000 @@ -3464,23 +3464,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 " << sslHostName << " is incorrect"); } else { - if (reply_message.getCode() != "OK") { + if (reply.status != HelperReply::OK) { debugs(33, 5, HERE << "Certificate for " << sslHostName << " cannot be generated. ssl_crtd response: " << reply_message.getBody()); } else { debugs(33, 5, HERE << "Certificate for " << sslHostName << " was successfully recieved from ssl_crtd"); === modified file 'src/client_side.h' --- src/client_side.h 2012-04-25 05:29:20 +0000 +++ src/client_side.h 2012-06-16 06:17:29 +0000 @@ -49,6 +49,7 @@ class ClientHttpRequest; class clientStreamNode; class ChunkedCodingParser; +class HelperReply; /** * Badly named. @@ -322,9 +323,9 @@ */ bool 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); bool switchToHttps(const char *host); bool switchedToHttps() const { return switchedToHttps_; } === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2012-05-08 18:14:08 +0000 +++ src/client_side_request.cc 2012-07-04 08:16:20 +0000 @@ -68,11 +68,13 @@ #include "compat/inet_pton.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" @@ -118,7 +120,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; @@ -880,7 +882,7 @@ if (answer == ACCESS_ALLOWED) redirectStart(http, clientRedirectDoneWrapper, context); else - context->clientRedirectDone(NULL); + context->clientRedirectDone(HelperReply(NULL,0)); } void @@ -1166,7 +1168,7 @@ } void -clientRedirectDoneWrapper(void *data, char *result) +clientRedirectDoneWrapper(void *data, const HelperReply &result) { ClientRequestContext *calloutContext = (ClientRequestContext *)data; @@ -1177,14 +1179,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 @@ -1192,11 +1199,17 @@ || 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; - http->redirect.location = xstrdup(t + 1); + if (!strcmp(http->redirect.location, http->uri)) { + // prevent redirector setting up an infinite loop of 302. + xfree(http->redirect.location); + debugs(85, DBG_CRITICAL, "ERROR: URL-rewriter redirects the client back to " << http->uri); + } else { + http->redirect.status = status; + http->redirect.location = xstrdup(t + 1); + } // TODO: validate the URL produced here is RFC 2616 compliant absolute URI } else { debugs(85, DBG_CRITICAL, "ERROR: URL-rewrite produces invalid " << status << " redirect Location: " << result); @@ -1228,6 +1241,9 @@ old_request->method << " " << result << " " << old_request->http_ver); delete new_request; } + } if (!strcmp(result, http->uri)) { + static int temp = 0; + debugs(85, (!(temp++%50)?DBG_CRITICAL:2), "ERROR: URL-rewriter redirects the client back to the same URL. Ignoring reply " << reply); } } === modified file 'src/client_side_request.h' --- src/client_side_request.h 2012-01-20 18:55:04 +0000 +++ src/client_side_request.h 2012-06-16 11:08:38 +0000 @@ -209,8 +209,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-06-15 11:10:38 +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-17 01:19:07 +0000 +++ src/external_acl.cc 2012-07-04 08:39:22 +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.status == HelperReply::OK) + entryData.result = ACCESS_ALLOWED; + // XXX: handle other non-DENIED results better + + if (reply.other().hasContent()) { + char *temp = xstrdup(reply.other().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 */ @@ -1358,12 +1356,14 @@ #endif } } + xfree(temp); } dlinkDelete(&state->list, &state->def->queue); if (cbdataReferenceValid(state->def)) { - if (reply) + // only cache OK and ERR results. + if (reply.status == HelperReply::OK || reply.status == HelperReply::ERR) 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-05 03:05:35 +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-02-29 06:32:14 +0000 +++ src/helper.cc 2012-06-14 04:32:56 +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 nulReply(NULL,0); + nulReply.lastserver = srv; + r->callback(cbdata, nulReply); + } 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.lastserver = 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 nulReply(NULL,0); + nulReply.lastserver = srv; + r->callback(r->data, nulReply); /* 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-06-12 03:31:06 +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-05 03:44:23 +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-04 09:03:13 +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,47 @@ 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.status == 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 (char * res = const_cast(reply.other().content())) { + char *t; + if ((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."); + *t = '\0'; + } + + if (*res == '\0') + res = NULL; + + // The reply passed to us is constant, so we need to duplicate it and relay the duplicate onward. + // The helper is using old format (result 'Unknown') so everything we need is inside the 'res' variable + HelperReply newRes(res,(res?strlen(res):0)); + void *cbdata; + if (cbdataReferenceValidDone(r->data, &cbdata)) + r->handler(cbdata, newRes); + + redirectStateFree(r); + return; // backward compatibility cleanups finished. + } + } + 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 +145,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 +162,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-06-16 11:09:39 +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-06-14 01:41:28 +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-04 22:24:58 +0000 +++ src/typedefs.h 2012-06-16 14:18:49 +0000 @@ -118,7 +118,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; @@ -133,7 +132,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);