Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Tunnel Server RESPONSE dumps #279

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Improve Tunnel Server RESPONSE dumps #279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f80ca75
Fix Tunnel Server RESPONSE dumps
eduard-bagdasaryan Dec 26, 2024
6f56194
Avoid printing multiple RESPONSE messages
eduard-bagdasaryan Dec 27, 2024
05e4c0f
Undone an out-of-scope change
eduard-bagdasaryan Dec 27, 2024
9664b02
Added an XXX about code dulication
eduard-bagdasaryan Dec 29, 2024
fb51e46
Bug 5093: List http_port params that https_port/ftp_port lack (#1977)
kinkie Jan 7, 2025
5fa48bb
Bug 5311: clarify configuration byte units (#1979)
kinkie Jan 9, 2025
544997c
Merge branch 'master' into SQUID-1033-fix-tunnel-server-resonse-dumps
rousskov Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/cf.data.pre
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ COMMENT_START

Units accepted by Squid are:
bytes - byte
KB - Kilobyte (1024 bytes)
MB - Megabyte
GB - Gigabyte
KB - Kilobyte (2^10, 1'024 bytes)
MB - Megabyte (2^20, 1'048'576 bytes)
GB - Gigabyte (2^30, 1'073'741'824 bytes)
Squid does not yet support KiB, MiB, and GiB unit names.

Values with time units

Expand Down Expand Up @@ -2575,6 +2576,9 @@ DOC_START
The tls-cert= option is mandatory on HTTPS ports.

See http_port for a list of modes and options.
Not all http_port options are available for https_port.
Among the unavalable options:
- require-proxy-header
DOC_END

NAME: ftp_port
Expand Down Expand Up @@ -2639,6 +2643,9 @@ DOC_START

Other http_port modes and options that are not specific to HTTP and
HTTPS may also work.
Among the options that are not available for ftp_port:
- require-proxy-header
- ssl-bump
DOC_END

NAME: tcp_outgoing_tos tcp_outgoing_ds tcp_outgoing_dscp
Expand Down
17 changes: 11 additions & 6 deletions src/clients/HttpTunneler.cc
View file
Open in desktop
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----------");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code suffers from the same two problems that equivalent HttpStateData::processReplyHeader() code suffers from. We can propose to duplicate those problems (for now), but (if we do) we should be explicit about it (so that it does not look as if we are unknowingly adding bugs):

Suggested change
debugs(11, 3, "Non-HTTP-compliant header:\n---------\n" << readBuf << "\n----------");
// 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----------");

The above suggestion also documents code duplication itself.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

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