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

SOCKS Proxy Support #2953

Merged
merged 8 commits into from Apr 29, 2016
Merged

SOCKS Proxy Support #2953

merged 8 commits into from Apr 29, 2016

Conversation

Lukasa
Copy link
Member

@Lukasa Lukasa commented Dec 30, 2015

This pull request finally adds SOCKS proxy support to the Requests module, pretty much four years to the day since we were first asked (in #324 for those who want to follow it).

This pull request relies on a pull request I have opened against urllib3, so we can't land it until urllib3 1.14.1 is released containing that code. However, once that's done this PR should be ready to rock.

I'm setting this up for the 2.10.0 milestone as I see no reason to wait for 3.0.0 for SOCKS support.

Resolves #324, #723, #1147, #1982, #2156, #2425, and #2562.

@Lukasa Lukasa added this to the 2.10.0 milestone Dec 30, 2015
@jezdez
Copy link
Member

jezdez commented Dec 30, 2015

🙏

@@ -150,14 +156,26 @@ def proxy_manager_for(self, proxy, **proxy_kwargs):
:returns: ProxyManager
"""
if not proxy in self.proxy_manager:
Copy link
Contributor

Choose a reason for hiding this comment

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

While we're here, let's reformat this a bit:

if proxy in self.proxy_manager:
    manager = self.proxy_manager[proxy]
elif proxy.lower().startswith('socks'):
    username, password = get_auth_from_url(proxy)
    manager = self.proxy_manager[proxy] = SOCKSProxyManager(
        # ...
    )
else:
    proxy_headers = self.proxy_headers(proxy)
    manager = self.proxy_manager[proxy] = proxy_from_url(
        # ...
    )
return manager

I feel like this will be easier to read

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

if proxy:
proxy_scheme = urlparse(proxy).scheme

if proxy and proxy_scheme.lower().startswith('socks'):
Copy link
Contributor

Choose a reason for hiding this comment

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

I see how you were being tricksy now. I wonder if we can be better about this.

We essentially have the same code path (url = request.path_url) if we're using a 'socks' proxy or if we're making an HTTPS request. What if we do

proxied_http_request = proxy and scheme != 'https'
using_socks_proxy = False
if proxy:
    using_socks_proxy == urlparse(proxy).scheme.lower().startswith('socks')

url = request.path_url
if proxied_http_request and not using_socks_proxy:
    url = urldefragauth(request.url)

I puzzled through my logic with

proxy http_scheme && !socks_proxy_scheme https_scheme socks_proxy_scheme
truthy urldefragauth(request.url) request.path_url request.path_url
falsey request.path_url request.path_url request.path_url

where http_scheme is where (scheme != 'https') is True and https_scheme is where (scheme != 'https') is False. Hopefully the table makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I'm going with that. I degolfed this a little bit to aid clarity, but not much.

@sigmavirus24
Copy link
Contributor

👍 on the code and 👍 on the milestone.

@sigmavirus24
Copy link
Contributor

@Lukasa what do you think about having a socks extra for requests that installs PySocks and whatever else?

@Lukasa
Copy link
Member Author

Lukasa commented Dec 31, 2015

Done. =)

@kennethreitz kennethreitz changed the title [WIP] SOCKS proxy support [WIP] SOCKS Proxy Support Jan 30, 2016
@kennethreitz
Copy link
Contributor

Does PySocks require any compilation during installation? We should consider vendoring it if we want to consider this an actual feature of Requests.

@Lukasa
Copy link
Member Author

Lukasa commented Feb 2, 2016

PySocks requires no compilation. However, we'll have to rewrite import paths in urllib3 if you want to vendor PySocks.

@kennethreitz
Copy link
Contributor

Well, if it's going to be used by a very small subset of users (likely the case), it's likely fine to leave it as an optional dependency.

We could pack it in without modifying urllib3's code, technically:

(packages/__init__.py)

try:
    from . import socks
    sys.modules['socks'] = socks
except ImportError:
    try:
        import socks
    except ImportError:
        pass

This is obviously not the best idea, but it does function properly.

@sigmavirus24
Copy link
Contributor

Mucking with sys.modules in the past has caused us enough headaches that I'd rather not go too far down that path again

@kennethreitz
Copy link
Contributor

I don't disagree :)

@loretoparisi
Copy link

@Lukasa is this was merged or not? Btw, how to configure the socks proxy?

@Lukasa
Copy link
Member Author

Lukasa commented Apr 5, 2016

This has not been merged.

When it is released, you can configure the socks proxy as you would configure other proxies: proxies={'http': 'socks5://localhost:8888/'}, for example.

@loretoparisi
Copy link

@Lukasa so it should work like

#proxy
        # SOCKS5 proxy for HTTP/HTTPS
        proxies = {
            'http' : "socks5://myproxy:9191",
            'https' : "socks5://myproxy:9191"
        }

        #headers
        headers = {

        }

        url='http://icanhazip.com/'
        res = requests.get(url, headers=headers, proxies=proxies)

@Lukasa
Copy link
Member Author

Lukasa commented Apr 5, 2016

Correct. Of course, that won't work unless you use a patched version of requests that contains both this patch and an updated urllib3, and you install the appropriate optional dependencies.

@loretoparisi
Copy link

@Lukasa uhm, so supposed to use on google appengine, I have to wait the devappserver2 python sdk update...

@Lukasa
Copy link
Member Author

Lukasa commented Apr 5, 2016

On GAE I suspect using a SOCKS proxy is going to take a while. I should also note that to use a SOCKS proxy on GAE will require that you enable socket support, which as I understand it is substantially more expensive than simply using the standard httplib replacement.

@loretoparisi
Copy link

btw, In the meanwhile I'm doing

>>> import requesocks
>>> import requests
>>> session = requesocks.session()
>>> session.proxies = {
...     'http' : "socks5://myproxy.net:9050",
...     'https' : "socks5://smyproxy.net:9050"
... }
>>> url='http://icanhazip.com/'
>>> resp = requests.get(url)
>>> resp
<Response [200]>
>>> resp.text
u'89.97.90.230\n'
>>> resp.text.strip()
u'89.97.90.230'
>>> resp = session.get(url)
>>> resp.text.strip()
u'171.25.193.78'
>>> 

that in most envs will work ( a part again appengine that lacks of pwd support 😡

@kennethreitz
Copy link
Contributor

Is this done? Let's merge it if so.

@Lukasa
Copy link
Member Author

Lukasa commented Apr 6, 2016

We need an updated urllib3. I think @shazow is expecting to release this week.

@shazow
Copy link
Contributor

shazow commented Apr 6, 2016

Released!

@kennethreitz
Copy link
Contributor

Sweet, let's do it.

@kennethreitz
Copy link
Contributor

We'll include it in the next release, which can therefore happen this week (or even today).

@Lukasa
Copy link
Member Author

Lukasa commented Apr 6, 2016

@kennethreitz I'm away from my laptop, so if you're in a merging mood you can land the urllib3 update and then merge this.

Please note that it makes everyone's life easier if you merge in urllib3 directly from the brand new tag (1.15). Just a suggestion!

@darvid
Copy link

darvid commented Apr 10, 2016

The anticipation is killing me.

@dequis
Copy link

dequis commented Apr 24, 2016

What's [WIP] about this? Previous comments seem to suggest it's done, just waiting for the release.

(Also, referencing urllib3/urllib3#762 and #478)

@Lukasa Lukasa changed the title [WIP] SOCKS Proxy Support SOCKS Proxy Support Apr 24, 2016
brettdh referenced this pull request in brettdh/compose Apr 25, 2016
Currently, this has to be done by monkey-patching `socket.socket`,
using the [PySocks][pysocks] library to create a replacement that checks for
`ALL_PROXY` and routes connections through the proxy if present.

The hopefully-soon release of [Requests 2.10.0][requests] with [this
update][urllib3-socks] to [urllib3][urllib3-release] (assuming its adoption
docker-py), should make this unnecessary, but I'm not sure a) what's
been keeping that from happening; or b) whether docker-compose would
be able to quickly adopt the brand-new version of Requests (it's
currently a few minor versions behind), so this seems a reasonable
stopgap.

Signed-off-by: Brett Higgins <brhiggins@arbor.net>

[pysocks]: https://github.com/Anorov/PySocks
[requests]: https://github.com/kennethreitz/requests/pull/2953
[urllib3-socks]: urllib3/urllib3#762
[urllib3-release]: https://github.com/shazow/urllib3/blob/master/CHANGES.rst#114-2015-12-29
@kennethreitz kennethreitz merged commit 39d6935 into psf:master Apr 29, 2016
@kennethreitz
Copy link
Contributor

✨ 🍰 ✨

@darvid
Copy link

darvid commented Apr 29, 2016

DJ Khaled

@sigmavirus24 sigmavirus24 deleted the socks_proxy_support branch April 29, 2016 22:06
@kennethreitz
Copy link
Contributor

Released, announced.

brettdh referenced this pull request in brettdh/compose May 4, 2016
The recent release of [Requests 2.10.0][requests] with [this
update][urllib3-socks] to [urllib3][urllib3-release]
means that docker-compose can support a SOCKS proxy configured
via the HTTP[S]_PROXY environment variables.
We simply need to update the version of requests to
2.10.0, with the 'socks' extra, and the rest just works.

Signed-off-by: Brett Higgins <brhiggins@arbor.net>

[pysocks]: https://github.com/Anorov/PySocks
[requests]: https://github.com/kennethreitz/requests/pull/2953
[urllib3-socks]: urllib3/urllib3#762
[urllib3-release]: https://github.com/shazow/urllib3/blob/master/CHANGES.rst#114-2015-12-29

Signed-off-by: Brett Higgins <brhiggins@arbor.net>
brettdh referenced this pull request in brettdh/compose May 4, 2016
The recent release of [Requests 2.10.0][requests] with [this
update][urllib3-socks] to [urllib3][urllib3-release]
means that docker-compose can support a SOCKS proxy configured
via the HTTP[S]_PROXY environment variables.
We simply need to update the version of requests to
2.10.0, with the 'socks' extra, and the rest just works.

Signed-off-by: Brett Higgins <brhiggins@arbor.net>

[pysocks]: https://github.com/Anorov/PySocks
[requests]: https://github.com/kennethreitz/requests/pull/2953
[urllib3-socks]: urllib3/urllib3#762
[urllib3-release]: https://github.com/shazow/urllib3/blob/master/CHANGES.rst#114-2015-12-29

Signed-off-by: Brett Higgins <brhiggins@arbor.net>
brettdh referenced this pull request in brettdh/docker-py May 5, 2016
Related: docker/compose#3376

The recent release of [Requests 2.10.0][requests] with [this
update][urllib3-socks] to [urllib3][urllib3-release]
means that docker-py can support a SOCKS proxy configured
via the HTTP[S]_PROXY environment variables.
We simply need to update the version of requests to
2.10.0, with the 'socks' extra, and the rest just works.

[pysocks]: https://github.com/Anorov/PySocks
[requests]: kennethreitz/requests#2953
[urllib3-socks]: urllib3/urllib3#762
[urllib3-release]: https://github.com/shazow/urllib3/blob/master/CHANGES.rst#114-2015-12-29

Signed-off-by: Brett Higgins <brhiggins@arbor.net>
brettdh referenced this pull request in brettdh/compose May 9, 2016
The recent release of [Requests 2.10.0][requests] with [this
update][urllib3-socks] to [urllib3][urllib3-release]
means that docker-compose can support a SOCKS proxy configured
via the HTTP[S]_PROXY environment variables.
We simply need to update the version of requests to
2.10.0, with the 'socks' extra, and the rest just works.

[pysocks]: https://github.com/Anorov/PySocks
[requests]: https://github.com/kennethreitz/requests/pull/2953
[urllib3-socks]: urllib3/urllib3#762
[urllib3-release]: https://github.com/shazow/urllib3/blob/master/CHANGES.rst#114-2015-12-29

Signed-off-by: Brett Higgins <brhiggins@arbor.net>
brettdh referenced this pull request in brettdh/compose Sep 6, 2016
The recent release of [Requests 2.10.0][requests] with [this
update][urllib3-socks] to [urllib3][urllib3-release]
means that docker-compose can support a SOCKS proxy configured
via the HTTP[S]_PROXY environment variables.
We simply need to update the version of requests to
2.10.0, with the 'socks' extra, and the rest just works.

[pysocks]: https://github.com/Anorov/PySocks
[requests]: https://github.com/kennethreitz/requests/pull/2953
[urllib3-socks]: urllib3/urllib3#762
[urllib3-release]: https://github.com/shazow/urllib3/blob/master/CHANGES.rst#114-2015-12-29

Signed-off-by: Brett Higgins <brhiggins@arbor.net>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants