From 864f0e0efa2701c9dc1120e6a80485e86da500ce Mon Sep 17 00:00:00 2001 From: Chris Hunt Date: Wed, 29 Jul 2020 17:51:58 -0400 Subject: [PATCH 1/3] Explicitly handle incorrect .data paths during wheel install Previously our wheel installation process allowed wheels which contained non-conforming contents in a contained .data directory. After the refactoring to enable direct-from-wheel installation, pip throws an exception when encountering these wheels, but does not include any helpful information to pinpoint the cause. Now if we encounter such a wheel, we trace an error that includes the name of the requirement we're trying to install, the path to the wheel file, the path we didn't understand, and a hint about what we expect. --- src/pip/_internal/operations/install/wheel.py | 10 +++++++++- tests/functional/test_install_wheel.py | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py index 8f73a88b074..0bd7b28be4f 100644 --- a/src/pip/_internal/operations/install/wheel.py +++ b/src/pip/_internal/operations/install/wheel.py @@ -583,7 +583,15 @@ def data_scheme_file_maker(zip_file, scheme): def make_data_scheme_file(record_path): # type: (RecordPath) -> File normed_path = os.path.normpath(record_path) - _, scheme_key, dest_subpath = normed_path.split(os.path.sep, 2) + try: + _, scheme_key, dest_subpath = normed_path.split(os.path.sep, 2) + except ValueError: + message = ( + "Unexpected file in {}: {!r}. .data directory contents" + " should be named like: '/'." + ).format(wheel_path, record_path) + raise InstallationError(message) + scheme_path = scheme_paths[scheme_key] dest_path = os.path.join(scheme_path, dest_subpath) assert_no_path_traversal(scheme_path, dest_path) diff --git a/tests/functional/test_install_wheel.py b/tests/functional/test_install_wheel.py index c53f13ca415..89d24a62fc0 100644 --- a/tests/functional/test_install_wheel.py +++ b/tests/functional/test_install_wheel.py @@ -681,3 +681,21 @@ def test_correct_package_name_while_creating_wheel_bug(script, package_name): package = create_basic_wheel_for_package(script, package_name, '1.0') wheel_name = os.path.basename(package) assert wheel_name == 'simple_package-1.0-py2.py3-none-any.whl' + + +@pytest.mark.parametrize("name", ["purelib", "abc"]) +def test_wheel_with_file_in_data_dir_has_reasonable_error( + script, tmpdir, name +): + """Normally we expect entities in the .data directory to be in a + subdirectory, but if they are not then we should show a reasonable error + message that includes the path. + """ + wheel_path = make_wheel( + "simple", "0.1.0", extra_data_files={name: "hello world"} + ).save_to_dir(tmpdir) + + result = script.pip( + "install", "--no-index", str(wheel_path), expect_error=True + ) + assert "simple-0.1.0.data/{}".format(name) in result.stderr From 3f9b326c115741a0a60e647e242961d80ccda08a Mon Sep 17 00:00:00 2001 From: Chris Hunt Date: Wed, 29 Jul 2020 18:06:25 -0400 Subject: [PATCH 2/3] Provide a reasonable error on invalid scheme keys Originally we would throw an `AttributeError` if a bad scheme key was used. After refactoring we would throw a `KeyError`, which isn't much better. Now we call out the wheel being processed, scheme key we didn't recognize, and provide a list of the valid scheme keys. This would likely be useful for people developing/testing the wheel. --- src/pip/_internal/operations/install/wheel.py | 14 +++++++++++++- tests/functional/test_install_wheel.py | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py index 0bd7b28be4f..681fc0aa8ef 100644 --- a/src/pip/_internal/operations/install/wheel.py +++ b/src/pip/_internal/operations/install/wheel.py @@ -592,7 +592,19 @@ def make_data_scheme_file(record_path): ).format(wheel_path, record_path) raise InstallationError(message) - scheme_path = scheme_paths[scheme_key] + try: + scheme_path = scheme_paths[scheme_key] + except KeyError: + valid_scheme_keys = ", ".join(sorted(scheme_paths)) + message = ( + "Unknown scheme key used in {}: {} (for file {!r}). .data" + " directory contents should be in subdirectories named" + " with a valid scheme key ({})" + ).format( + wheel_path, scheme_key, record_path, valid_scheme_keys + ) + raise InstallationError(message) + dest_path = os.path.join(scheme_path, dest_subpath) assert_no_path_traversal(scheme_path, dest_path) return ZipBackedFile(record_path, dest_path, zip_file) diff --git a/tests/functional/test_install_wheel.py b/tests/functional/test_install_wheel.py index 89d24a62fc0..ad4e749676f 100644 --- a/tests/functional/test_install_wheel.py +++ b/tests/functional/test_install_wheel.py @@ -699,3 +699,18 @@ def test_wheel_with_file_in_data_dir_has_reasonable_error( "install", "--no-index", str(wheel_path), expect_error=True ) assert "simple-0.1.0.data/{}".format(name) in result.stderr + + +def test_wheel_with_unknown_subdir_in_data_dir_has_reasonable_error( + script, tmpdir +): + wheel_path = make_wheel( + "simple", + "0.1.0", + extra_data_files={"unknown/hello.txt": "hello world"} + ).save_to_dir(tmpdir) + + result = script.pip( + "install", "--no-index", str(wheel_path), expect_error=True + ) + assert "simple-0.1.0.data/unknown/hello.txt" in result.stderr From 127c5b026c9e810a8bfb795662cd1be6567b8b64 Mon Sep 17 00:00:00 2001 From: Chris Hunt Date: Wed, 29 Jul 2020 18:14:58 -0400 Subject: [PATCH 3/3] Add news --- news/8654.bugfix | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 news/8654.bugfix diff --git a/news/8654.bugfix b/news/8654.bugfix new file mode 100644 index 00000000000..ec0df7a903e --- /dev/null +++ b/news/8654.bugfix @@ -0,0 +1,2 @@ +Trace a better error message on installation failure due to invalid ``.data`` +files in wheels.