-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
urllib.request.getproxies() misparses Windows registry proxy settings #86793
Comments
If
for consistency with WinInet and Chromium, but it actually returns
This bug has existed for a very long time (since Python 2.0.1 if not earlier), but it was exposed recently when urllib3 added support for HTTPS-in-HTTPS proxies in version 1.26. Before that, an There are additional bugs in the treatment of single-proxy strings (the case when the string contains no The Chromium code for parsing the ProxyServer string can be found here: https://source.chromium.org/chromium/chromium/src/+/refs/tags/89.0.4353.1:net/proxy_resolution/proxy_config.cc;l=86 Below is my attempt at modifying the code from if '=' not in proxyServer and ';' not in proxyServer:
# Use one setting for all protocols.
# Chromium treats this as a separate category, and some software
# uses the ALL_PROXY environment variable for a similar purpose,
# so arguably this should be 'all={}'.format(proxyServer),
# but this is more backward compatible.
proxyServer = 'http={0};https={0};ftp={0}'.format(proxyServer)
for p in proxyServer.split(';'):
# Chromium and WinInet are inconsistent in their treatment of
# invalid strings with the wrong number of = characters. It
# probably doesn't matter.
protocol, addresses = p.split('=', 1)
protocol = protocol.strip()
# Chromium supports more than one proxy per protocol. I don't
# know how many clients support the same, but handling it is at
# least no worse than leaving the commas uninterpreted.
for address in addresses.split(','):
if protocol in {'http', 'https', 'ftp', 'socks'}:
# See if address has a type:// prefix
if not re.match('(?:[^/:]+)://', address):
if protocol == 'socks':
# Chromium notes that the correct protocol here
# is SOCKS4, but "socks://" is interpreted
# as SOCKS5 elsewhere. I don't know whether
# prepending socks4:// here would break code.
address = 'socks://' + address
else:
address = 'http://' + address
# A string like 'http=foo;http=bar' will produce a
# comma-separated list, while previously 'bar' would
# override 'foo'. That could potentially break something.
if protocol not in proxies:
proxies[protocol] = address
else:
proxies[protocol] += ',' + address |
I came across this issue as well. https://docs.microsoft.com/en-US/troubleshoot/windows-client/networking/configure-client-proxy-server-settings-by-registry-file |
We know Windows reslove: Now I want to know what if Windows slove this: |
I make some black box tests with the HTTPS proxy. benrg should add your patch as soon as possible. |
I found this problem too. |
I removed the multi-proxies-per-protocol support from the PR I submitted. The reason is that:
If you want to implement the multi-proxies-per-protocol support, then I think you should open another issue and make changes to all OS platforms. |
We should have no problem with how to parse HTTP proxy and HTTPS proxy. But I recently discovered an additional problem when using I investigated the parsing of some commonly used software:
The
Since it is customary to resolve SOCKS proxies in this way, I think CPython can consider doing the same with Windows registry. In previous versions, CPython parsed the SOCKS proxy in the Windows registry
I think it can be changed to
The use of If you have any comments or suggestions, please let me know. |
The fix is available as a pull request on GitHub for months (#26307). However, it seems that this pull request needs an approval from one maintainer before running any test. Could anyone help this out? |
I think the PR is basically ready, unfortunately it's stuck behind a CI issue we only just fixed, and needs to merge from main before it'll clear. Posting here once CI is green will get attention faster than on GitHub. |
Sorry I didn't see this comment before. Can it be merged now? |
any updates? |
I retriggered the CI to see if it passes now. |
Hello, I am curious if this will also address https://stackoverflow.com/questions/65931275/python-requests-module-does-it-use-system-level-on-windows-proxy-settings or if it's yet another problem? |
@TTimo From a quick glance, I think it'll deal with part of the second half of that question. It should be able to pick up manually customised proxies, but I don't think it'll get automatically configured ones. And anything that's bypassing urllib here won't get any benefit, naturally. |
Convert proxies extracted from windows registry to http for older Python versions. See: python/cpython#86793 Authored by: coletdjnz
Convert proxies extracted from windows registry to http for older Python versions. See: python/cpython#86793 Authored by: coletdjnz
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: