Patch ported from 2.5 branch. Various checks on HTTP header compliance,

for both requests and responses.

diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc
index d587cf62f41e2282a0f5f9d010eda7f062d2be37..e6681cac189d1d635fa015a070c1488f7901d9b7 100644 (file)
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpHeader.cc,v 1.98 2004/09/26 16:38:01 hno Exp $
+ * $Id: HttpHeader.cc,v 1.99 2004/11/16 23:11:46 wessels Exp $
  *
  * DEBUG: section 55    HTTP Header
  * AUTHOR: Alex Rousskov
@@ -496,15 +496,25 @@ httpHeaderParse(HttpHeader * hdr, const char *header_start, const char *header_e
 
         e = httpHeaderEntryParseCreate(field_start, field_end);
 
-        if (e != NULL)
-            httpHeaderAddEntry(hdr, e);
-        else
-            debug(55, 2) ("warning: ignoring unparseable http header field near '%s'\n",
+        if (NULL == e) {
+            debug(55, 1) ("WARNING: ignoring unparseable HTTP header field near '%s'\n",
                           getStringPrefix(field_start, field_end));
+        } else if (e->id == HDR_CONTENT_LENGTH && httpHeaderHas(hdr, HDR_CONTENT_LENGTH)) {
+            debug(55, 1) ("WARNING: found double content-length header\n");
+            httpHeaderEntryDestroy(e);
+            return httpHeaderReset(hdr);
+        } else if (e->id == HDR_OTHER && stringHasWhitespace(e->name.buf())) {
+            debug(55, 1) ("WARNING: found whitespace in HTTP header {%s}\n", e->name.buf());
+            httpHeaderEntryDestroy(e);
+            return httpHeaderReset(hdr);
+        } else {
+            httpHeaderAddEntry(hdr, e);
+        }
 
         field_start = field_end;
 
         /* skip CRLF */
+
         if (*field_start == '\r')
             field_start++;
 

diff --git a/src/HttpVersion.h b/src/HttpVersion.h
index de26ff74e897b2c7da393b436aa2d5980a4e956b..a33f74c9f08c2228c0757bd2fd8a986b386f5579 100644 (file)
--- a/src/HttpVersion.h
+++ b/src/HttpVersion.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpVersion.h,v 1.1 2003/09/01 03:49:37 robertc Exp $
+ * $Id: HttpVersion.h,v 1.2 2004/11/16 23:11:46 wessels Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -53,6 +53,18 @@ public:
 
     unsigned int major;
     unsigned int minor;
+
+    bool operator==(const HttpVersion& that) const
+    {
+        if (this->major != that.major)
+            return false;
+
+        if (this->minor != that.minor)
+            return false;
+
+        return true;
+    }
+
 };
 
 #endif /* SQUID_HTTPVERSION_H */

diff --git a/src/client_side.cc b/src/client_side.cc
index 89eb7464726955813b76c5772910d68202a2edf7..4d60b8dbb003fe70423ecd11c5804a8f58097c82 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side.cc,v 1.673 2004/10/18 12:16:22 hno Exp $
+ * $Id: client_side.cc,v 1.674 2004/11/16 23:11:46 wessels Exp $
  *
  * DEBUG: section 33    Client-side Routines
  * AUTHOR: Duane Wessels
@@ -1937,6 +1937,12 @@ parseHttpRequest(ConnStateData::Pointer & conn, method_t * method_p,
 
     debug(33, 3) ("parseHttpRequest: end = {%s}\n", end);
 
+    if (strstr(req_hdr, "\r\r\n")) {
+        debug(33, 1) ("WARNING: suspicious HTTP request contains double CR\n");
+        xfree(inbuf);
+        return parseHttpRequestAbort(conn, "error:double-CR");
+    }
+
     prefix_sz = end - inbuf;
 
     debug(33, 3) ("parseHttpRequest: prefix_sz = %d, req_line_sz = %d\n",
@@ -2207,12 +2213,19 @@ clientProcessRequest(ConnStateData::Pointer &conn, ClientSocketContext *context,
 
     /* compile headers */
     /* we should skip request line! */
-    if (!httpRequestParseHeader(request, prefix + req_line_sz))
-        if (http->http_ver.major >= 1)
-            debug(33, 1) ("Failed to parse request headers: %s\n%s\n",
-                          http->uri, prefix);
-
-    /* continue anyway? */
+    if (!httpRequestParseHeader(request, prefix + req_line_sz)) {
+        clientStreamNode *node = context->getClientReplyContext();
+        debug(33, 5) ("Failed to parse request headers:\n%s\n", prefix);
+        clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
+        assert (repContext);
+        repContext->setReplyToError(
+            ERR_INVALID_URL, HTTP_BAD_REQUEST, method, http->uri,
+            &conn->peer.sin_addr, NULL, NULL, NULL);
+        assert(context->http->out.offset == 0);
+        context->pullData();
+        conn->flags.readMoreRequests = 0;
+        return;
+    }
 
     request->flags.accelerated = http->flags.accel;
 

diff --git a/src/http.cc b/src/http.cc
index 9bfbff7d7109cf0003600379741cbb7012e792aa..a0f773325e87a84458aa641d3e72efc837e927f1 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: http.cc,v 1.434 2004/11/06 22:20:47 hno Exp $
+ * $Id: http.cc,v 1.435 2004/11/16 23:11:46 wessels Exp $
  *
  * DEBUG: section 11    Hypertext Transfer Protocol (HTTP)
  * AUTHOR: Harvest Derived
@@ -649,6 +649,7 @@ HttpStateData::processReplyHeader(const char *buf, int size)
     if (hdr_len > 4 && strncmp(reply_hdr, "HTTP/", 5)) {
         debug(11, 3) ("httpProcessReplyHeader: Non-HTTP-compliant header: '%s'\n", reply_hdr);
         reply_hdr_state += 2;
+        reply->sline.version = HttpVersion(1, 0);
         reply->sline.status = HTTP_INVALID_HEADER;
         storeEntryReplaceObject (entry, reply);
 
@@ -688,6 +689,21 @@ HttpStateData::processReplyHeader(const char *buf, int size)
     /* Parse headers into reply structure */
     /* what happens if we fail to parse here? */
     httpReplyParse(reply, reply_hdr, hdr_len);
+
+    if (reply->sline.status >= HTTP_INVALID_HEADER) {
+        debug(11, 3) ("httpProcessReplyHeader: Non-HTTP-compliant header: '%s'\n", reply_hdr);
+        reply->sline.version = HttpVersion(1, 0);
+        reply->sline.status = HTTP_INVALID_HEADER;
+        storeEntryReplaceObject (entry, reply);
+
+        if (eof == 1) {
+            fwdComplete(fwd);
+            comm_close(fd);
+        }
+
+        return;
+    }
+
     processSurrogateControl (reply);
     /* TODO: we need our own reply * in the httpState, as we probably don't want to replace
      * the storeEntry with interim headers
@@ -972,7 +988,13 @@ HttpStateData::readReply (int fd, char *readBuf, size_t len, comm_err_t flag, in
              */
             /* doesn't return */
             processReplyHeader(buf, len);
-        else {
+        else if (entry->getReply()->sline.status == HTTP_INVALID_HEADER && HttpVersion(0,9) != entry->getReply()->sline.version) {
+            ErrorState *err;
+            err = errorCon(ERR_INVALID_REQ, HTTP_BAD_GATEWAY);
+            err->request = requestLink((HttpRequest *) request);
+            fwdFail(fwd, err);
+            do_next_read = 0;
+        } else {
             fwdComplete(fwd);
             do_next_read = 0;
             comm_close(fd);
@@ -983,9 +1005,22 @@ HttpStateData::readReply (int fd, char *readBuf, size_t len, comm_err_t flag, in
 
             if (reply_hdr_state == 2) {
                 http_status s = entry->getReply()->sline.status;
+                HttpVersion httpver = entry->getReply()->sline.version;
+
+                if (s == HTTP_INVALID_HEADER && httpver != HttpVersion(0,9)) {
+                    ErrorState *err;
+                    storeEntryReset(entry);
+                    err = errorCon(ERR_INVALID_REQ, HTTP_BAD_GATEWAY);
+                    err->request = requestLink((HttpRequest *) request);
+                    fwdFail(fwd, err);
+                    comm_close(fd);
+                    return;
+                }
+
 #if WIP_FWD_LOG
 
                 fwdStatus(fwd, s);
+
 #endif
                 /*
                  * If its not a reply that we will re-forward, then