Make WHEEL file errors more explicit (#7529)

* Raise exception on exception in finding wheel dist

We plan to replace this code with direct extraction from a zip, so no
point catching anything more precise.

* Raise exception if no dist is found in wheel_version
* Catch file read errors when reading WHEEL

get_metadata delegates to the underlying implementation which tries
to locate and read the file, throwing an IOError (Python 2) or OSError
subclass on any errors.

Since the new explicit test checks the same case as brokenwheel in
test_wheel_version we remove the redundant test.

* Check for WHEEL decoding errors explicitly

This was the last error that could be thrown by get_metadata, so we can
also remove the catch-all except block.

* Move WHEEL parsing outside try...except

This API does not raise an exception, but returns any errors on the
message object itself. We are preserving the original behavior, and can
decide later whether to start warning or raising our own exception.

* Raise explicit error if Wheel-Version is missing

`email.message.Message.__getitem__` returns None on missing values, so
we have to check for ourselves explicitly.

* Raise explicit exception on failure to parse Wheel-Version

This is also the last exception that can be raised, so we remove
`except Exception`.

* Remove dead code

Since wheel_version never returns None, this exception will never be
raised.

Author: chrahunt

src/pip/_internal/operations/install/wheel.py

@@ -324,7 +324,13 @@ def install_unpacked_wheel(
     # TODO: Look into moving this into a dedicated class for representing an
     #       installation.
 
-    version = wheel_version(wheeldir)
+    try:
+        version = wheel_version(wheeldir)
+    except UnsupportedWheel as e:
+        raise UnsupportedWheel(
+            "{} has an invalid wheel, {}".format(name, str(e))
+        )
+
     check_compatibility(version, name)
 
     if root_is_purelib(name, wheeldir):
@@ -651,25 +657,48 @@ def install_wheel(
 
 
 def wheel_version(source_dir):
-    # type: (Optional[str]) -> Optional[Tuple[int, ...]]
+    # type: (Optional[str]) -> Tuple[int, ...]
     """Return the Wheel-Version of an extracted wheel, if possible.
-    Otherwise, return None if we couldn't parse / extract it.
+    Otherwise, raise UnsupportedWheel if we couldn't parse / extract it.
     """
     try:
-        dist = [d for d in pkg_resources.find_on_path(None, source_dir)][0]
+        dists = [d for d in pkg_resources.find_on_path(None, source_dir)]
+    except Exception as e:
+        raise UnsupportedWheel(
+            "could not find a contained distribution due to: {!r}".format(e)
+        )
 
-        wheel_data = dist.get_metadata('WHEEL')
-        wheel_data = Parser().parsestr(wheel_data)
+    if not dists:
+        raise UnsupportedWheel("no contained distribution found")
 
-        version = wheel_data['Wheel-Version'].strip()
-        version = tuple(map(int, version.split('.')))
-        return version
-    except Exception:
-        return None
+    dist = dists[0]
+
+    try:
+        wheel_text = dist.get_metadata('WHEEL')
+    except (IOError, OSError) as e:
+        raise UnsupportedWheel("could not read WHEEL file: {!r}".format(e))
+    except UnicodeDecodeError as e:
+        raise UnsupportedWheel("error decoding WHEEL: {!r}".format(e))
+
+    # FeedParser (used by Parser) does not raise any exceptions. The returned
+    # message may have .defects populated, but for backwards-compatibility we
+    # currently ignore them.
+    wheel_data = Parser().parsestr(wheel_text)
+
+    version_text = wheel_data["Wheel-Version"]
+    if version_text is None:
+        raise UnsupportedWheel("WHEEL is missing Wheel-Version")
+
+    version = version_text.strip()
+
+    try:
+        return tuple(map(int, version.split('.')))
+    except ValueError:
+        raise UnsupportedWheel("invalid Wheel-Version: {!r}".format(version))
 
 
 def check_compatibility(version, name):
-    # type: (Optional[Tuple[int, ...]], str) -> None
+    # type: (Tuple[int, ...], str) -> None
     """Raises errors or warns if called with an incompatible Wheel-Version.
 
     Pip should refuse to install a Wheel-Version that's a major series
@@ -681,10 +710,6 @@ def check_compatibility(version, name):
 
     :raises UnsupportedWheel: when an incompatible Wheel-Version is given
     """
-    if not version:
-        raise UnsupportedWheel(
-            "%s is in an unsupported or invalid wheel" % name
-        )
     if version[0] > VERSION_COMPATIBLE[0]:
         raise UnsupportedWheel(
             "%s's Wheel-Version (%s) is not compatible with this version "

tests/lib/path.py

@@ -183,6 +183,11 @@ def read_bytes(self):
         with open(self, "rb") as fp:
             return fp.read()
 
+    def write_bytes(self, content):
+        # type: (bytes) -> None
+        with open(self, "wb") as f:
+            f.write(content)
+
     def read_text(self):
         with open(self, "r") as fp:
             return fp.read()

tests/unit/test_wheel.py

@@ -5,7 +5,8 @@
 import textwrap
 
 import pytest
-from mock import patch
+from mock import Mock, patch
+from pip._vendor import pkg_resources
 from pip._vendor.packaging.requirements import Requirement
 
 from pip._internal.exceptions import UnsupportedWheel
@@ -22,7 +23,7 @@
 from pip._internal.utils.compat import WINDOWS
 from pip._internal.utils.misc import hash_file
 from pip._internal.utils.unpacking import unpack_file
-from tests.lib import DATA_DIR, assert_paths_equal
+from tests.lib import DATA_DIR, assert_paths_equal, skip_if_python2
 
 
 def call_get_legacy_build_wheel_path(caplog, names):
@@ -191,14 +192,76 @@ def test_get_csv_rows_for_installed__long_lines(tmpdir, caplog):
 
 def test_wheel_version(tmpdir, data):
     future_wheel = 'futurewheel-1.9-py2.py3-none-any.whl'
-    broken_wheel = 'brokenwheel-1.0-py2.py3-none-any.whl'
     future_version = (1, 9)
 
     unpack_file(data.packages.joinpath(future_wheel), tmpdir + 'future')
-    unpack_file(data.packages.joinpath(broken_wheel), tmpdir + 'broken')
 
     assert wheel.wheel_version(tmpdir + 'future') == future_version
-    assert not wheel.wheel_version(tmpdir + 'broken')
+
+
+def test_wheel_version_fails_on_error(monkeypatch):
+    err = RuntimeError("example")
+    monkeypatch.setattr(pkg_resources, "find_on_path", Mock(side_effect=err))
+    with pytest.raises(UnsupportedWheel) as e:
+        wheel.wheel_version(".")
+    assert repr(err) in str(e.value)
+
+
+def test_wheel_version_fails_no_dists(tmpdir):
+    with pytest.raises(UnsupportedWheel) as e:
+        wheel.wheel_version(str(tmpdir))
+    assert "no contained distribution found" in str(e.value)
+
+
+def test_wheel_version_fails_missing_wheel(tmpdir):
+    dist_info_dir = tmpdir / "simple-0.1.0.dist-info"
+    dist_info_dir.mkdir()
+    dist_info_dir.joinpath("METADATA").touch()
+
+    with pytest.raises(UnsupportedWheel) as e:
+        wheel.wheel_version(str(tmpdir))
+    assert "could not read WHEEL file" in str(e.value)
+
+
+@skip_if_python2
+def test_wheel_version_fails_on_bad_encoding(tmpdir):
+    dist_info_dir = tmpdir / "simple-0.1.0.dist-info"
+    dist_info_dir.mkdir()
+    dist_info_dir.joinpath("METADATA").touch()
+    dist_info_dir.joinpath("WHEEL").write_bytes(b"\xff")
+
+    with pytest.raises(UnsupportedWheel) as e:
+        wheel.wheel_version(str(tmpdir))
+    assert "error decoding WHEEL" in str(e.value)
+
+
+def test_wheel_version_fails_on_no_wheel_version(tmpdir):
+    dist_info_dir = tmpdir / "simple-0.1.0.dist-info"
+    dist_info_dir.mkdir()
+    dist_info_dir.joinpath("METADATA").touch()
+    dist_info_dir.joinpath("WHEEL").touch()
+
+    with pytest.raises(UnsupportedWheel) as e:
+        wheel.wheel_version(str(tmpdir))
+    assert "missing Wheel-Version" in str(e.value)
+
+
+@pytest.mark.parametrize("version", [
+    ("",),
+    ("1.b",),
+    ("1.",),
+])
+def test_wheel_version_fails_on_bad_wheel_version(tmpdir, version):
+    dist_info_dir = tmpdir / "simple-0.1.0.dist-info"
+    dist_info_dir.mkdir()
+    dist_info_dir.joinpath("METADATA").touch()
+    dist_info_dir.joinpath("WHEEL").write_text(
+        "Wheel-Version: {}".format(version)
+    )
+
+    with pytest.raises(UnsupportedWheel) as e:
+        wheel.wheel_version(str(tmpdir))
+    assert "invalid Wheel-Version" in str(e.value)
 
 
 def test_check_compatibility():