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 72ad39cc9e5725d9b6417cd896d53af2361819d3..8653c51b4349412049d76093512fc8e5d3eb9578 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -1914,14 +1914,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
@@ -2224,7 +2262,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);
@@ -2395,6 +2432,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);
         switch (hp->request_parse_status) {
@@ -2416,6 +2455,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);
@@ -2433,6 +2474,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);
@@ -2448,6 +2491,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 169dfa2f0c0f034d27fad14d4b0e63296767d28e..3e8123894947a7d4b092fbb69c607eda8ecc6bb8 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -321,7 +321,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);