From 03c2ea0ff527bbe71b6b8f34a883a5aaef7d8c86 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 17 Oct 2021 18:41:30 +0200 Subject: [PATCH 1/6] Improve BadZipFile reporting --- news/10535.feature.rst | 2 ++ src/pip/_internal/exceptions.py | 11 +++++++++++ src/pip/_internal/metadata/pkg_resources.py | 14 ++++++++++++-- 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 news/10535.feature.rst diff --git a/news/10535.feature.rst b/news/10535.feature.rst new file mode 100644 index 00000000000..37ca23b055b --- /dev/null +++ b/news/10535.feature.rst @@ -0,0 +1,2 @@ +Catch ``BadZipFile`` errors and re-raise them as ``InvalidWheel`` which +additionally reports wheel name and path. diff --git a/src/pip/_internal/exceptions.py b/src/pip/_internal/exceptions.py index ef5bc75118b..f9b07eefb6c 100644 --- a/src/pip/_internal/exceptions.py +++ b/src/pip/_internal/exceptions.py @@ -132,6 +132,17 @@ class UnsupportedWheel(InstallationError): """Unsupported wheel.""" +class InvalidWheel(InstallationError): + """Invalid (e.g. corrupt) wheel.""" + + def __init__(self, location: str, name: str): + self.location = location + self.name = name + + def __str__(self) -> str: + return f"Wheel '{self.name}' located at {self.location} is invalid." + + class MetadataInconsistent(InstallationError): """Built metadata contains inconsistent information. diff --git a/src/pip/_internal/metadata/pkg_resources.py b/src/pip/_internal/metadata/pkg_resources.py index e8a8a38076a..0c054f8c973 100644 --- a/src/pip/_internal/metadata/pkg_resources.py +++ b/src/pip/_internal/metadata/pkg_resources.py @@ -1,12 +1,14 @@ import email.message import logging from typing import Collection, Iterable, Iterator, List, NamedTuple, Optional +from zipfile import BadZipFile from pip._vendor import pkg_resources from pip._vendor.packaging.requirements import Requirement from pip._vendor.packaging.utils import NormalizedName, canonicalize_name from pip._vendor.packaging.version import parse as parse_version +from pip._internal.exceptions import InvalidWheel from pip._internal.utils import misc # TODO: Move definition here. from pip._internal.utils.packaging import get_installer, get_metadata from pip._internal.utils.wheel import pkg_resources_distribution_for_wheel @@ -34,8 +36,16 @@ def __init__(self, dist: pkg_resources.Distribution) -> None: @classmethod def from_wheel(cls, wheel: Wheel, name: str) -> "Distribution": - with wheel.as_zipfile() as zf: - dist = pkg_resources_distribution_for_wheel(zf, name, wheel.location) + """Load the distribution from a given wheel. + + :raises InvalidWheel: Whenever loading of the wheel causes a + :py:exc:`zipfile.BadZipFile` exception to be thrown. + """ + try: + with wheel.as_zipfile() as zf: + dist = pkg_resources_distribution_for_wheel(zf, name, wheel.location) + except BadZipFile as e: + raise InvalidWheel(wheel.location, name) from e return cls(dist) @property From cd3aefe7ad4576bdb32a3eda20419a1a0e9e07a6 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 17 Oct 2021 20:05:25 +0200 Subject: [PATCH 2/6] adapt network test to `InvalidWheel` --- tests/unit/test_network_lazy_wheel.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_network_lazy_wheel.py b/tests/unit/test_network_lazy_wheel.py index 1d959d6b16a..42ff05080e8 100644 --- a/tests/unit/test_network_lazy_wheel.py +++ b/tests/unit/test_network_lazy_wheel.py @@ -1,6 +1,6 @@ from typing import Iterator -from zipfile import BadZipfile +from pip._internal.exceptions import InvalidWheel from pip._vendor.packaging.version import Version from pytest import fixture, mark, raises @@ -62,5 +62,5 @@ def test_dist_from_wheel_url_no_range( @mark.network def test_dist_from_wheel_url_not_zip(session: PipSession) -> None: """Test handling with the given URL does not point to a ZIP.""" - with raises(BadZipfile): + with raises(InvalidWheel): dist_from_wheel_url("python", "https://www.python.org/", session) From 58996b5ddb50dc0a15150224b27590d3ceb1a852 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 17 Oct 2021 20:06:08 +0200 Subject: [PATCH 3/6] Add tests for `BadZipFile` handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note that the functional test does not actually detect the behavioral change of throwing unhandled `BadZipFile` → throwing unhandled `InvalidWheel`, whereas the unit test does. --- .../packages/corruptwheel-1.0-py2.py3-none-any.whl | 1 + tests/functional/test_install_wheel.py | 11 +++++++++-- tests/unit/test_wheel.py | 9 +++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 tests/data/packages/corruptwheel-1.0-py2.py3-none-any.whl diff --git a/tests/data/packages/corruptwheel-1.0-py2.py3-none-any.whl b/tests/data/packages/corruptwheel-1.0-py2.py3-none-any.whl new file mode 100644 index 00000000000..bf285f13f40 --- /dev/null +++ b/tests/data/packages/corruptwheel-1.0-py2.py3-none-any.whl @@ -0,0 +1 @@ +This is a corrupt wheel which _clearly_ is not a zip file. diff --git a/tests/functional/test_install_wheel.py b/tests/functional/test_install_wheel.py index a87fe293311..c7045a9dc5b 100644 --- a/tests/functional/test_install_wheel.py +++ b/tests/functional/test_install_wheel.py @@ -45,13 +45,20 @@ def test_install_from_future_wheel_version(script, tmpdir): result.assert_installed("futurewheel", without_egg_link=True, editable=False) -def test_install_from_broken_wheel(script, data): +@pytest.mark.parametrize( + "wheel_name", + [ + "brokenwheel-1.0-py2.py3-none-any.whl", + "corruptwheel-1.0-py2.py3-none-any.whl", + ], +) +def test_install_from_broken_wheel(script, data, wheel_name): """ Test that installing a broken wheel fails properly """ from tests.lib import TestFailure - package = data.packages.joinpath("brokenwheel-1.0-py2.py3-none-any.whl") + package = data.packages.joinpath(wheel_name) result = script.pip("install", package, "--no-index", expect_error=True) with pytest.raises(TestFailure): result.assert_installed("futurewheel", without_egg_link=True, editable=False) diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 37b5974eb39..682027d152b 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -252,6 +252,15 @@ def test_wheel_root_is_purelib(text: str, expected: bool) -> None: assert wheel.wheel_root_is_purelib(message_from_string(text)) == expected +def test_dist_from_broken_wheel_fails(data) -> None: + from pip._internal.exceptions import InvalidWheel + from pip._internal.metadata import get_wheel_distribution, FilesystemWheel + + package = data.packages.joinpath("corruptwheel-1.0-py2.py3-none-any.whl") + with pytest.raises(InvalidWheel): + get_wheel_distribution(FilesystemWheel(package), "brokenwheel") + + class TestWheelFile: def test_unpack_wheel_no_flatten(self, tmpdir: Path) -> None: filepath = os.path.join(DATA_DIR, "packages", "meta-1.0-py2.py3-none-any.whl") From 2e67986782b47828f463e43819dd8ab145cc5d34 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sun, 17 Oct 2021 22:23:54 +0200 Subject: [PATCH 4/6] make linters happy --- tests/unit/test_network_lazy_wheel.py | 2 +- tests/unit/test_wheel.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_network_lazy_wheel.py b/tests/unit/test_network_lazy_wheel.py index 42ff05080e8..79e86321793 100644 --- a/tests/unit/test_network_lazy_wheel.py +++ b/tests/unit/test_network_lazy_wheel.py @@ -1,9 +1,9 @@ from typing import Iterator -from pip._internal.exceptions import InvalidWheel from pip._vendor.packaging.version import Version from pytest import fixture, mark, raises +from pip._internal.exceptions import InvalidWheel from pip._internal.network.lazy_wheel import ( HTTPRangeRequestUnsupported, dist_from_wheel_url, diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 682027d152b..a698656b2df 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -252,9 +252,9 @@ def test_wheel_root_is_purelib(text: str, expected: bool) -> None: assert wheel.wheel_root_is_purelib(message_from_string(text)) == expected -def test_dist_from_broken_wheel_fails(data) -> None: +def test_dist_from_broken_wheel_fails(data: TestData) -> None: from pip._internal.exceptions import InvalidWheel - from pip._internal.metadata import get_wheel_distribution, FilesystemWheel + from pip._internal.metadata import FilesystemWheel, get_wheel_distribution package = data.packages.joinpath("corruptwheel-1.0-py2.py3-none-any.whl") with pytest.raises(InvalidWheel): From add2ab0fb5cbcc285c0547c842b3294e8ddfaf1a Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 23 Oct 2021 18:23:56 +0200 Subject: [PATCH 5/6] Re-word news entry to hide implementation details --- news/10535.feature.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/news/10535.feature.rst b/news/10535.feature.rst index 37ca23b055b..4476eaaa5c1 100644 --- a/news/10535.feature.rst +++ b/news/10535.feature.rst @@ -1,2 +1,2 @@ -Catch ``BadZipFile`` errors and re-raise them as ``InvalidWheel`` which -additionally reports wheel name and path. +Catch exception when an invalid wheel is accessed +and provide more information about the culprit in a new stacktrace. From a51a6447c0b4770e6e7186074432d110a8248f85 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Wed, 10 Nov 2021 13:29:14 +0100 Subject: [PATCH 6/6] Improve wording of feature summary 10535 Co-authored-by: Pradyun Gedam --- news/10535.feature.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/news/10535.feature.rst b/news/10535.feature.rst index 4476eaaa5c1..f843012592b 100644 --- a/news/10535.feature.rst +++ b/news/10535.feature.rst @@ -1,2 +1 @@ -Catch exception when an invalid wheel is accessed -and provide more information about the culprit in a new stacktrace. +Present a better error message when an invalid wheel file is encountered, providing more context where the invalid wheel file is.