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

[2.0] Https proxy support #1515

Merged
merged 6 commits into from Aug 14, 2013

Conversation

Projects
None yet
7 participants
@schlamar
Contributor

schlamar commented Aug 3, 2013

No description provided.

@schlamar schlamar referenced this pull request Aug 3, 2013

Closed

HTTPS proxy support #1512

@Lukasa

This comment has been minimized.

Show comment
Hide comment
@Lukasa

Lukasa Aug 3, 2013

Member

Looks like these test failures are transient, I think this is good. Thanks so much for your work @schlamar, as well as all of the urllib3 people who worked on this! =)

Member

Lukasa commented Aug 3, 2013

Looks like these test failures are transient, I think this is good. Thanks so much for your work @schlamar, as well as all of the urllib3 people who worked on this! =)

@sigmavirus24

This comment has been minimized.

Show comment
Hide comment
@sigmavirus24

sigmavirus24 Aug 4, 2013

Member

Yeah once HTTPbin is pushed to heroku, these too will pass.

Member

sigmavirus24 commented Aug 4, 2013

Yeah once HTTPbin is pushed to heroku, these too will pass.

@Lukasa

This comment has been minimized.

Show comment
Hide comment
@Lukasa
Member

Lukasa commented Aug 4, 2013

+1.

@paragbaxi

This comment has been minimized.

Show comment
Hide comment
@paragbaxi

paragbaxi commented Aug 7, 2013

+1

@yfeng

This comment has been minimized.

Show comment
Hide comment
@yfeng

yfeng Aug 8, 2013

socks5 proxy support

yfeng commented Aug 8, 2013

socks5 proxy support

@schlamar

This comment has been minimized.

Show comment
Hide comment
@schlamar

schlamar Aug 8, 2013

Contributor

@yfengz This will probably take some time, see urllib3/urllib3#68

Contributor

schlamar commented Aug 8, 2013

@yfengz This will probably take some time, see urllib3/urllib3#68

# constructor's proxy_headers instead.
kw['headers'] = self._set_proxy_headers(url, kw.get('headers',
self.headers))
kw['headers'].update(self.proxy_headers)

This comment has been minimized.

@Lukasa

Lukasa Aug 13, 2013

Member

We need the set the Proxy-Authorization header regardless of HTTP or HTTPS. Is there a reason urllib3 can't do it? Because if there is, we'll need to fix it up in Requests.

@Lukasa

Lukasa Aug 13, 2013

Member

We need the set the Proxy-Authorization header regardless of HTTP or HTTPS. Is there a reason urllib3 can't do it? Because if there is, we'll need to fix it up in Requests.

This comment has been minimized.

@schlamar
@schlamar

This comment has been minimized.

@stanvit

stanvit Aug 14, 2013

Appropriate Proxy-Authorization header could be set in proxy_headers keyword argument for proxy_from_url(). I don't really think that proxy authentication belongs to urllib3, so I suggest to wrap proxy_from_url in requests with something like:

from urlparse import urlparse
import base64
from .packages.urllib3.poolmanager import  proxy_from_url

def authenticated_proxy_from_url(url, **kw):
    parsed_url = urlparse(url)
    if '@' in parsed_url.netloc:
        credentials = parsed_url.netloc.split('@')[0]
        base64string = base64.encodestring(credentials)[:-1]
        if 'proxy_headers' not in kw:
            kw['proxy_headers'] = {}
        kw['proxy_headers']['Proxy-Authorization'] = 'Basic %s' % base64string
    return proxy_from_url(url, **kw)

Of course it's the most crude example, there's room for further sophistication, such as authentication methods other than Basic.

@stanvit

stanvit Aug 14, 2013

Appropriate Proxy-Authorization header could be set in proxy_headers keyword argument for proxy_from_url(). I don't really think that proxy authentication belongs to urllib3, so I suggest to wrap proxy_from_url in requests with something like:

from urlparse import urlparse
import base64
from .packages.urllib3.poolmanager import  proxy_from_url

def authenticated_proxy_from_url(url, **kw):
    parsed_url = urlparse(url)
    if '@' in parsed_url.netloc:
        credentials = parsed_url.netloc.split('@')[0]
        base64string = base64.encodestring(credentials)[:-1]
        if 'proxy_headers' not in kw:
            kw['proxy_headers'] = {}
        kw['proxy_headers']['Proxy-Authorization'] = 'Basic %s' % base64string
    return proxy_from_url(url, **kw)

