Skip to content

if installing directly from a wheel, fail if it has an invalid name or is unsupported #1305

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

Closed
wants to merge 1 commit into from
Closed
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
8 changes: 8 additions & 0 deletions pip/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,11 @@ class PreviousBuildDirError(PipError):

class HashMismatch(InstallationError):
"""Distribution file hash values don't match."""


class InvalidWheelFilename(InstallationError):
"""Invalid wheel filename."""


class UnsupportedWheel(InstallationError):
"""Unsupported wheel."""
15 changes: 10 additions & 5 deletions pip/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

from pip.log import logger
from pip.util import Inf, normalize_name, splitext, is_prerelease
from pip.exceptions import DistributionNotFound, BestVersionAlreadyInstalled,\
InstallationError
from pip.exceptions import (DistributionNotFound, BestVersionAlreadyInstalled,
InstallationError, InvalidWheelFilename)
from pip.backwardcompat import urlparse, url2pathname
from pip.download import PipSession, path_to_url2, url_to_path
from pip.wheel import Wheel, wheel_ext, wheel_setuptools_support
Expand Down Expand Up @@ -481,6 +481,9 @@ def _link_package_versions(self, link, search_name):
logger.debug('Skipping link %s; macosx10 one' % (link))
self.logged_links.add(link)
return []
if link.invalid_wheel_filename:
logger.debug('Skipping %s because the wheel filename is invalid' % link)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

without this change, we currently hard fail if we encounter in invalid wheel filename. the package finder crashes

