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

Vendor PySocks? #6668

Open
iamthad opened this issue Jul 1, 2019 · 19 comments
Open

Vendor PySocks? #6668

iamthad opened this issue Jul 1, 2019 · 19 comments
Labels
C: proxy Dealing with proxies and networking project: vendored dependency Related to a vendored dependency state: needs discussion This needs some more discussion type: enhancement Improvements to functionality

Comments

@iamthad
Copy link

iamthad commented Jul 1, 2019

What's the problem this feature will solve?
pip works with a SOCKS proxy if PySocks is installed. On a host which can access the internet only via a SOCKS proxy, it's possible to download get-pip.py e.g. through curl. However, since PySocks is not vendored in pip, running get-pip.py fails with requests.exceptions.InvalidSchema: Missing dependencies for SOCKS support..

Describe the solution you'd like
Consider vendoring PySocks along with urllib3 and Requests to allow better behavior when internet access is only available via a SOCKS proxy.

Alternative Solutions
It is possible to download the necessary packages on an internet-connected host using pip download, tar them up, copy them over via flash drive or scp, and then run get-pip.py with the --no-index and --find-links arguments pointing at these packages. I was able to get a working pip using this process after a couple of attempts.

It's probably also possible to download the necessary wheels manually through PyPI's web interface, but I did not attempt this.

Additional context
While the issue relates to the get-pip bootstrap, it seemed to be appropriate to report the issue on this repository since get-pip.py is just a self-extracting version of pip.

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Jul 1, 2019
@chrahunt
Copy link
Member

chrahunt commented Jul 5, 2019

PySocks:

  • pure Python
  • supports Python 2.7 and Python 3.4+
  • only dependency is win-inet-pton (for Python 2.7 on Windows)
  • BSD 3-Clause (link)

win-inet-pton:

  • pure Python (uses ctypes)
  • supports Python 2.7
  • no dependencies
  • "public domain" (link)

@uranusjr
Copy link
Member

uranusjr commented Jul 6, 2019

@chrahut Do you know whether pysocks (or requests, or pip as a whole) behaves correctly on non-Windows platforms if win-inet-pton is installed? This would affect how easy it is to vendor them (since vendored packages would be importatble at all times regardless of platform, and we’d need to patch things if win-inet-pton can only present on Windows).

@chrahunt
Copy link
Member

chrahunt commented Jul 6, 2019

  • I was able to successfully install and import win-inet-pton in Ubuntu 18.04 on Python 3.7.2 and 2.7.15.
  • PySocks guards the import of win-inet-pton behind a platform check here.
  • win-inet-pton also does a platform check here and importing it is a no-op otherwise.

Nothing else in pip references win-inet-pton, so given the above I think the non-Windows behavior should be the same.

@uranusjr
Copy link
Member

uranusjr commented Jul 7, 2019

Nice! It seems the vendoring would not be too complicated then. I’ll leave it to the core devs to decide whether to do it (I’m personally +0 because why not; the only downside is probably package size).

@pradyunsg
Copy link
Member

pradyunsg commented Jul 7, 2019

I don't have any major concerns with this.

Mostly, just the regular considerations with vendoring packages apply:

  • is it big?
  • is it a blocker?
  • does it depend on some C extension? i.e. not pure Python?
  • cross platform support?
  • how sustainable/stable is the project?

@pradyunsg pradyunsg reopened this Jul 7, 2019
@pradyunsg
Copy link
Member

/me hates GitHub's mobile UI.

@cjerdonek cjerdonek added C: proxy Dealing with proxies and networking project: vendored dependency Related to a vendored dependency state: needs discussion This needs some more discussion type: enhancement Improvements to functionality labels Jul 8, 2019
@triage-new-issues triage-new-issues bot removed S: needs triage Issues/PRs that need to be triaged labels Jul 8, 2019
@cjerdonek
Copy link
Member

cjerdonek commented Jul 8, 2019

Since this is a nice-to-have enhancement to begin with, what about the idea of only supporting this when win-inet-pton isn’t needed? It seems like it might be better not to complicate things too much overall for everything else when Python 2 is on its last legs anyways (and the absence of the feature would even be limited just to a subset of Python 2 users).

@pradyunsg
Copy link
Member

I'm fine with that. It's in line with our "Python 2.7 is community support" plan, and certain non-core features not making it to Python 2.7 is something I'm okay with as well.

@uranusjr
Copy link
Member

uranusjr commented Jul 8, 2019

Would we need to rewrite some import lines to make requests not blow up in the “PySocks is present, but not win-inet-pton” scenario? I’m also fine with it as long as it does not induce unnecessary patching work.

@cjerdonek
Copy link
Member

Someone would have to look into it. Is PySocks opt in or opt out? How does one do that?

@uranusjr
Copy link
Member

uranusjr commented Jul 8, 2019

My understanding is it’s opt-in; you just install PySocks along with Requests to get SOCKS support. I do not know how Requests implements the detection part, however.

