From: serassio <>
Date: Mon, 7 Mar 2005 04:08:13 +0000 (+0000)
Subject: Bug #890: Various HTTP workarounds and minor corrections
X-Git-Tag: SQUID_3_0_PRE4~860
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=21b927620fbab06770ae2fe04ac8ddf5f4d66666;p=thirdparty%2Fsquid.git

Bug #890: Various HTTP workarounds and minor corrections

- Automatically time out incorrectly signalled persistent connections
  after 10 seconds of inactitivy. Also gives a warning in cache.log

- New detect_broken_pconn squid.conf option

- Do not strip whitespace from the beginning of HTTP/0.9 replies

- Do not delay forwarding of HTTP/0.9 replies

- Do not delay forwarding of POST/PUT replies. Also includes detection
  of some common forms of abuse of the same for non-HTTP requests.

Forward port of 2.5 patch.
---

diff --git a/src/HttpMsg.cc b/src/HttpMsg.cc
index 11ec84edef..07ead36f78 100644
--- a/src/HttpMsg.cc
+++ b/src/HttpMsg.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpMsg.cc,v 1.13 2003/09/01 03:49:37 robertc Exp $
+ * $Id: HttpMsg.cc,v 1.14 2005/03/06 21:08:13 serassio Exp $
  *
  * DEBUG: section 74    HTTP Message
  * AUTHOR: Alex Rousskov
