From: Christos Tsantilas Date: Sun, 3 Apr 2011 11:18:30 +0000 (-0600) Subject: Apply uri_whitespace before logging malformed requests X-Git-Tag: SQUID_3_1_12~7 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=96102714bc2413349009eceee7bac875740083cf;p=thirdparty%2Fsquid.git Apply uri_whitespace before logging malformed requests This patch try to implement the first option from those described at the squid-dev thread with subject "Request URI logging for malformed requests": http://www.squid-cache.org/mail-archive/squid-dev/201101/0004.html Currently the logged URI set using the setLogUri method (in client_side.cc and client_side_reply.cc files). Also the setLogUri called at least two times for every normal request. Moreover the setLogUri always check if the URI contains characters which must escaped which in the case of normal requests it is not needed because urlCanonicalClean always used before pass the URI to setLogUri. This patch: - add a parameter to the setLogUri to say if the URI must cleaned and the uri_whitespace filtering must applied. - Remove the setLogUri call from the parseHttpRequest. - Call in all cases (HTTP request error or not) the setLogUri in clientProcessRequest - In the case the URL is not a valid HTTP request applies the uri_whitespace filtering. - In the case the URI is valid the uri_whitespace filtering is not required because it is already applied by the urpParse function. This is a Measurement Factory project --- diff --git a/src/client_side.cc b/src/client_side.cc index 9d42444744..b0f04e9015 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1775,14 +1775,52 @@ findTrailingHTTPVersion(const char *uriAndHTTPVersion, const char *end) } void -setLogUri(ClientHttpRequest * http, char const *uri) +setLogUri(ClientHttpRequest * http, char const *uri, bool cleanUrl) { safe_free(http->log_uri); - if (!stringHasCntl(uri)) + if (!cleanUrl) + // The uri is already clean just dump it. http->log_uri = xstrndup(uri, MAX_URL); - else - http->log_uri = xstrndup(rfc1738_escape_unescaped(uri), MAX_URL); + else { + int flags = 0; + switch (Config.uri_whitespace) { + case URI_WHITESPACE_ALLOW: + flags |= RFC1738_ESCAPE_NOSPACE; + + case URI_WHITESPACE_ENCODE: + flags |= RFC1738_ESCAPE_UNESCAPED; + http->log_uri = xstrndup(rfc1738_do_escape(uri, flags), MAX_URL); + break; + + case URI_WHITESPACE_CHOP: { + flags |= RFC1738_ESCAPE_NOSPACE; + flags |= RFC1738_ESCAPE_UNESCAPED; + http->log_uri = xstrndup(rfc1738_do_escape(uri, flags), MAX_URL); + int pos = strcspn(http->log_uri, w_space); + http->log_uri[pos] = '\0'; + } + break; + + case URI_WHITESPACE_DENY: + case URI_WHITESPACE_STRIP: + default: { + const char *t; + char *tmp_uri = static_cast(xmalloc(strlen(uri) + 1)); + char *q = tmp_uri; + t = uri; + while (*t) { + if (!xisspace(*t)) + *q++ = *t; + t++; + } + *q = '\0'; + http->log_uri = xstrndup(rfc1738_escape_unescaped(tmp_uri), MAX_URL); + xfree(tmp_uri); + } + break; + } + } } static void @@ -2133,7 +2171,6 @@ parseHttpRequest(ConnStateData *conn, HttpParser *hp, HttpRequestMethod * method strcpy(http->uri, url); } - setLogUri(http, http->uri); debugs(33, 5, "parseHttpRequest: Complete request received"); result->flags.parsed_ok = 1; xfree(url); @@ -2324,6 +2361,8 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c if (context->flags.parsed_ok == 0) { clientStreamNode *node = context->getClientReplyContext(); debugs(33, 1, "clientProcessRequest: Invalid Request"); + // setLogUri should called before repContext->setReplyToError + setLogUri(http, http->uri, true); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); repContext->setReplyToError(ERR_INVALID_REQ, HTTP_BAD_REQUEST, method, NULL, conn->peer, NULL, conn->in.buf, NULL); @@ -2336,6 +2375,8 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c if ((request = HttpRequest::CreateFromUrlAndMethod(http->uri, method)) == NULL) { clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Invalid URL: " << http->uri); + // setLogUri should called before repContext->setReplyToError + setLogUri(http, http->uri, true); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); repContext->setReplyToError(ERR_INVALID_URL, HTTP_BAD_REQUEST, method, http->uri, conn->peer, NULL, NULL, NULL); @@ -2353,6 +2394,8 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Unsupported HTTP version discovered. :\n" << HttpParserHdrBuf(hp)); + // setLogUri should called before repContext->setReplyToError + setLogUri(http, http->uri, true); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); repContext->setReplyToError(ERR_UNSUP_HTTPVERSION, HTTP_HTTP_VERSION_NOT_SUPPORTED, method, http->uri, conn->peer, NULL, HttpParserHdrBuf(hp), NULL); @@ -2368,6 +2411,8 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c if (http_ver.major >= 1 && !request->parseHeader(HttpParserHdrBuf(hp), HttpParserHdrSz(hp))) { clientStreamNode *node = context->getClientReplyContext(); debugs(33, 5, "Failed to parse request headers:\n" << HttpParserHdrBuf(hp)); + // setLogUri should called before repContext->setReplyToError + setLogUri(http, http->uri, true); clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); repContext->setReplyToError(ERR_INVALID_REQ, HTTP_BAD_REQUEST, method, http->uri, conn->peer, NULL, NULL, NULL); diff --git a/src/client_side.h b/src/client_side.h index 6fa187b11a..3636317c45 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -289,7 +289,7 @@ private: /* convenience class while splitting up body handling */ /* temporary existence only - on stack use expected */ -void setLogUri(ClientHttpRequest * http, char const *uri); +void setLogUri(ClientHttpRequest * http, char const *uri, bool cleanUrl = false); const char *findTrailingHTTPVersion(const char *uriAndHTTPVersion, const char *end = NULL);