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.

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

adding host details in proxy headers #3532

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

HarikrishnanTechie
Copy link

Added host details in proxy headers so that, request message will have host details

@@ -1033,6 +1033,9 @@ def _prepare_proxy(self, conn: HTTPSConnection) -> None: # type: ignore[overrid
else:
tunnel_scheme = "http"

if self._tunnel_host: # Adding host details in proxy headers
Copy link
Contributor

Choose a reason for hiding this comment

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

We use this unconditionally a little lower. Why are we guarding it here?

Also, is this just for debugging because this:

a. Breaks tests
b. Seems superfluous and unnecessary

Copy link
Author

@HarikrishnanTechie HarikrishnanTechie Dec 23, 2024

Choose a reason for hiding this comment

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

Hi @sigmavirus24,

The main enhancement in my code is the inclusion of a Host header in the proxy_headers dictionary when a tunneling host (_tunnel_host) is defined. This change is specifically for cases where a proxy is being used.

Why This Change is Important
Complying with Proxy Protocols:
Including the Host header in the CONNECT request ensures the proxy server can correctly identify the target host for establishing the tunnel. This is especially critical for HTTPS connections where secure communication is required.

Enhancing Proxy Support:
By adding host details dynamically, our code becomes more robust and compatible with environments that rely on proxies. Some proxies may explicitly require the Host header to process requests correctly.

python_verison: 3.10.
Can we connect to dicuss more about my requirement?

Copy link
Contributor

@sigmavirus24 sigmavirus24 Dec 23, 2024

Choose a reason for hiding this comment

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

This feeds into the stdlib http.client.HTTPConnection and thus isn't necessary https://github.com/python/cpython/blob/180d417e9f9456defd4c5b53cae678c318287921/Lib/http/client.py#L919

Furthermore, what you have here is borderline dangerous. In the event someone is using a proxy host with a internationalized domain name, this will introduce potentially unsafe behaviour. The stdlib, which already does this, IDNA encodes the string to bytes, and then back to an ascii string. This is just pushing through the un-encoded data which may be harmless in some cases, and dangerous in others.

Connecting isn't necessary because the tests still fail and you still haven't shown how this fixes a problem or explained what the problem is. We need something that is reproducible to understand what needs fixing and why. If you can't provide a reproducible test case for your problem that clearly fails without this change, I don't believe we can help you with whatever the problem is you're encountering

Copy link
Author

@HarikrishnanTechie HarikrishnanTechie Dec 24, 2024

Choose a reason for hiding this comment

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

Problem Statement: In the connect request header, the expected host details (e.g., Host: google.com) is missing.

Evidence: The screenshot (as described) shows the connect request header missing the Host field. This deviation is causing issues with meeting the application's requirements.

Note:
We are using proxy environment. And python 3.10 using http 1.0 version by default.

Also I couldn't find changes related tunnel host (mentioned in below) in python 3.10 version.
if not any(header.lower() == "host" for header in self._tunnel_headers):
encoded_host = self._tunnel_host.encode("idna").decode("ascii")
self._tunnel_headers["Host"] = "%s:%d" % (
encoded_host, self._tunnel_port)
https://github.com/python/cpython/blob/180d417e9f9456defd4c5b53cae678c318287921/Lib/http/client.py#L919

is possible to get those changes in 3.8 and 3.10 version?

Copy link
Contributor

Choose a reason for hiding this comment

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

3.12 is the first Python to have that logic. You'll need to advocate for the backport of this commit as a bug fix to 3.11, 3.10, and 3.9. 3.8 is EOL. Also if you read that commit closely, you are not using HTTP/1.0 otherwise this wouldn't fail.

Also your screenshot contains clear-text credentials username alteryx, password alt3ryx. You should change those credentials immediately.

Copy link
Author

Choose a reason for hiding this comment

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

we are using 3.10 version. Also is there any other alternative to add host details in request headers for connect request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants