From: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Date: Mon, 13 Jan 2025 19:53:36 +0000 (+0000)
Subject: Improve Tunnel Server RESPONSE dumps (#1975)
X-Git-Tag: SQUID_7_0_1~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a14169990e66a0622165ea3f16712b58621e48f3;p=thirdparty%2Fsquid.git

Improve Tunnel Server RESPONSE dumps (#1975)

Level-2 "Tunnel Server RESPONSE:..." debugs() incorrectly assumed that
its readBuf parameter contained hdr_sz header bytes. In reality, by the
time code reached that debugs(), readBuf no longer had any header bytes
(and often had no bytes at all). Besides broken header dumps, this bug
could lead to problems that Valgrind reports as "Conditional jump or
move depends on uninitialised value" in DebugChannel::writeToStream().

This fix mimics HttpStateData::processReplyHeader() reporting code,
including its known problems. Future changes should address those
problems and reduce code duplication across at least ten functions
containing similar "decorated" level-2 message dumps.
---

diff --git a/src/clients/HttpTunneler.cc b/src/clients/HttpTunneler.cc
index d52dd457e8..48b3206863 100644
--- a/src/clients/HttpTunneler.cc
+++ b/src/clients/HttpTunneler.cc
@@ -296,15 +296,25 @@ Http::Tunneler::handleResponse(const bool eof)
     }
 
     if (!parsedOk) {
+        // XXX: This code and Server RESPONSE reporting code below duplicate
+        // HttpStateData::processReplyHeader() reporting code, including its problems.
+        debugs(11, 3, "Non-HTTP-compliant header:\n---------\n" << readBuf << "\n----------");
         bailOnResponseError("malformed CONNECT response from peer", nullptr);
         return;
     }
 
+    /* We know the whole response is in parser now */
+    debugs(11, 2, "Tunnel Server " << connection);
+    debugs(11, 2, "Tunnel Server RESPONSE:\n---------\n" <<
+           hp->messageProtocol() << " " << hp->messageStatus() << " " << hp->reasonPhrase() << "\n" <<
+           hp->mimeHeader() <<
+           "----------");
+
     HttpReply::Pointer rep = new HttpReply;
     rep->sources |= Http::Message::srcHttp;
     rep->sline.set(hp->messageProtocol(), hp->messageStatus());
     if (!rep->parseHeader(*hp) && rep->sline.status() == Http::scOkay) {
-        bailOnResponseError("malformed CONNECT response from peer", nullptr);
+        bailOnResponseError("malformed CONNECT response headers mime block from peer", nullptr);
         return;
     }
 
@@ -313,11 +323,6 @@ Http::Tunneler::handleResponse(const bool eof)
     futureAnswer.peerResponseStatus = rep->sline.status();
     request->hier.peer_reply_status = rep->sline.status();
 
-    debugs(11, 2, "Tunnel Server " << connection);
-    debugs(11, 2, "Tunnel Server RESPONSE:\n---------\n" <<
-           Raw(nullptr, readBuf.rawContent(), rep->hdr_sz).minLevel(2).gap(false) <<
-           "----------");
-
     // bail if we did not get an HTTP 200 (Connection Established) response
     if (rep->sline.status() != Http::scOkay) {
         // TODO: To reuse the connection, extract the whole error response.