From a14169990e66a0622165ea3f16712b58621e48f3 Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Mon, 13 Jan 2025 19:53:36 +0000 Subject: [PATCH] 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. --- src/clients/HttpTunneler.cc | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) 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. -- 2.47.3