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

Fix HTTPS websites with system-wide HTTP proxy on Windows #6162

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ValdikSS
Copy link

@ValdikSS ValdikSS commented Jun 11, 2022

Due to urllib bug, requests fails to open any HTTPS websites on Windows
if there's system-wide HTTP proxy configured. This is because urllib
incorrectly prepends the protocol to the proxy host and port, as in:

{'http':  'http://host:port',
 'https': 'https://host:port',
 'ftp':   'ftp://host:port'}

Such configuration forces urllib3 to use HTTPS proxy (Secure Web Proxy,
also known as TLS Proxy) for HTTPS URLs, which the configured proxy
most likely does not support.

Detect incorrect behavior and rewrite the protocol to http.

Due to urllib bug, requests fails to open any HTTPS websites on Windows
if there's system-wide HTTP proxy configured. This is because urllib
incorrectly prepends the protocol to the proxy host and port, as in:

{'http':  'http://host:port',
 'https': 'https://host:port',
 'ftp':   'ftp://host:port'}

Such configuration forces urllib3 to use HTTPS proxy (Secure Web Proxy,
also known as TLS Proxy) for HTTPS URLs, which the configured proxy
most likely does not support.

Detect incorrect behavior and rewrite the protocol to http.
@ValdikSS
Copy link
Author

Issue #6139

@@ -816,6 +816,25 @@ def get_proxy(key):
return False


def getproxies_fix_system_proxies_windows():
"""Fix for urllib getproxies() bug on Windows with Python < 3.10.5
See: https://bugs.python.org/issue42627
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing precludes this from running on the versions unaffected according to you. This comment is aside from whether this is the correct fix

Copy link
Author

Choose a reason for hiding this comment

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

This is a no-op for a fixed Python/urllib version. This commit checks whether getproxies() returns the same proxy host and port for three protocols, each prefixed with the protocol, and rewrites it to "http://". The fixed urllib already returns proxies with "http://", so this commit does nothing in this case.

Copy link
Author

Choose a reason for hiding this comment

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

@sigmavirus24, do you think the code needs more checks?

@ValdikSS
Copy link
Author

Could anyone take a look at this merge request please?

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.

2 participants