@@ -101,6 +101,8 @@ httpMsgIsolateHeaders(const char **parse_start, const char **blk_start, const ch
 int
 httpMsgIsPersistent(HttpVersion const &http_ver, const HttpHeader * hdr)
 {
+#if WHEN_SQUID_IS_NOT_HTTP1_1
+
     if ((http_ver.major >= 1) && (http_ver.minor >= 1)) {
         /*
          * for modern versions of HTTP: persistent unless there is
@@ -108,6 +110,9 @@ httpMsgIsPersistent(HttpVersion const &http_ver, const HttpHeader * hdr)
          */
         return !httpHeaderHasConnDir(hdr, "close");
     } else {
+#else
+    {
+#endif
         /*
          * Persistent connections in Netscape 3.x are allegedly broken,
          * return false if it is a browser connection.  If there is a
@@ -115,7 +120,8 @@ httpMsgIsPersistent(HttpVersion const &http_ver, const HttpHeader * hdr)
          */
         const char *agent = httpHeaderGetStr(hdr, HDR_USER_AGENT);
 
-        if (agent && !httpHeaderHas(hdr, HDR_VIA)) {
+        if (agent && !httpHeaderHas(hdr, HDR_VIA))
+        {
             if (!strncasecmp(agent, "Mozilla/3.", 10))
                 return 0;
 
diff --git a/src/cf.data.pre b/src/cf.data.pre
index 144a489d97..2e9d79111e 100644
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -1,6 +1,6 @@
 
 #
-# $Id: cf.data.pre,v 1.378 2005/03/06 14:52:49 serassio Exp $
+# $Id: cf.data.pre,v 1.379 2005/03/06 21:08:13 serassio Exp $
 #
 #
 # SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -4447,6 +4447,21 @@ DOC_START
 	this directive only connection failure triggers rotation.
 DOC_END
 
+NAME: detect_broken_pconn
+TYPE: onoff
+LOC: Config.onoff.detect_broken_server_pconns
+DEFAULT: off
+DOC_START
+	Some servers have been found to incorrectly signal the use
+	of HTTP/1.0 persistent connections even on replies not
+	compatible, causing significant delays. This server problem
+	has mostly been seen on redirects.
+
+	By enabling this directive Squid attempts to detect such
+	broken replies and automatically assume the reply is finished
+	after 10 seconds timeout.
+DOC_END
+
 NAME: pipeline_prefetch
 TYPE: onoff
 LOC: Config.onoff.pipeline_prefetch
diff --git a/src/client_side.cc b/src/client_side.cc
index 2844333ddd..f6435f3be7 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side.cc,v 1.680 2005/02/09 13:01:40 serassio Exp $
+ * $Id: client_side.cc,v 1.681 2005/03/06 21:08:13 serassio Exp $
  *
  * DEBUG: section 33    Client-side Routines
  * AUTHOR: Duane Wessels
@@ -2653,12 +2653,11 @@ clientAbortBody(HttpRequest * request)
     ConnStateData::Pointer conn = request->body_connection;
     char *buf;
     CBCB *callback;
-    request->body_connection = NULL;
 
     if (conn.getRaw() == NULL || conn->body.size_left <= 0)
         return 0;		/* No body to abort */
 
-    if (conn->body.callback != NULL) {
+    if (conn->body.callback != NULL || conn->body.request != request) {
         buf = conn->body.buf;
         callback = conn->body.callback;
         assert(request == conn->body.request);
diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index 3b0693b88a..ad6773e6c7 100644
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side_reply.cc,v 1.80 2004/12/21 15:03:01 robertc Exp $
+ * $Id: client_side_reply.cc,v 1.81 2005/03/06 21:08:13 serassio Exp $
  *
  * DEBUG: section 88    Client-side Reply Routines
  * AUTHOR: Robert Collins (Originally Duane Wessels in client_side.c)
@@ -2049,21 +2049,23 @@ clientReplyContext::sendMoreData (StoreIOBuffer result)
         return;
     }
 
-    /* handle headers */
-    if (Config.onoff.log_mime_hdrs) {
-        size_t k;
-
-        if ((k = headersEnd(buf, reqofs))) {
-            safe_free(http->al.headers.reply);
-            http->al.headers.reply = (char *)xcalloc(k + 1, 1);
-            xstrncpy(http->al.headers.reply, buf, k);
-        }
-    }
-
     buildReply(buf, reqofs);
     ssize_t body_size = reqofs;
 
     if (holdingReply) {
+
+        /* handle headers */
+
+        if (Config.onoff.log_mime_hdrs) {
+            size_t k;
+
+            if ((k = headersEnd(buf, reqofs))) {
+                safe_free(http->al.headers.reply);
+                http->al.headers.reply = (char *)xcalloc(k + 1, 1);
+                xstrncpy(http->al.headers.reply, buf, k);
+            }
+        }
+
         holdingBuffer = result;
         processReplyAccess ();
         return;
diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index eba596bf46..e8583f0d9b 100644
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side_request.cc,v 1.43 2004/12/21 17:28:29 robertc Exp $
+ * $Id: client_side_request.cc,v 1.44 2005/03/06 21:08:13 serassio Exp $
  * 
  * DEBUG: section 85    Client-side Request Routines
  * AUTHOR: Robert Collins (Originally Duane Wessels in client_side.c)
@@ -235,8 +235,10 @@ ClientHttpRequest::~ClientHttpRequest()
      * found the end of the body yet
      */
 
-    if (request && request->body_connection.getRaw() != NULL)
+    if (request && request->body_connection.getRaw() != NULL) {
         clientAbortBody(request);	/* abort body transter */
+        request->body_connection = NULL;
+    }
 
     /* the ICP check here was erroneous
      * - storeReleaseRequest was always called if entry was valid 
diff --git a/src/http.cc b/src/http.cc
index 826adbdceb..8d0c16a9b6 100644
--- a/src/http.cc
+++ b/src/http.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: http.cc,v 1.446 2005/03/06 19:37:17 serassio Exp $
+ * $Id: http.cc,v 1.447 2005/03/06 21:08:13 serassio Exp $
  *
  * DEBUG: section 11    Hypertext Transfer Protocol (HTTP)
  * AUTHOR: Harvest Derived
@@ -47,6 +47,7 @@
 #include "MemObject.h"
 #include "HttpHdrContRange.h"
 #include "ACLChecklist.h"
+#include "fde.h"
 #if DELAY_POOLS
 #include "DelayPools.h"
 #endif
@@ -79,6 +80,15 @@ httpStateFree(int fd, void *data)
     if (httpState == NULL)
         return;
 
+    if (httpState->body_buf) {
+        clientAbortBody(httpState->orig_request);
+
+        if (httpState->body_buf) {
+            memFree(httpState->body_buf, MEM_8K_BUF);
+            httpState->body_buf = NULL;
+        }
+    }
+
     storeUnlockObject(httpState->entry);
 
     if (!memBufIsNull(&httpState->reply_hdr)) {
@@ -799,10 +809,17 @@ no_cache:
         if (_peer)
             _peer->stats.n_keepalives_sent++;
 
-    if (entry->getReply()->keep_alive)
+    if (entry->getReply()->keep_alive) {
         if (_peer)
             _peer->stats.n_keepalives_recv++;
 
+        if (Config.onoff.detect_broken_server_pconns && httpReplyBodySize(request->method, reply) == -1) {
+            debug(11, 1) ("httpProcessReplyHeader: Impossible keep-alive header from '%s'\n", storeUrl(entry));
+            debug(11, 2) ("GOT HTTP REPLY HDR:\n---------\n%s\n----------\n", reply_hdr.buf);
+            flags.keepalive_broken = 1;
+        }
+    }
+
     if (entry->getReply()->date > -1 && !_peer) {
         int skew = abs((int)(entry->getReply()->date - squid_curtime));
 
@@ -941,7 +958,6 @@ HttpStateData::readReply (int fd, char *readBuf, size_t len, comm_err_t flag, in
 
         kb_incr(&statCounter.server.all.kbytes_in, len);
         kb_incr(&statCounter.server.http.kbytes_in, len);
-        commSetTimeout(fd, Config.Timeout.read, NULL, NULL);
         IOStats.Http.reads++;
 
         for (clen = len - 1, bin = 0; clen; bin++)
@@ -950,7 +966,7 @@ HttpStateData::readReply (int fd, char *readBuf, size_t len, comm_err_t flag, in
         IOStats.Http.read_hist[bin]++;
     }
 
-    if (!memBufIsNull(&reply_hdr) && flag == COMM_OK && len > 0) {
+    if (!memBufIsNull(&reply_hdr) && flag == COMM_OK && len > 0 && fd_table[fd].uses > 1) {
         /* Skip whitespace */
 
         while (len > 0 && xisspace(*buf))
@@ -958,6 +974,7 @@ HttpStateData::readReply (int fd, char *readBuf, size_t len, comm_err_t flag, in
 
         if (len == 0) {
             /* Continue to read... */
+            /* Timeout NOT increased. This whitespace was from previous reply */
             do_next_read = 1;
             maybeReadData();
             return;
@@ -1111,7 +1128,14 @@ HttpStateData::processReplyData(const char *buf, size_t len)
         switch (persistentConnStatus()) {
 
         case INCOMPLETE_MSG:
-            /* Wait for EOF condition */
+            /* Wait for more data or EOF condition */
+
+            if (flags.keepalive_broken) {
+                commSetTimeout(fd, 10, NULL, NULL);
+            } else {
+                commSetTimeout(fd, Config.Timeout.read, NULL, NULL);
+            }
+
             do_next_read = 1;
             break;
 
@@ -1196,8 +1220,6 @@ HttpStateData::SendComplete(int fd, char *bufnotused, size_t size, comm_err_t er
         comm_close(fd);
         return;
     } else {
-        /* Schedule read reply. */
-        entry->delayAwareRead(fd, httpState->buf, SQUID_TCP_SO_RCVBUF, httpReadReply, httpState);
         /*
          * Set the read timeout here because it hasn't been set yet.
          * We only set the read timeout after the request has been
@@ -1625,10 +1647,15 @@ httpSendRequest(HttpStateData * httpState)
     StoreEntry *entry = httpState->entry;
     peer *p = httpState->_peer;
     CWCB *sendHeaderDone;
+    int fd = httpState->fd;
 
-    debug(11, 5) ("httpSendRequest: FD %d: httpState %p.\n", httpState->fd,
+    debug(11, 5) ("httpSendRequest: FD %d: httpState %p.\n", fd,
                   httpState);
 
+    /* Schedule read reply. */
+    commSetTimeout(fd, Config.Timeout.lifetime, httpTimeout, httpState);
+    entry->delayAwareRead(fd, httpState->buf, SQUID_TCP_SO_RCVBUF, httpReadReply, httpState);
+
     if (httpState->orig_request->body_connection.getRaw() != NULL)
         sendHeaderDone = httpSendRequestEntity;
     else
@@ -1674,8 +1701,8 @@ httpSendRequest(HttpStateData * httpState)
                            entry,
                            &mb,
                            httpState->flags);
-    debug(11, 6) ("httpSendRequest: FD %d:\n%s\n", httpState->fd, mb.buf);
-    comm_old_write_mbuf(httpState->fd, mb, sendHeaderDone, httpState);
+    debug(11, 6) ("httpSendRequest: FD %d:\n%s\n", fd, mb.buf);
+    comm_old_write_mbuf(fd, mb, sendHeaderDone, httpState);
 }
 
 void
@@ -1789,8 +1816,22 @@ static void
 httpRequestBodyHandler(char *buf, ssize_t size, void *data)
 {
     HttpStateData *httpState = (HttpStateData *) data;
+    httpState->body_buf = NULL;
 
     if (size > 0) {
+        if (httpState->reply_hdr_state >= 2 && !httpState->flags.abuse_detected) {
+            httpState->flags.abuse_detected = 1;
+            debug(11, 1) ("httpSendRequestEntryDone: Likely proxy abuse detected '%s' -> '%s'\n",
+                          inet_ntoa(httpState->orig_request->client_addr),
+                          storeUrl(httpState->entry));
+
+            if (httpState->entry->getReply()->sline.status == HTTP_INVALID_HEADER) {
+                memFree8K(buf);
+                comm_close(httpState->fd);
+                return;
+            }
+        }
+
         comm_old_write(httpState->fd, buf, size, httpSendRequestEntity, data, memFree8K);
     } else if (size == 0) {
         /* End of body */
@@ -1835,7 +1876,8 @@ httpSendRequestEntity(int fd, char *bufnotused, size_t size, comm_err_t errflag,
         return;
     }
 
-    clientReadBody(httpState->orig_request, (char *)memAllocate(MEM_8K_BUF), 8192, httpRequestBodyHandler, httpState);
+    httpState->body_buf = (char *)memAllocate(MEM_8K_BUF);
+    clientReadBody(httpState->orig_request, httpState->body_buf, 8192, httpRequestBodyHandler, httpState);
 }
 
 void
diff --git a/src/http.h b/src/http.h
index 9f04690d9f..5332c20af8 100644
--- a/src/http.h
+++ b/src/http.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: http.h,v 1.10 2004/12/21 17:52:53 robertc Exp $
+ * $Id: http.h,v 1.11 2005/03/06 21:08:13 serassio Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -59,6 +59,7 @@ public:
     int fd;
     http_state_flags flags;
     FwdState *fwd;
+    char *body_buf;
     off_t currentOffset;
     int do_next_read;
     size_t read_sz;
diff --git a/src/structs.h b/src/structs.h
index 082f360742..ae2e534b11 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: structs.h,v 1.511 2005/03/06 14:46:31 serassio Exp $
+ * $Id: structs.h,v 1.512 2005/03/06 21:08:13 serassio Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -568,6 +568,7 @@ struct _SquidConfig
 #endif
 
         int request_entities;
+        int detect_broken_server_pconns;
         int balance_on_multiple_ip;
         int relaxed_header_parser;
         int check_hostnames;
@@ -966,6 +967,12 @@ unsigned int front_end_https:
 
 unsigned int originpeer:
     1;
+
+unsigned int keepalive_broken:
+    1;
+
+unsigned int abuse_detected:
+    1;
 };
 
 struct _ipcache_addrs