From fc10bc7dfd9d86e25804dbe9c90146054465b218 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Thu, 18 Aug 2016 20:32:01 -0600 Subject: [PATCH] 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. --- src/client_side.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index de14fe96ab..bcc25d4410 100644 --- 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; } } -- 2.47.2