Fix Handling of HTTP CONNECT Header in Proxy Connections #8021
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the HTTP CONNECT header formatting for proxy connections to ensure proper SSL certificate verification. The changes address issues where the server was receiving IP addresses instead of domain names and missing port numbers in Host headers.
- Modified the CONNECT request logic to use domain names instead of IP addresses when available
- Added proper port number formatting to Host headers following HTTP standards
- Implemented IPv6 address handling for bracketed and non-bracketed formats
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bearx3f
force-pushed
the
send_host_in_connect_fix
branch
2 times, most recently
from
2054c17 to
d1a69ea
Compare
…m_host is not empty
bearx3f
force-pushed
the
send_host_in_connect_fix
branch
from
d1a69ea to
74223e9
Compare
arvidn
left a comment
arvidn
left a comment
There was a problem hiding this comment.
in your description there's also a difference in the host: header. but this is not important then?
bearx3f
force-pushed
the
send_host_in_connect_fix
branch
2 times, most recently
from
6e11686 to
4257926
Compare
…handling of IPv6 and port suffixes
bearx3f
force-pushed
the
send_host_in_connect_fix
branch
from
4257926 to
39d4c39
Compare
…t formatting for HTTP CONNECT requests, including IPv6 support and edge case handling; add comprehensive tests for various scenarios
| host, port) | ||
|
|
||
| # If require_host_header is set, reject requests without Host header | ||
| if self.require_host_header and not host_header: |
There was a problem hiding this comment.
you you remove the check for self.require_host_header (and make it mandatory), you should remove the comment above as well as setting this member to True at the top of the class
…ce; update logging for Host header checks and add tests for CONNECT requests with/without Host header
bearx3f
force-pushed
the
send_host_in_connect_fix
branch
from
be4f2db to
baedd6b
Compare
|
it looks like the simulation error is legitimate and needs to be addressed |
…on send_host_in_connect setting; ensure compliance with RFC 9110 and RFC 9112
The problem is my patch will always send Host header but simulation test check if send_host_in_connect=false host should be empty. I will fix. |
bearx3f
changed the title
2 participants
From #7651 After I tested the latest pull request, which I'd previously said passed, the proxy server still couldn't sign the certificate when deployed with the actual application. This was because the server didn't receive the remote server in the correct
domain:portformat, such asexample.com:443. In this patch, I've fixed theCONNECTrequest to be correct.I noticed this by observing the request sent by Curl, which sends:
Originally, Libtorrent was sending:
In this original request, the Host header was missing the port, and the CONNECT method was sending an IP address instead of a domain name.