Fix no_proxy and no, not being respected. Merge self.proxies and proxies #5596
Conversation
Check if the proxy should be bypassed in case no_proxy or no match the host of the url. Also, since proxy can be defined in the session and the reuquest itself, both of them were never merged. Now they self.proxies will be updated by proxies.
Make it match the variable being returned
It makes more sense to have the check be outside the trust_env. Since it has to be always executed. Because proxy configuration can be performed on the Sessions class.
That logic was left from previous tires fixing no_proxy and since it's quite compact now, it can be removed and should_bypass_proxies should be used instead of setting a var.
| @@ -512,6 +512,14 @@ def request(self, method, url, | |||
|
|
|||
| proxies = proxies or {} | |||
|
|
|||
| # Update self.proxy with proxy and assing the result to proxies | |||
| if isinstance(proxies,dict): | |||
| slef_proxies_tmp = self.proxies.copy() | |||
There was a problem hiding this comment.
Is this a typo? slef_proxies_tmp should be self_proxies_tmp I suppose?
| if isinstance(proxies,dict): | ||
| slef_proxies_tmp = self.proxies.copy() | ||
| slef_proxies_tmp.update(proxies) | ||
| proxies = slef_proxies_tmp.copy() |
There was a problem hiding this comment.
There is no need to copy() the dict again. slef_proxies_tmp is already a copy of self.proxies
|
This patched fixed #5731 for me. I was very surprised this patch has not gotten any attention yet. Is there anything missing before it can be considered? |
|
Hi @Suika and @RichieB2B, we are also dealing with this issue, any updates? |
|
I found a work around that I described at #5731 (comment) and in #6172 |
|
Hi, we are also being hit by that and would appreciate if this issue would be fixed. |
|
Also being hit by this; in our case it's setting |
|
@Suika thank you for your work
this issue is known for at least 5 years (#4871) in general, waiting for pypi releases is a bad habit |
|
LGTM, im working for corp env, when setting up a Windows proxy, the 'no' and 'no_proxy' are fail in requests.get('http://localhost:8000', proxies={'http': proxy, 'https': proxy, 'no': 'localhost, 127.0.0.1'}) |
Since no_proxy is not working as intended, I tried fixing it with least intrsion into whole process.
It is contained to the
merge_environment_settingsandrequestfunction. In therequestthe proxy from request and proxy from session are merged. Having the request proxy overwrite the session proxy where needed.The
should_bypass_proxiesis applied outsideget_environ_proxiesinmerge_environment_settings, sinceno_proxyis only applied to OS ENV and not the proxies set inrequestsorSessionsShould close #4871, I hope.