From e92de61ee01c8676a23b10189d94890931ad166a Mon Sep 17 00:00:00 2001 From: David Hotham Date: Mon, 13 Feb 2023 17:05:48 +0000 Subject: [PATCH] don't set shell=True on subprocess calls (#7471) --- src/poetry/utils/_compat.py | 8 ---- src/poetry/utils/env.py | 55 +++++++-------------------- tests/console/commands/env/helpers.py | 19 ++++----- tests/utils/test_env.py | 23 +++++------ 4 files changed, 36 insertions(+), 69 deletions(-) diff --git a/src/poetry/utils/_compat.py b/src/poetry/utils/_compat.py index 1a37ad13e8b..5887436e530 100644 --- a/src/poetry/utils/_compat.py +++ b/src/poetry/utils/_compat.py @@ -60,19 +60,11 @@ def to_str(string: str) -> str: return decode(string) -def list_to_shell_command(cmd: list[str]) -> str: - return " ".join( - f'"{token}"' if " " in token and token[0] not in {"'", '"'} else token - for token in cmd - ) - - __all__ = [ "WINDOWS", "cached_property", "decode", "encode", - "list_to_shell_command", "metadata", "to_str", "tomllib", diff --git a/src/poetry/utils/env.py b/src/poetry/utils/env.py index 70f4b9b4772..ac4bc547c9e 100644 --- a/src/poetry/utils/env.py +++ b/src/poetry/utils/env.py @@ -40,7 +40,6 @@ from poetry.utils._compat import WINDOWS from poetry.utils._compat import decode from poetry.utils._compat import encode -from poetry.utils._compat import list_to_shell_command from poetry.utils._compat import metadata from poetry.utils.helpers import get_real_windows_path from poetry.utils.helpers import is_dir_writable @@ -171,9 +170,9 @@ def _version_nodot(version): """ GET_PYTHON_VERSION_ONELINER = ( - "\"import sys; print('.'.join([str(s) for s in sys.version_info[:3]]))\"" + "import sys; print('.'.join([str(s) for s in sys.version_info[:3]]))" ) -GET_ENV_PATH_ONELINER = '"import sys; print(sys.prefix)"' +GET_ENV_PATH_ONELINER = "import sys; print(sys.prefix)" GET_SYS_PATH = """\ import json @@ -522,10 +521,7 @@ def _full_python_path(python: str) -> str: try: executable = decode( subprocess.check_output( - list_to_shell_command( - [python, "-c", '"import sys; print(sys.executable)"'] - ), - shell=True, + [python, "-c", "import sys; print(sys.executable)"], ).strip() ) except CalledProcessError as e: @@ -572,10 +568,7 @@ def get_python_version( if executable: python_patch = decode( subprocess.check_output( - list_to_shell_command( - [executable, "-c", GET_PYTHON_VERSION_ONELINER] - ), - shell=True, + [executable, "-c", GET_PYTHON_VERSION_ONELINER], ).strip() ) @@ -603,8 +596,7 @@ def activate(self, python: str) -> Env: try: python_version_string = decode( subprocess.check_output( - list_to_shell_command([python, "-c", GET_PYTHON_VERSION_ONELINER]), - shell=True, + [python, "-c", GET_PYTHON_VERSION_ONELINER], ) ) except CalledProcessError as e: @@ -799,8 +791,7 @@ def remove(self, python: str) -> Env: try: env_dir = decode( subprocess.check_output( - list_to_shell_command([python, "-c", GET_ENV_PATH_ONELINER]), - shell=True, + [python, "-c", GET_ENV_PATH_ONELINER], ) ).strip("\n") env_name = Path(env_dir).name @@ -859,8 +850,7 @@ def remove(self, python: str) -> Env: try: python_version_string = decode( subprocess.check_output( - list_to_shell_command([python, "-c", GET_PYTHON_VERSION_ONELINER]), - shell=True, + [python, "-c", GET_PYTHON_VERSION_ONELINER], ) ) except CalledProcessError as e: @@ -935,10 +925,7 @@ def create_venv( if executable: python_patch = decode( subprocess.check_output( - list_to_shell_command( - [executable, "-c", GET_PYTHON_VERSION_ONELINER] - ), - shell=True, + [executable, "-c", GET_PYTHON_VERSION_ONELINER], ).strip() ) python_minor = ".".join(python_patch.split(".")[:2]) @@ -985,11 +972,8 @@ def create_venv( try: python_patch = decode( subprocess.check_output( - list_to_shell_command( - [python, "-c", GET_PYTHON_VERSION_ONELINER] - ), + [python, "-c", GET_PYTHON_VERSION_ONELINER], stderr=subprocess.STDOUT, - shell=True, ).strip() ) except CalledProcessError: @@ -1531,18 +1515,9 @@ def _run(self, cmd: list[str], **kwargs: Any) -> int | str: env = kwargs.pop("env", dict(os.environ)) try: - if self._is_windows: - kwargs["shell"] = True - - command: str | list[str] - if kwargs.get("shell", False): - command = list_to_shell_command(cmd) - else: - command = cmd - if input_: output = subprocess.run( - command, + cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, input=encode(input_), @@ -1550,12 +1525,10 @@ def _run(self, cmd: list[str], **kwargs: Any) -> int | str: **kwargs, ).stdout elif call: - return subprocess.call( - command, stderr=subprocess.STDOUT, env=env, **kwargs - ) + return subprocess.call(cmd, stderr=subprocess.STDOUT, env=env, **kwargs) else: output = subprocess.check_output( - command, stderr=subprocess.STDOUT, env=env, **kwargs + cmd, stderr=subprocess.STDOUT, env=env, **kwargs ) except CalledProcessError as e: raise EnvCommandError(e, input=input_) @@ -1570,7 +1543,7 @@ def execute(self, bin: str, *args: str, **kwargs: Any) -> int: return os.execvpe(command[0], command, env=env) kwargs["shell"] = True - exe = subprocess.Popen([command[0]] + command[1:], env=env, **kwargs) + exe = subprocess.Popen(command, env=env, **kwargs) exe.communicate() return exe.returncode @@ -1909,7 +1882,7 @@ def execute(self, bin: str, *args: str, **kwargs: Any) -> int: if not self._is_windows: return os.execvpe(command[0], command, env=env) - exe = subprocess.Popen([command[0]] + command[1:], env=env, **kwargs) + exe = subprocess.Popen(command, env=env, **kwargs) exe.communicate() return exe.returncode diff --git a/tests/console/commands/env/helpers.py b/tests/console/commands/env/helpers.py index 0bf94128154..0a067b3c430 100644 --- a/tests/console/commands/env/helpers.py +++ b/tests/console/commands/env/helpers.py @@ -10,8 +10,6 @@ if TYPE_CHECKING: from collections.abc import Callable - from poetry.core.version.pep440.version import PEP440Version - VERSION_3_7_1 = Version.parse("3.7.1") @@ -20,16 +18,19 @@ def build_venv(path: Path | str, **_: Any) -> None: def check_output_wrapper( - version: PEP440Version = VERSION_3_7_1, -) -> Callable[[str, Any, Any], str]: - def check_output(cmd: str, *_: Any, **__: Any) -> str: - if "sys.version_info[:3]" in cmd: + version: Version = VERSION_3_7_1, +) -> Callable[[list[str], Any, Any], str]: + def check_output(cmd: list[str], *args: Any, **kwargs: Any) -> str: + # cmd is a list, like ["python", "-c", "do stuff"] + python_cmd = cmd[2] + if "sys.version_info[:3]" in python_cmd: return version.text - elif "sys.version_info[:2]" in cmd: + elif "sys.version_info[:2]" in python_cmd: return f"{version.major}.{version.minor}" - elif '-c "import sys; print(sys.executable)"' in cmd: - return f"/usr/bin/{cmd.split()[0]}" + elif "import sys; print(sys.executable)" in python_cmd: + return f"/usr/bin/{cmd[0]}" else: + assert "import sys; print(sys.prefix)" in python_cmd return str(Path("/prefix")) return check_output diff --git a/tests/utils/test_env.py b/tests/utils/test_env.py index 1d9f6c3ba63..86bf4dfbc50 100644 --- a/tests/utils/test_env.py +++ b/tests/utils/test_env.py @@ -94,7 +94,7 @@ def test_virtualenvs_with_spaces_in_their_path_work_as_expected( venv = VirtualEnv(venv_path) - assert venv.run("python", "-V", shell=True).startswith("Python") + assert venv.run("python", "-V").startswith("Python") @pytest.mark.skipif(sys.platform != "darwin", reason="requires darwin") @@ -124,7 +124,7 @@ def test_env_commands_with_spaces_in_their_arg_work_as_expected( venv_path = Path(tmp_dir) / "Virtual Env" manager.build_venv(str(venv_path)) venv = VirtualEnv(venv_path) - assert venv.run("python", venv.pip, "--version", shell=True).startswith( + assert venv.run("python", venv.pip, "--version").startswith( f"pip {venv.pip_version} from " ) @@ -135,9 +135,7 @@ def test_env_shell_commands_with_stdinput_in_their_arg_work_as_expected( venv_path = Path(tmp_dir) / "Virtual Env" manager.build_venv(str(venv_path)) venv = VirtualEnv(venv_path) - run_output_path = Path( - venv.run("python", "-", input_=GET_BASE_PREFIX, shell=True).strip() - ) + run_output_path = Path(venv.run("python", "-", input_=GET_BASE_PREFIX).strip()) venv_base_prefix_path = Path(str(venv.get_base_prefix())) assert run_output_path.resolve() == venv_base_prefix_path.resolve() @@ -189,15 +187,18 @@ def build_venv(path: Path | str, **__: Any) -> None: def check_output_wrapper( version: Version = VERSION_3_7_1, -) -> Callable[[str, Any, Any], str]: - def check_output(cmd: str, *args: Any, **kwargs: Any) -> str: - if "sys.version_info[:3]" in cmd: +) -> Callable[[list[str], Any, Any], str]: + def check_output(cmd: list[str], *args: Any, **kwargs: Any) -> str: + # cmd is a list, like ["python", "-c", "do stuff"] + python_cmd = cmd[2] + if "sys.version_info[:3]" in python_cmd: return version.text - elif "sys.version_info[:2]" in cmd: + elif "sys.version_info[:2]" in python_cmd: return f"{version.major}.{version.minor}" - elif '-c "import sys; print(sys.executable)"' in cmd: - return f"/usr/bin/{cmd.split()[0]}" + elif "import sys; print(sys.executable)" in python_cmd: + return f"/usr/bin/{cmd[0]}" else: + assert "import sys; print(sys.prefix)" in python_cmd return str(Path("/prefix")) return check_output