Proxy configuration is not re-evaluated on redirects #1727

Closed
rassie opened this Issue Nov 6, 2013 · 9 comments

Comments

Projects
None yet
4 participants
@rassie

rassie commented Nov 6, 2013

requests.get will fail if a URL which falls under no_proxy policy is redirected to an URL for which a proxy is mandatory.

@Lukasa

This comment has been minimized.

Show comment Hide comment
@Lukasa

Lukasa Nov 9, 2013

Owner

Yep, that's definitely right. Good spot! Thanks for raising this issue! 🍰

Owner

Lukasa commented Nov 9, 2013

Yep, that's definitely right. Good spot! Thanks for raising this issue! 🍰

@Lukasa

This comment has been minimized.

Show comment Hide comment
@Lukasa

Lukasa Nov 10, 2013

Owner

@sigmavirus24 I really want to add a regression test for this, but we have essentially got no testing infrasrtucture for proxies. Do you have any bright ideas/will Betamax help with this, or do I need to look into spinning up an open HTTP proxy?

Owner

Lukasa commented Nov 10, 2013

@sigmavirus24 I really want to add a regression test for this, but we have essentially got no testing infrasrtucture for proxies. Do you have any bright ideas/will Betamax help with this, or do I need to look into spinning up an open HTTP proxy?

@sigmavirus24

This comment has been minimized.

Show comment Hide comment
@sigmavirus24

sigmavirus24 Nov 10, 2013

Owner

Frankly I'm not sure if Betamax would help. It works on the Adapter level so the answer is possibly not.

Owner

sigmavirus24 commented Nov 10, 2013

Frankly I'm not sure if Betamax would help. It works on the Adapter level so the answer is possibly not.

@sigmavirus24

This comment has been minimized.

Show comment Hide comment
@sigmavirus24

sigmavirus24 Nov 10, 2013

Owner

@Lukasa frankly, I don't understand the problem well. Proxies have always been your thing and I never attempted to get enough background on them.

So what Betamax does is replace the adapters that the Session object is using and then use a regular HTTPAdapter to make the actual request if necessary. The reason I'm not certain, is because of [get_connection](https://github.com/kennethreitz/requests/blob/master/requests/adapters.py#L187..L211] on the HTTPAdapter.

Betamax looks at the Prepared Request that comes through and then matches that against previously recorded (if any) requests to find an appropriate response. Because of how requests handles redirects, even redirects are recorded properly. If the logic you're looking for happens outside of the HTTPAdapter (any of the layers above it), Betamax can probably help so long as the info is on the Prepared Request. If it happens underneath the covers of the HTTPAdapter, then the answer is still "maybe" because the request may have the data you're looking for, but I'm not certain.

Owner

sigmavirus24 commented Nov 10, 2013

@Lukasa frankly, I don't understand the problem well. Proxies have always been your thing and I never attempted to get enough background on them.

So what Betamax does is replace the adapters that the Session object is using and then use a regular HTTPAdapter to make the actual request if necessary. The reason I'm not certain, is because of [get_connection](https://github.com/kennethreitz/requests/blob/master/requests/adapters.py#L187..L211] on the HTTPAdapter.

Betamax looks at the Prepared Request that comes through and then matches that against previously recorded (if any) requests to find an appropriate response. Because of how requests handles redirects, even redirects are recorded properly. If the logic you're looking for happens outside of the HTTPAdapter (any of the layers above it), Betamax can probably help so long as the info is on the Prepared Request. If it happens underneath the covers of the HTTPAdapter, then the answer is still "maybe" because the request may have the data you're looking for, but I'm not certain.

@Lukasa

This comment has been minimized.

Show comment Hide comment
@Lukasa

Lukasa Nov 10, 2013

Owner

Hmm. I'll have a think. This is awkward to test. It might be possible to stub out something really low level.

Owner

Lukasa commented Nov 10, 2013

Hmm. I'll have a think. This is awkward to test. It might be possible to stub out something really low level.

@rassie

This comment has been minimized.

Show comment Hide comment
@rassie

rassie Jan 28, 2014

Have there been any progress on this issue?

rassie commented Jan 28, 2014

Have there been any progress on this issue?

@Lukasa

This comment has been minimized.

Show comment Hide comment
@Lukasa

Lukasa Jan 28, 2014

Owner

Ah, sorry, there has not. I'll put this on my list of things to look at this week. =)

Owner

Lukasa commented Jan 28, 2014

Ah, sorry, there has not. I'll put this on my list of things to look at this week. =)

@blueyed

This comment has been minimized.

Show comment Hide comment
@blueyed

blueyed Sep 12, 2014

Contributor

This appears to get handled now via rebuild_proxies and rebuild_auth:
https://github.com/kennethreitz/requests/blob/master/requests/sessions.py#L175-177

(via 4d8cb32, should be 2.3.0)

Contributor

blueyed commented Sep 12, 2014

This appears to get handled now via rebuild_proxies and rebuild_auth:
https://github.com/kennethreitz/requests/blob/master/requests/sessions.py#L175-177

(via 4d8cb32, should be 2.3.0)

@Lukasa

This comment has been minimized.

Show comment Hide comment
@Lukasa

Lukasa Sep 12, 2014

Owner

Ah, correct, so it is. Thanks @blueyed!

Owner

Lukasa commented Sep 12, 2014

Ah, correct, so it is. Thanks @blueyed!

@Lukasa Lukasa closed this Sep 12, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment