This patch updates the helper reponse callback API from using char*
buffer to a HelperReply object.
* the helper I/O read handler is updated to parse the result code off
the start of the helper response as is currently done for channel-ID.
The callback handlers are altered to use the HelperReply::status instead
of parsing it off themselves individually.
* the remaining I/O read buffer is stored in a MemBuf and callbacks are
updated to use it via the method other().
* the responding helper-server is stored into the HelperReply object and
stateful helper callbacks are combined into the same API as stateless.
The callback handlers are updated to use HelperReply::lastserver instead
of function parameter.
After this patch the helper response format is: [channel-ID] [result]
[blob] <terminator>
The behavour changes expected from this is that all helpers are now able
to send OK/ERR/BH states. Although the handlers for some helpers will
deal with the new states as unknown response. None of the bundled
helpers have been altered to make use of this changed potential.
TODO:
* implement key=value parser for the blob area of the format, and update
handlers to use the HelperReply API to retrieve them.
Amos
=== 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 <ostream>
+#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<helper_stateful_server> 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<Auth::StateData *>(data);
BasicAuthQueueNode *tmpnode;
- char *t = NULL;
void *cbdata;
- debugs(29, 5, HERE << "{" << (reply ? reply : "<NULL>") << "}");
-
- 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<Auth::StateData *>(data);
- char *t = NULL;
- void *cbdata;
- debugs(29, 9, HERE << "{" << (reply ? reply : "<NULL>") << "}");
-
- 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::Digest::UserRequest *>(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::Digest::User
*>(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<Auth::StateData *>(data);
- char *blob, *arg = NULL;
-
- debugs(29, 8, HERE << "helper: '" << lastserver << "' sent us '" << (reply
? reply : "<NULL>") << "'");
+ 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<helper_stateful_server*>(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<Auth::StateData *>(data);
- char *blob;
- debugs(29, 8, HERE << "helper: '" << lastserver << "' sent us '" << (reply
? reply : "<NULL>") << "'");
+ 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<helper_stateful_server*>(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 <NULL> 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<char*>(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<externalAclState *>(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<redirectStateData *>(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<char*>(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
: "<NULL>") << "}");
-
- 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);