Of course it's the most crude example, there's room for further sophistication, such as authentication methods other than Basic.

This comment has been minimized.

@Lukasa

Lukasa Aug 14, 2013

Member

Alright, I think we should add this header as part of this PR. We have a get_auth_from_url function in utils.py which should be used for this. @schlamar you can do this yourself, or if you prefer I can raise a PR against your branch that includes the change.

@Lukasa

Lukasa Aug 14, 2013

Member

Alright, I think we should add this header as part of this PR. We have a get_auth_from_url function in utils.py which should be used for this. @schlamar you can do this yourself, or if you prefer I can raise a PR against your branch that includes the change.

This comment has been minimized.

@Lukasa

Lukasa Aug 14, 2013

Member

To my utter disgust, GitHub seems unwilling to let me open a PR against your fork, @schlamar. Are you happy to accept an emailed patch? =D

@Lukasa

Lukasa Aug 14, 2013

Member

To my utter disgust, GitHub seems unwilling to let me open a PR against your fork, @schlamar. Are you happy to accept an emailed patch? =D

This comment has been minimized.

@Lukasa

Lukasa Aug 14, 2013

Member

Yeah, I don't really know how GitHub decides that. There are loads of forks I can open PRs against, just not yours. Very strange. Anyway, I can just email the patch if it's faster.

@Lukasa

Lukasa Aug 14, 2013

Member

Yeah, I don't really know how GitHub decides that. There are loads of forks I can open PRs against, just not yours. Very strange. Anyway, I can just email the patch if it's faster.

This comment has been minimized.

@schlamar

schlamar Aug 14, 2013

Contributor

I guess

$ git remote add lukasa https://github.com/Lukasa/requests.git
$ git fetch lukasa
$ git merge lukasa/https-proxy-2.0

is faster than processing a patch per mail :)

@schlamar

schlamar Aug 14, 2013

Contributor

I guess

$ git remote add lukasa https://github.com/Lukasa/requests.git
$ git fetch lukasa
$ git merge lukasa/https-proxy-2.0

is faster than processing a patch per mail :)

This comment has been minimized.

@Lukasa

Lukasa Aug 14, 2013

Member

Almost certainly true. I blame GitHub for our pain today. =D

@Lukasa

Lukasa Aug 14, 2013

Member

Almost certainly true. I blame GitHub for our pain today. =D

This comment has been minimized.

@schlamar

schlamar Aug 14, 2013

Contributor

That network has a very large number of forks. For performance reasons, we don't show all of the available forked repositories in the Compare drop-down. We're looking into ways of improving this, but in the meantime you can generate a comparison by manually entering the URL. Try this one, for example:

Lukasa/requests@schlamar:master...master

@schlamar

schlamar Aug 14, 2013

Contributor

That network has a very large number of forks. For performance reasons, we don't show all of the available forked repositories in the Compare drop-down. We're looking into ways of improving this, but in the meantime you can generate a comparison by manually entering the URL. Try this one, for example:

Lukasa/requests@schlamar:master...master

This comment has been minimized.

@Lukasa

Lukasa Aug 14, 2013

Member

Ah, that makes perfect sense. I could have sworn I tried that, but clearly I didn't. Awesome!

@Lukasa

Lukasa Aug 14, 2013

Member

Ah, that makes perfect sense. I could have sworn I tried that, but clearly I didn't. Awesome!

Add proxy_headers functionality.
This brings us in line with urllib3's fancy new magic for sending
headers to HTTP(S) proxies.

kennethreitz added a commit that referenced this pull request Aug 14, 2013

@kennethreitz kennethreitz merged commit d8268fb into requests:2.0 Aug 14, 2013

1 check failed

default The Travis CI build failed
Details
@kennethreitz

This comment has been minimized.

Show comment
Hide comment
@kennethreitz

kennethreitz Aug 14, 2013

Member

🍰

Member

kennethreitz commented Aug 14, 2013

🍰

@schlamar schlamar deleted the schlamar:https-proxy-2.0 branch Aug 14, 2013

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