Skip to content

Commit

Permalink
Improve Tunnel Server RESPONSE dumps (#1975)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
eduard-bagdasaryan authored and squid-anubis committed Jan 13, 2025
1 parent 5fa48bb commit a141699
Showing 1 changed file with 11 additions and 6 deletions.
17 changes: 11 additions & 6 deletions src/clients/HttpTunneler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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.
Expand Down

0 comments on commit a141699

Please sign in to comment.