Fix force_request_body_continuation bug

When the force_request_body_continuation access list is configured squid is
sends 100-Continue responses to all HTTP GET messages unless the admin
is very careful with the ACLs. This can be reproduced trivially with

  force_request_body_continuation allow all

We should not evaluate force_request_body_continuation if the request
does not include "Expect: 100-continue" header.

This is a Measurement Factory project

diff --git a/src/client_side.cc b/src/client_side.cc
index 68aca4eb01c38cb649eba6d2e9b9e87493020474..f075afd95bc66a4df443fd92d226fbbc6dd315db 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2663,23 +2663,6 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp,
         return;
     }
 
-    if (request->header.has(HDR_EXPECT)) {
-        const String expect = request->header.getList(HDR_EXPECT);
-        const bool supportedExpect = (expect.caseCmp("100-continue") == 0);
-        if (!supportedExpect) {
-            clientStreamNode *node = context->getClientReplyContext();
-            clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
-            assert (repContext);
-            conn->quitAfterError(request.getRaw());
-            repContext->setReplyToError(ERR_INVALID_REQ, Http::scExpectationFailed, request->method, http->uri,
-                                        conn->clientConnection->remote, request.getRaw(), NULL, NULL);
-            assert(context->http->out.offset == 0);
-            context->pullData();
-            clientProcessRequestFinished(conn, request);
-            return;
-        }
-    }
-
     clientSetKeepaliveFlag(http);
     // Let tunneling code be fully responsible for CONNECT requests
     if (http->request->method == Http::METHOD_CONNECT) {

diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc
index 28ca5c5488c0a00aff953a835dda1d779194c662..6c03f3b882aca6ef981c407fdf8b133787ebd28d 100644 (file)
--- a/src/servers/Http1Server.cc
+++ b/src/servers/Http1Server.cc
@@ -190,22 +190,42 @@ Http::One::Server::processParsedRequest(ClientSocketContext *context)
     if (!buildHttpRequest(context))
         return;
 
-    if (Config.accessList.forceRequestBodyContinuation) {
-        ClientHttpRequest *http = context->http;
-        HttpRequest *request = http->request;
-        ACLFilledChecklist bodyContinuationCheck(Config.accessList.forceRequestBodyContinuation, request, NULL);
-        if (bodyContinuationCheck.fastCheck() == ACCESS_ALLOWED) {
-            debugs(33, 5, "Body Continuation forced");
-            request->forcedBodyContinuation = true;
-            //sendControlMsg
-            HttpReply::Pointer rep = new HttpReply;
-            rep->sline.set(Http::ProtocolVersion(), Http::scContinue);
-
-            typedef UnaryMemFunT<Http1::Server, ClientSocketContext::Pointer> CbDialer;
-            const AsyncCall::Pointer cb = asyncCall(11, 3,  "Http1::Server::proceedAfterBodyContinuation", CbDialer(this, &Http1::Server::proceedAfterBodyContinuation, ClientSocketContext::Pointer(context)));
-            sendControlMsg(HttpControlMsg(rep, cb));
+    ClientHttpRequest *http = context->http;
+    HttpRequest::Pointer request = http->request;
+
+    if (request->header.has(HDR_EXPECT)) {
+        const String expect = request->header.getList(HDR_EXPECT);
+        const bool supportedExpect = (expect.caseCmp("100-continue") == 0);
+        if (!supportedExpect) {
+            clientStreamNode *node = context->getClientReplyContext();
+            quitAfterError(request.getRaw());
+            // setLogUri should called before repContext->setReplyToError
+            setLogUri(http, urlCanonicalClean(request.getRaw()));
+            clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
+            assert (repContext);
+            repContext->setReplyToError(ERR_INVALID_REQ, Http::scExpectationFailed, request->method, http->uri,
+                                        clientConnection->remote, request.getRaw(), NULL, NULL);
+            assert(context->http->out.offset == 0);
+            context->pullData();
+            clientProcessRequestFinished(this, request);
             return;
         }
+
+        if (Config.accessList.forceRequestBodyContinuation) {       
+            ACLFilledChecklist bodyContinuationCheck(Config.accessList.forceRequestBodyContinuation, request.getRaw(), NULL);
+            if (bodyContinuationCheck.fastCheck() == ACCESS_ALLOWED) {
+                debugs(33, 5, "Body Continuation forced");
+                request->forcedBodyContinuation = true;
+                //sendControlMsg
+                HttpReply::Pointer rep = new HttpReply;
+                rep->sline.set(Http::ProtocolVersion(), Http::scContinue);
+
+                typedef UnaryMemFunT<Http1::Server, ClientSocketContext::Pointer> CbDialer;
+                const AsyncCall::Pointer cb = asyncCall(11, 3,  "Http1::Server::proceedAfterBodyContinuation", CbDialer(this, &Http1::Server::proceedAfterBodyContinuation, ClientSocketContext::Pointer(context)));
+                sendControlMsg(HttpControlMsg(rep, cb));
+                return;
+            }
+        }
     }
     clientProcessRequest(this, parser_, context);
 }