@cjerdonek
Copy link
Member

  • how sustainable/stable is the project?

Regarding this, I noticed this message on PySocks's PyPI project page, which was added over two years ago:

Status Update
I no longer have the time to actively work on this project. I will gladly accept thoughtful pull requests and continue to update here and on PyPI in response to PRs, but I won't be putting in any changes of my own other than version bumps. If anyone would like to take the project off of my hands, please email me or create an issue. Thanks.

Is it wise to vendor a project whose maintainer no longer has time to work on it?

@cjerdonek
Copy link
Member

cjerdonek commented Jul 8, 2019

I do not know how Requests implements the detection part, however.

It looks like requests looks for an ImportError to know if it's not installed:

try:
from pip._vendor.urllib3.contrib.socks import SOCKSProxyManager
except ImportError:
def SOCKSProxyManager(*args, **kwargs):
raise InvalidSchema("Missing dependencies for SOCKS support.")

And the imported function (which raises InvalidSchema("Missing dependencies for SOCKS support.") if there was an ImportError) is only called if the proxy string starts with "socks":

elif proxy.lower().startswith('socks'):
username, password = get_auth_from_url(proxy)
manager = self.proxy_manager[proxy] = SOCKSProxyManager(
proxy,
username=username,
password=password,
num_pools=self._pool_connections,
maxsize=self._pool_maxsize,
block=self._pool_block,
**proxy_kwargs
)

And PySocks raises ImportError if win_inet_pton isn't installed (but only when it's required, i.e. Python 2 with Windows): https://github.com/Anorov/PySocks/blob/91dcdf0fec424b6afe9ceef88de63b72d2f8fcfe/socks.py#L19-L24

@chrahunt
Copy link
Member

chrahunt commented Jul 8, 2019

Regarding win-inet-pton, it does not seem complicated (single file, pure python), and it may be worth considering that the enterprise users most likely to need SOCKS proxy support are also the ones most likely to be on 2.7. I could see it saving several issues over the next 1-2 years to include it.

@uranusjr
Copy link
Member

uranusjr commented Jul 8, 2019

PySocks raises an ImportError when win-inet-pton is needed but not present, so IIUC no patching is needed. get-pip would simply emit the same error as it does now.

Regarding the maintainer issue… I don’t know? Maybe we should ask Requests maintainers for their opinion. (e.g. do they plan to continue using it for Requests 3)

@cjerdonek
Copy link
Member

and it may be worth considering that the enterprise users most likely to need SOCKS proxy support are also the ones most likely to be on 2.7. I could see it saving several issues over the next 1-2 years to include it.

If this feature was really needed for 2.7 (by the enterprise or not), it seems like it would have been requested before now, but I don't see it requested in the tracker. So I wonder if that 2.7 support is really worth it. Also, if we vendor, it seems like it would be one more thing that we'd need to keep updated.

By the way, after looking at the tracker, I see now that including PySocks was previously considered a few years ago (PR #4043), and it resulted in this code in pip's internal/__init__.py:

# 2016-06-17 barry@debian.org: urllib3 1.14 added optional support for socks,
# but if invoked (i.e. imported), it will issue a warning to stderr if socks
# isn't available. requests unconditionally imports urllib3's socks contrib
# module, triggering this warning. The warning breaks DEP-8 tests (because of
# the stderr output) and is just plain annoying in normal usage. I don't want
# to add socks as yet another dependency for pip, nor do I want to allow-stderr
# in the DEP-8 tests, so just suppress the warning. pdb tells me this has to
# be done before the import of pip.vcs.

@uranusjr
Copy link
Member

uranusjr commented Jul 8, 2019

Reading the PR (and its accompanying issue report), PySocks wasn’t considered for inclusion at all. I wonder if there’s a reason behind it (probably not?) or the absence was simply accepted as a fact.

@cjerdonek
Copy link
Member

I was simply referring to the text of the commit itself:

I don't want to add socks as yet another dependency for pip

If the author of the commit wrote that, it tells me they must have thought about adding a dependency. (I'm not saying, by the way, that I think we shouldn't vendor it. I'm just pointing out this history that hadn't been mentioned, even if only to point out a comment that should probably be updated.)

@Julian
Copy link
Contributor

Julian commented Sep 25, 2019

Besides a thumbs up from me (I found this issue just before opening my own for exactly the same request), this is extra difficult to deal with at the minute if you're also trying to coordinate installing into virtualenvs, because in our situation, we can reach PyPI for anything public, but our private packages not, and require SOCKS, but trying to bootstrap a venv you have to unset PIP_SOCKS, otherwise even the venv creation itself will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: proxy Dealing with proxies and networking project: vendored dependency Related to a vendored dependency state: needs discussion This needs some more discussion type: enhancement Improvements to functionality
Projects
None yet
Development

No branches or pull requests

6 participants