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 9d42444744003224d9431256ac7577c620b6b0c5..b0f04e9015899529944f33c1651d76352a514eeb 100644 (file)
--- 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<char*>(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<clientReplyContext *>(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<clientReplyContext *>(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<clientReplyContext *>(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<clientReplyContext *>(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 6fa187b11a954347e2694061461463c7ad48c934..3636317c456214d0bafd5fd21b94d07e444bf142 100644 (file)
--- 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);