Do not log error:transaction-end-before-headers after invalid requests.

Squid was not consuming read leftovers after failing to parse a request.
Starting with r14752, those leftovers were misinterpreted as another
unparsed request, creating an extra error:transaction-end-before-headers
access.log line after every error:invalid-request line (and probably
after every error:request-too-large line).

To stop Squid from accidentally reading new bytes and misinterpreting
them as another request, I was tempted to also clear flags.readMore
after consuming unparsable leftovers. In my tests, the flag is cleared
in ConnStateData::quitAfterError() called from clientTunnelOnError(),
but that logic looks rather fragile. I resisted the temptation to
improve it because controlling reads is a complicated matter (especially
in on_unsupported_protocol context) outside this logging fix scope.

diff --git a/src/client_side.cc b/src/client_side.cc
index de14fe96ab9d217e64eccd566baeb9702bd1945a..bcc25d4410d4fbe619b074fc064ab7b95546eec7 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -1313,10 +1313,14 @@ parseHttpRequest(ConnStateData *csd, const Http1::RequestParserPointer &hp)
         }
 
         if (!parsedOk) {
-            if (hp->parseStatusCode == Http::scRequestHeaderFieldsTooLarge || hp->parseStatusCode == Http::scUriTooLong)
-                return csd->abortRequestParsing("error:request-too-large");
-
-            return csd->abortRequestParsing("error:invalid-request");
+            const bool tooBig =
+                hp->parseStatusCode == Http::scRequestHeaderFieldsTooLarge ||
+                hp->parseStatusCode == Http::scUriTooLong;
+            auto result = csd->abortRequestParsing(
+                              tooBig ? "error:request-too-large" : "error:invalid-request");
+            // assume that remaining leftovers belong to this bad request
+            csd->consumeInput(csd->inBuf.length());
+            return result;
         }
     }