Remove HTTP reply header completion hack (#910)

Treat responses with truncated HTTP headers (i.e. no CRLF after all the
field-lines) as malformed, replacing them with an HTTP 502
ERR_INVALID_RESP error (same as any other HTTP response with malformed
headers would get).

Since Bug 2879 (commit da6c841 and earlier v2-only changes), Squid was
"fixing" such truncated headers by adding an extra CRLF sequence and
re-parsing them. Depending on the truncation circumstances, that old
workaround could result in rather bad outcomes for Squid and its
clients. Hopefully, we no longer need to work around such bugs. If we
do, we should do it in a safer manner and with admin permission.

diff --git a/src/http.cc b/src/http.cc
index 4a7c3daf63462ae68941c1bc5a2e60f407a31827..a669b87004aaf892b85743fd99fe2673eef42e47 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -672,15 +672,8 @@ HttpStateData::processReplyHeader()
 
         if (hp->needsMoreData()) {
             if (eof) { // no more data coming
-                /* Bug 2879: Replies may terminate with \r\n then EOF instead of \r\n\r\n.
-                 * We also may receive truncated responses.
-                 * Ensure here that we have at minimum two \r\n when EOF is seen.
-                 */
-                inBuf.append("\r\n\r\n", 4);
-                // retry the parse
-                parsedOk = hp->parse(inBuf);
-                // sync the buffers after parsing.
-                inBuf = hp->remaining();
+                assert(!parsedOk);
+                // fall through to handle this premature EOF as an error
             } else {
                 debugs(33, 5, "Incomplete response, waiting for end of response headers");
                 return;
@@ -693,7 +686,11 @@ HttpStateData::processReplyHeader()
             debugs(11, 3, "Non-HTTP-compliant header:\n---------\n" << inBuf << "\n----------");
             flags.headers_parsed = true;
             HttpReply *newrep = new HttpReply;
-            newrep->sline.set(Http::ProtocolVersion(), hp->parseStatusCode);
+            // hp->needsMoreData() means hp->parseStatusCode is unusable, but, here,
+            // it also means that the reply header got truncated by a premature EOF
+            assert(!hp->needsMoreData() || eof);
+            const auto scode = hp->needsMoreData() ? Http::scInvalidHeader : hp->parseStatusCode;
+            newrep->sline.set(Http::ProtocolVersion(), scode);
             setVirginReply(newrep);
             return;
         }