From dcece2e26252957ee538b1d66a92f5f659848035 Mon Sep 17 00:00:00 2001 From: Xuan Hu Date: Fri, 19 Apr 2024 13:27:12 +0800 Subject: [PATCH 1/5] fix: validate the absolute path as packages argument on Windows correctly --- changelog.d/1355.bugfix.md | 1 + src/pipx/main.py | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 changelog.d/1355.bugfix.md diff --git a/changelog.d/1355.bugfix.md b/changelog.d/1355.bugfix.md new file mode 100644 index 0000000000..3d3a564df3 --- /dev/null +++ b/changelog.d/1355.bugfix.md @@ -0,0 +1 @@ + validate the absolute path as packages argument on Windows correctly diff --git a/src/pipx/main.py b/src/pipx/main.py index c49f2d31ed..6903a1b097 100644 --- a/src/pipx/main.py +++ b/src/pipx/main.py @@ -174,6 +174,12 @@ def get_venv_args(parsed_args: Dict[str, str]) -> List[str]: return venv_args +def validate_package_arg(package: str): + url_parse_package = urllib.parse.urlparse(package) + if url_parse_package.scheme and url_parse_package.netloc: + raise PipxError("Package cannot be a url. Package name should be passed instead.") + + def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.ArgumentParser]) -> ExitCode: # noqa: C901 verbose = args.verbose if "verbose" in args else False if not constants.WINDOWS and args.is_global: @@ -185,9 +191,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar if "package" in args: package = args.package - url_parse_package = urllib.parse.urlparse(package) - if url_parse_package.scheme and url_parse_package.netloc: - raise PipxError("Package cannot be a url") + validate_package_arg(package) if "spec" in args and args.spec is not None: if urllib.parse.urlparse(args.spec).scheme: @@ -212,8 +216,8 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar logger.info(f"Virtual Environment location is {venv_dir}") if "packages" in args: - if any(urllib.parse.urlparse(package).scheme for package in args.packages): - raise PipxError("Package cannot be a url. Package name should be passed instead.") + for package in args.packages: + validate_package_arg(package) venv_dirs = {package: venv_container.get_venv_dir(package) for package in args.packages} venv_dirs_msg = "\n".join(f"- {key} : {value}" for key, value in venv_dirs.items()) logger.info(f"Virtual Environment locations are:\n{venv_dirs_msg}") From 8db65e466191d6ebee0f1143206e39ab9ff8ce67 Mon Sep 17 00:00:00 2001 From: Xuan Hu Date: Fri, 19 Apr 2024 14:01:06 +0800 Subject: [PATCH 2/5] add test --- tests/test_upgrade.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index b9ca2a8a65..94c4e1ec2a 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -116,3 +116,10 @@ def test_upgrade_multiple(pipx_temp_env, capsys): captured = capsys.readouterr() assert f"upgraded package {name} from {initial_version} to" in captured.out assert "pycowsay is already at latest version" in captured.out + + +def test_upgrade_absolute_path(pipx_temp_env, capsys, root): + print(str((root / "testdata" / "empty_project").resolve())) + assert run_pipx_cli(["upgrade", "--verbose", str((root / "testdata" / "empty_project").resolve())]) + captured = capsys.readouterr() + assert "Package cannot be a url" not in captured.err From 64b1320ca8d03b515cc0b6da4ba9431624055a37 Mon Sep 17 00:00:00 2001 From: "Xuan (Sean) Hu" Date: Sat, 20 Apr 2024 07:18:30 +0800 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: chrysle --- changelog.d/1355.bugfix.md | 2 +- src/pipx/main.py | 13 ++++++++----- tests/test_upgrade.py | 3 +-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/changelog.d/1355.bugfix.md b/changelog.d/1355.bugfix.md index 3d3a564df3..337075b624 100644 --- a/changelog.d/1355.bugfix.md +++ b/changelog.d/1355.bugfix.md @@ -1 +1 @@ - validate the absolute path as packages argument on Windows correctly +Validate whether a package is an URL correctly. diff --git a/src/pipx/main.py b/src/pipx/main.py index 6903a1b097..b6f2d34366 100644 --- a/src/pipx/main.py +++ b/src/pipx/main.py @@ -174,10 +174,13 @@ def get_venv_args(parsed_args: Dict[str, str]) -> List[str]: return venv_args -def validate_package_arg(package: str): +def package_is_url(package: str, raise: bool = True): -> bool url_parse_package = urllib.parse.urlparse(package) if url_parse_package.scheme and url_parse_package.netloc: - raise PipxError("Package cannot be a url. Package name should be passed instead.") + if not raise: + return True + raise PipxError("Package cannot be a URL. A valid package name should be passed instead.") + return False def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.ArgumentParser]) -> ExitCode: # noqa: C901 @@ -191,10 +194,10 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar if "package" in args: package = args.package - validate_package_arg(package) + package_is_url(package) if "spec" in args and args.spec is not None: - if urllib.parse.urlparse(args.spec).scheme: + if package_is_url(args.spec, raise=False): if "#egg=" not in args.spec: args.spec = args.spec + f"#egg={package}" @@ -217,7 +220,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar if "packages" in args: for package in args.packages: - validate_package_arg(package) + package_is_url(package) venv_dirs = {package: venv_container.get_venv_dir(package) for package in args.packages} venv_dirs_msg = "\n".join(f"- {key} : {value}" for key, value in venv_dirs.items()) logger.info(f"Virtual Environment locations are:\n{venv_dirs_msg}") diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index 94c4e1ec2a..d5fa0bfee9 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -119,7 +119,6 @@ def test_upgrade_multiple(pipx_temp_env, capsys): def test_upgrade_absolute_path(pipx_temp_env, capsys, root): - print(str((root / "testdata" / "empty_project").resolve())) assert run_pipx_cli(["upgrade", "--verbose", str((root / "testdata" / "empty_project").resolve())]) captured = capsys.readouterr() - assert "Package cannot be a url" not in captured.err + assert "Package cannot be a URL" not in captured.err From 947f258829a17e8bd093548fe5c1501666f88f68 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 19 Apr 2024 23:18:35 +0000 Subject: [PATCH 4/5] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/pipx/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pipx/main.py b/src/pipx/main.py index b6f2d34366..ba5358bcf0 100644 --- a/src/pipx/main.py +++ b/src/pipx/main.py @@ -180,7 +180,7 @@ def package_is_url(package: str, raise: bool = True): -> bool if not raise: return True raise PipxError("Package cannot be a URL. A valid package name should be passed instead.") - return False + return False def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.ArgumentParser]) -> ExitCode: # noqa: C901 From e024312fe87e01814508490c9b279c42906989ae Mon Sep 17 00:00:00 2001 From: Xuan Hu Date: Sat, 20 Apr 2024 07:24:38 +0800 Subject: [PATCH 5/5] fix test --- src/pipx/main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pipx/main.py b/src/pipx/main.py index ba5358bcf0..528bf2e1de 100644 --- a/src/pipx/main.py +++ b/src/pipx/main.py @@ -174,10 +174,10 @@ def get_venv_args(parsed_args: Dict[str, str]) -> List[str]: return venv_args -def package_is_url(package: str, raise: bool = True): -> bool +def package_is_url(package: str, raise_error: bool = True) -> bool: url_parse_package = urllib.parse.urlparse(package) if url_parse_package.scheme and url_parse_package.netloc: - if not raise: + if not raise_error: return True raise PipxError("Package cannot be a URL. A valid package name should be passed instead.") return False @@ -197,7 +197,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar package_is_url(package) if "spec" in args and args.spec is not None: - if package_is_url(args.spec, raise=False): + if package_is_url(args.spec, raise_error=False): if "#egg=" not in args.spec: args.spec = args.spec + f"#egg={package}"