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

Allow PEP508 url dependencies in install_requires #5571

Merged
merged 1 commit into from
Jul 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions news/4187.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Allow PEP 508 URL requirements to be used as dependencies.

As a security measure, pip will raise an exception when installing packages from
PyPI if those packages depend on packages not also hosted on PyPI.
In the future, PyPI will block uploading packages with such external URL dependencies directly.
10 changes: 8 additions & 2 deletions src/pip/_internal/models/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@


class Index(object):
def __init__(self, url):
def __init__(self, url, file_storage_domain):
self.url = url
self.netloc = urllib_parse.urlsplit(url).netloc
self.simple_url = self.url_to_path('simple')
self.pypi_url = self.url_to_path('pypi')

# This is part of a temporary hack used to block installs of PyPI
# packages which depend on external urls only necessary until PyPI can
# block such packages themselves
self.file_storage_domain = file_storage_domain

def url_to_path(self, path):
return urllib_parse.urljoin(self.url, path)


PyPI = Index('https://pypi.org/')
PyPI = Index('https://pypi.org/', 'files.pythonhosted.org')
TestPyPI = Index('https://test.pypi.org/', 'test-files.pythonhosted.org')
15 changes: 12 additions & 3 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from pip._internal.locations import (
PIP_DELETE_MARKER_FILENAME, running_under_virtualenv,
)
from pip._internal.models.index import PyPI, TestPyPI
from pip._internal.req.req_uninstall import UninstallPathSet
from pip._internal.utils.deprecation import (
RemovedInPip11Warning, RemovedInPip12Warning,
Expand Down Expand Up @@ -169,11 +170,19 @@ def from_req(cls, req, comes_from=None, isolated=False, wheel_cache=None):
req = Requirement(req)
except InvalidRequirement:
raise InstallationError("Invalid requirement: '%s'" % req)
if req.url:

domains_not_allowed = [
PyPI.file_storage_domain,
TestPyPI.file_storage_domain,
]
if req.url and comes_from.link.netloc in domains_not_allowed:

Choose a reason for hiding this comment

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

If comes_from.link is None, this crashes. This can happen if a package is already installed, and it was installed from a local wheel. I tested this on Pip 18.1 released today, but can't figure out how to make a minimum example.

Exception:
Traceback (most recent call last):
  File "<local>/venv/lib/python3.7/site-packages/pip/_internal/cli/base_command.py", line 143, in main
    status = self.run(options, args)
  File "<local>/venv/lib/python3.7/site-packages/pip/_internal/commands/install.py", line 318, in run
    resolver.resolve(requirement_set)
  File "<local>/venv/lib/python3.7/site-packages/pip/_internal/resolve.py", line 102, in resolve
    self._resolve_one(requirement_set, req)
  File "<local>/venv/lib/python3.7/site-packages/pip/_internal/resolve.py", line 318, in _resolve_one
    add_req(subreq, extras_requested=available_requested)
  File "<local>/venv/lib/python3.7/site-packages/pip/_internal/resolve.py", line 275, in add_req
    wheel_cache=self.wheel_cache,
  File "<local>/venv/lib/python3.7/site-packages/pip/_internal/req/constructors.py", line 288, in install_req_from_req
    if req.url and comes_from.link.netloc in domains_not_allowed:
AttributeError: 'NoneType' object has no attribute 'netloc'

Copy link
Member

Choose a reason for hiding this comment

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

@wkschwartz : can you check with #5788?

Choose a reason for hiding this comment

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

I think this line can simply be changed to

if req.url and comes_from.link is not None and comes_from.link.netloc in domains_not_allowed:

# Explicitly disallow pypi packages that depend on external urls
raise InstallationError(
"Direct url requirement (like %s) are not allowed for "
"dependencies" % req
"Packages installed from PyPI cannot depend on packages "
"which are not also hosted on PyPI.\n"
"%s depends on %s " % (comes_from.name, req)
)

return cls(req, comes_from, isolated=isolated, wheel_cache=wheel_cache)

@classmethod
Expand Down
29 changes: 26 additions & 3 deletions tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import pytest

from pip._internal import pep425tags
from pip._internal.models.index import PyPI, TestPyPI
from pip._internal.status_codes import ERROR
from pip._internal.utils.misc import rmtree
from tests.lib import (
Expand Down Expand Up @@ -1267,9 +1268,31 @@ def test_install_pep508_with_url_in_install_requires(script):
'ce1a869fe039fbf7e217df36c4653d1dbe657778b2d41709593a0003584405f4'
],
)
res = script.pip('install', pkga_path, expect_error=True)
assert "Direct url requirement " in res.stderr, str(res)
assert "are not allowed for dependencies" in res.stderr, str(res)
res = script.pip('install', pkga_path)
assert "Successfully installed packaging-15.3" in str(res), str(res)


@pytest.mark.network
@pytest.mark.parametrize('index', (PyPI.simple_url, TestPyPI.simple_url))
def test_install_from_test_pypi_with_ext_url_dep_is_blocked(script, index):
res = script.pip(
'install',
'--index-url',
index,
'pep-508-url-deps',
expect_error=True,
)
error_message = (
"Packages installed from PyPI cannot depend on packages "
"which are not also hosted on PyPI."
)
error_cause = (
"pep-508-url-deps depends on sampleproject@ "
"https:/pypa/sampleproject/archive/master.zip"
)
assert res.returncode == 1
assert error_message in res.stderr, str(res)
assert error_cause in res.stderr, str(res)


def test_installing_scripts_outside_path_prints_warning(script):
Expand Down