return []
if link.wheel:
if link.wheel.name.lower() != search_name.lower():
logger.debug('Skipping link %s; wrong project name (not %s)' % (link, search_name))
Expand Down Expand Up @@ -835,11 +838,13 @@ def __init__(self, url, comes_from=None, internal=None, trusted=None,
self.internal = internal
self.trusted = trusted
self._deprecated_regex = _deprecated_regex

# Set whether it's a wheel
self.invalid_wheel_filename = False
self.wheel = None
if url != Inf and self.splitext()[1] == wheel_ext:
self.wheel = Wheel(self.filename)
try:
self.wheel = Wheel(self.filename)
except InvalidWheelFilename:
self.invalid_wheel_filename = True

def __str__(self):
if self.comes_from:
Expand Down
11 changes: 9 additions & 2 deletions pip/req.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
from distutils.util import change_root
from pip.locations import (bin_py, running_under_virtualenv,PIP_DELETE_MARKER_FILENAME,
write_delete_marker_file)
from pip.exceptions import (InstallationError, UninstallationError,
BestVersionAlreadyInstalled,
from pip.exceptions import (InstallationError, UninstallationError, UnsupportedWheel,
BestVersionAlreadyInstalled, InvalidWheelFilename,
DistributionNotFound, PreviousBuildDirError)
from pip.vcs import vcs
from pip.log import logger
Expand Down Expand Up @@ -133,6 +133,13 @@ def from_line(cls, name, comes_from=None, prereleases=None):
if link.scheme == 'file' and re.search(r'\.\./', url):
url = path_to_url(os.path.normpath(os.path.abspath(link.path)))

# fail early for invalid or unsupported wheels
if link.invalid_wheel_filename:
raise InvalidWheelFilename("%s is an invalid wheel filename." % link.filename)
elif link.wheel and not link.wheel.supported():
raise UnsupportedWheel("%s is not a supported wheel on this platform." % link.wheel.filename)


else:
req = name

Expand Down
13 changes: 10 additions & 3 deletions pip/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from base64 import urlsafe_b64encode

from pip.backwardcompat import ConfigParser
from pip.exceptions import InvalidWheelFilename
from pip.locations import distutils_scheme
from pip.log import logger
from pip import pep425tags
Expand Down Expand Up @@ -385,7 +386,12 @@ class Wheel(object):
re.VERBOSE)

def __init__(self, filename):
"""
:raises InvalidWheelFilename: when the filename is invalid for a wheel
"""
wheel_info = self.wheel_file_re.match(filename)
if not wheel_info:
raise InvalidWheelFilename("%s is not a valid wheel filename." % filename)
self.filename = filename
self.name = wheel_info.group('name').replace('_', '-')
# we'll assume "_" means "-" due to wheel naming scheme
Expand All @@ -401,9 +407,10 @@ def __init__(self, filename):

def support_index_min(self, tags=None):
"""
Return the lowest index that a file_tag achieves in the supported_tags list
e.g. if there are 8 supported tags, and one of the file tags is first in the
list, then return 0.
Return the lowest index that one of the wheel's file_tag combinations
achieves in the supported_tags list e.g. if there are 8 supported tags,
and one of the file tags is first in the list, then return 0. Returns
None is the wheel is not supported.
"""
if tags is None: # for mock
tags = pep425tags.supported_tags
Expand Down
Empty file added tests/data/packages/invalid.whl
Empty file.
17 changes: 17 additions & 0 deletions tests/unit/test_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from pkg_resources import parse_version, Distribution
from pip.backwardcompat import urllib
from pip.log import logger
from pip.req import InstallRequirement
from pip.index import PackageFinder, Link
from pip.exceptions import BestVersionAlreadyInstalled, DistributionNotFound, InstallationError
Expand Down Expand Up @@ -89,6 +90,22 @@ def test_finder_detects_latest_already_satisfied_pypi_links():

class TestWheel:

def teardown(self):
logger.consumers = []

def test_skip_invalid_wheel_link(self, data):
"""
Test if PackageFinder skips invalid wheel filenames
"""
log = []
logger.add_consumers((logger.DEBUG, log.append))
req = InstallRequirement.from_line("invalid")
#data.find_links contains "invalid.whl", which is an invalid wheel
finder = PackageFinder([data.find_links], [], use_wheel=True)
with pytest.raises(DistributionNotFound):
finder.find_requirement(req, True)
"invalid.whl because the wheel filename is invalid" in "".join(log)

def test_not_find_wheel_not_supported(self, data, monkeypatch):
"""
Test not finding an unsupported wheel.
Expand Down
25 changes: 17 additions & 8 deletions tests/unit/test_req.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from pkg_resources import Distribution
from mock import Mock, patch
from pip.exceptions import PreviousBuildDirError
from pip.exceptions import PreviousBuildDirError, UnsupportedWheel, InvalidWheelFilename
from pip.index import PackageFinder
from pip.log import logger
from pip.req import (InstallRequirement, RequirementSet, parse_editable,
Expand Down Expand Up @@ -53,13 +53,22 @@ def test_no_reuse_existing_build_dir(self, data):
)


def test_url_with_query():
"""InstallRequirement should strip the fragment, but not the query."""
url = 'http://foo.com/?p=bar.git;a=snapshot;h=v0.1;sf=tgz'
fragment = '#egg=bar'
req = InstallRequirement.from_line(url + fragment)
class TestInstallRequirement(object):

assert req.url == url, req.url
def test_url_with_query(self):
"""InstallRequirement should strip the fragment, but not the query."""
url = 'http://foo.com/?p=bar.git;a=snapshot;h=v0.1;sf=tgz'
fragment = '#egg=bar'
req = InstallRequirement.from_line(url + fragment)
assert req.url == url, req.url

def test_unsupported_wheel_requirement_raises(self):
with pytest.raises(UnsupportedWheel):
req = InstallRequirement.from_line('peppercorn-0.4-py2.py3-bogus-any.whl')

def test_invalid_wheel_requirement_raises(self):
with pytest.raises(InvalidWheelFilename):
req = InstallRequirement.from_line('invalid.whl')


def test_requirements_data_structure_keeps_order():
Expand Down Expand Up @@ -110,7 +119,7 @@ def test_parse_editable_explicit_vcs():
{'egg': 'foo'})

def test_parse_editable_vcs_extras():
assert parse_editable('svn+https://foo#egg=foo[extras]', 'git') == ('foo[extras]',
assert parse_editable('svn+https://foo#egg=foo[extras]', 'git') == ('foo[extras]',
'svn+https://foo#egg=foo[extras]',
{'egg': 'foo[extras]'})

Expand Down
6 changes: 5 additions & 1 deletion tests/unit/test_wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pkg_resources
from mock import patch, Mock
from pip import wheel
from pip.exceptions import InstallationError
from pip.exceptions import InstallationError, InvalidWheelFilename
from pip.index import PackageFinder
from tests.lib import assert_raises_regexp

Expand Down Expand Up @@ -80,6 +80,10 @@ def test_finder_no_raises_error(self, monkeypatch):

class TestWheelFile(object):

def test_inavlid_filename_raises(self):
with pytest.raises(InvalidWheelFilename):
w = wheel.Wheel('invalid.whl')

def test_supported_single_version(self):
"""
Test single-version wheel is known to be supported
Expand Down