Skip to content

P
python-poetry
Commit e92de61e by David Hotham Committed by GitHub
Options

don't set shell=True on subprocess calls (#7471)

parent 8558f0cb
Showing with 36 additions and 69 deletions
src/poetry/utils/_compat.py
...@@ -60,19 +60,11 @@ def to_str(string: str) -> str: ...@@ -60,19 +60,11 @@ def to_str(string: str) -> str:
return decode(string) 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__ = [ __all__ = [
"WINDOWS", "WINDOWS",
"cached_property", "cached_property",
"decode", "decode",
"encode", "encode",
"list_to_shell_command",
"metadata", "metadata",
"to_str", "to_str",
"tomllib", "tomllib",
......
src/poetry/utils/env.py
...@@ -40,7 +40,6 @@ from virtualenv.seed.wheels.embed import get_embed_wheel ...@@ -40,7 +40,6 @@ from virtualenv.seed.wheels.embed import get_embed_wheel
from poetry.utils._compat import WINDOWS from poetry.utils._compat import WINDOWS
from poetry.utils._compat import decode from poetry.utils._compat import decode
from poetry.utils._compat import encode from poetry.utils._compat import encode
from poetry.utils._compat import list_to_shell_command
from poetry.utils._compat import metadata from poetry.utils._compat import metadata
from poetry.utils.helpers import get_real_windows_path from poetry.utils.helpers import get_real_windows_path
from poetry.utils.helpers import is_dir_writable from poetry.utils.helpers import is_dir_writable
...@@ -171,9 +170,9 @@ print('.'.join([str(s) for s in sys.version_info[:3]])) ...@@ -171,9 +170,9 @@ print('.'.join([str(s) for s in sys.version_info[:3]]))
""" """
GET_PYTHON_VERSION_ONELINER = ( 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 = """\ GET_SYS_PATH = """\
import json import json
...@@ -522,10 +521,7 @@ class EnvManager: ...@@ -522,10 +521,7 @@ class EnvManager:
try: try:
executable = decode( executable = decode(
subprocess.check_output( subprocess.check_output(
list_to_shell_command( [python, "-c", "import sys; print(sys.executable)"],
[python, "-c", '"import sys; print(sys.executable)"']
),
shell=True,
).strip() ).strip()
) )
except CalledProcessError as e: except CalledProcessError as e:
...@@ -572,10 +568,7 @@ class EnvManager: ...@@ -572,10 +568,7 @@ class EnvManager:
if executable: if executable:
python_patch = decode( python_patch = decode(
subprocess.check_output( subprocess.check_output(
list_to_shell_command( [executable, "-c", GET_PYTHON_VERSION_ONELINER],
[executable, "-c", GET_PYTHON_VERSION_ONELINER]
),
shell=True,
).strip() ).strip()
) )
...@@ -603,8 +596,7 @@ class EnvManager: ...@@ -603,8 +596,7 @@ class EnvManager:
try: try:
python_version_string = decode( python_version_string = decode(
subprocess.check_output( subprocess.check_output(
list_to_shell_command([python, "-c", GET_PYTHON_VERSION_ONELINER]), [python, "-c", GET_PYTHON_VERSION_ONELINER],
shell=True,
) )
) )
except CalledProcessError as e: except CalledProcessError as e:
...@@ -799,8 +791,7 @@ class EnvManager: ...@@ -799,8 +791,7 @@ class EnvManager:
try: try:
env_dir = decode( env_dir = decode(
subprocess.check_output( subprocess.check_output(
list_to_shell_command([python, "-c", GET_ENV_PATH_ONELINER]), [python, "-c", GET_ENV_PATH_ONELINER],
shell=True,
) )
).strip("\n") ).strip("\n")
env_name = Path(env_dir).name env_name = Path(env_dir).name
...@@ -859,8 +850,7 @@ class EnvManager: ...@@ -859,8 +850,7 @@ class EnvManager:
try: try:
python_version_string = decode( python_version_string = decode(
subprocess.check_output( subprocess.check_output(
list_to_shell_command([python, "-c", GET_PYTHON_VERSION_ONELINER]), [python, "-c", GET_PYTHON_VERSION_ONELINER],
shell=True,
) )
) )
except CalledProcessError as e: except CalledProcessError as e:
...@@ -935,10 +925,7 @@ class EnvManager: ...@@ -935,10 +925,7 @@ class EnvManager:
if executable: if executable:
python_patch = decode( python_patch = decode(
subprocess.check_output( subprocess.check_output(
list_to_shell_command( [executable, "-c", GET_PYTHON_VERSION_ONELINER],
[executable, "-c", GET_PYTHON_VERSION_ONELINER]
),
shell=True,
).strip() ).strip()
) )
python_minor = ".".join(python_patch.split(".")[:2]) python_minor = ".".join(python_patch.split(".")[:2])
...@@ -985,11 +972,8 @@ class EnvManager: ...@@ -985,11 +972,8 @@ class EnvManager:
try: try:
python_patch = decode( python_patch = decode(
subprocess.check_output( subprocess.check_output(
list_to_shell_command( [python, "-c", GET_PYTHON_VERSION_ONELINER],
[python, "-c", GET_PYTHON_VERSION_ONELINER]
),
stderr=subprocess.STDOUT, stderr=subprocess.STDOUT,
shell=True,
).strip() ).strip()
) )
except CalledProcessError: except CalledProcessError:
...@@ -1531,18 +1515,9 @@ class Env: ...@@ -1531,18 +1515,9 @@ class Env:
env = kwargs.pop("env", dict(os.environ)) env = kwargs.pop("env", dict(os.environ))
try: 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_: if input_:
output = subprocess.run( output = subprocess.run(
command, cmd,
stdout=subprocess.PIPE, stdout=subprocess.PIPE,
stderr=subprocess.STDOUT, stderr=subprocess.STDOUT,
input=encode(input_), input=encode(input_),
...@@ -1550,12 +1525,10 @@ class Env: ...@@ -1550,12 +1525,10 @@ class Env:
**kwargs, **kwargs,
).stdout ).stdout
elif call: elif call:
return subprocess.call( return subprocess.call(cmd, stderr=subprocess.STDOUT, env=env, **kwargs)
command, stderr=subprocess.STDOUT, env=env, **kwargs
)
else: else:
output = subprocess.check_output( output = subprocess.check_output(
command, stderr=subprocess.STDOUT, env=env, **kwargs cmd, stderr=subprocess.STDOUT, env=env, **kwargs
) )
except CalledProcessError as e: except CalledProcessError as e:
raise EnvCommandError(e, input=input_) raise EnvCommandError(e, input=input_)
...@@ -1570,7 +1543,7 @@ class Env: ...@@ -1570,7 +1543,7 @@ class Env:
return os.execvpe(command[0], command, env=env) return os.execvpe(command[0], command, env=env)
kwargs["shell"] = True kwargs["shell"] = True
exe = subprocess.Popen([command[0]] + command[1:], env=env, **kwargs) exe = subprocess.Popen(command, env=env, **kwargs)
exe.communicate() exe.communicate()
return exe.returncode return exe.returncode
...@@ -1909,7 +1882,7 @@ class GenericEnv(VirtualEnv): ...@@ -1909,7 +1882,7 @@ class GenericEnv(VirtualEnv):
if not self._is_windows: if not self._is_windows:
return os.execvpe(command[0], command, env=env) 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() exe.communicate()
return exe.returncode return exe.returncode
......
tests/console/commands/env/helpers.py
...@@ -10,8 +10,6 @@ from poetry.core.constraints.version import Version ...@@ -10,8 +10,6 @@ from poetry.core.constraints.version import Version
if TYPE_CHECKING: if TYPE_CHECKING:
from collections.abc import Callable from collections.abc import Callable
from poetry.core.version.pep440.version import PEP440Version
VERSION_3_7_1 = Version.parse("3.7.1") VERSION_3_7_1 = Version.parse("3.7.1")
...@@ -20,16 +18,19 @@ def build_venv(path: Path | str, **_: Any) -> None: ...@@ -20,16 +18,19 @@ def build_venv(path: Path | str, **_: Any) -> None:
def check_output_wrapper( def check_output_wrapper(
version: PEP440Version = VERSION_3_7_1, version: Version = VERSION_3_7_1,
) -> Callable[[str, Any, Any], str]: ) -> Callable[[list[str], Any, Any], str]:
def check_output(cmd: str, *_: Any, **__: Any) -> str: def check_output(cmd: list[str], *args: Any, **kwargs: Any) -> str:
if "sys.version_info[:3]" in cmd: # cmd is a list, like ["python", "-c", "do stuff"]
python_cmd = cmd[2]
if "sys.version_info[:3]" in python_cmd:
return version.text return version.text
elif "sys.version_info[:2]" in cmd: elif "sys.version_info[:2]" in python_cmd:
return f"{version.major}.{version.minor}" return f"{version.major}.{version.minor}"
elif '-c "import sys; print(sys.executable)"' in cmd: elif "import sys; print(sys.executable)" in python_cmd:
return f"/usr/bin/{cmd.split()[0]}" return f"/usr/bin/{cmd[0]}"
else: else:
assert "import sys; print(sys.prefix)" in python_cmd
return str(Path("/prefix")) return str(Path("/prefix"))
return check_output return check_output
tests/utils/test_env.py
...@@ -94,7 +94,7 @@ def test_virtualenvs_with_spaces_in_their_path_work_as_expected( ...@@ -94,7 +94,7 @@ def test_virtualenvs_with_spaces_in_their_path_work_as_expected(
venv = VirtualEnv(venv_path) 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") @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( ...@@ -124,7 +124,7 @@ def test_env_commands_with_spaces_in_their_arg_work_as_expected(
venv_path = Path(tmp_dir) / "Virtual Env" venv_path = Path(tmp_dir) / "Virtual Env"
manager.build_venv(str(venv_path)) manager.build_venv(str(venv_path))
venv = VirtualEnv(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 " f"pip {venv.pip_version} from "
) )
...@@ -135,9 +135,7 @@ def test_env_shell_commands_with_stdinput_in_their_arg_work_as_expected( ...@@ -135,9 +135,7 @@ def test_env_shell_commands_with_stdinput_in_their_arg_work_as_expected(
venv_path = Path(tmp_dir) / "Virtual Env" venv_path = Path(tmp_dir) / "Virtual Env"
manager.build_venv(str(venv_path)) manager.build_venv(str(venv_path))
venv = VirtualEnv(venv_path) venv = VirtualEnv(venv_path)
run_output_path = Path( run_output_path = Path(venv.run("python", "-", input_=GET_BASE_PREFIX).strip())
venv.run("python", "-", input_=GET_BASE_PREFIX, shell=True).strip()
)
venv_base_prefix_path = Path(str(venv.get_base_prefix())) venv_base_prefix_path = Path(str(venv.get_base_prefix()))
assert run_output_path.resolve() == venv_base_prefix_path.resolve() assert run_output_path.resolve() == venv_base_prefix_path.resolve()
...@@ -189,15 +187,18 @@ VERSION_3_7_1 = Version.parse("3.7.1") ...@@ -189,15 +187,18 @@ VERSION_3_7_1 = Version.parse("3.7.1")
def check_output_wrapper( def check_output_wrapper(
version: Version = VERSION_3_7_1, version: Version = VERSION_3_7_1,
) -> Callable[[str, Any, Any], str]: ) -> Callable[[list[str], Any, Any], str]:
def check_output(cmd: str, *args: Any, **kwargs: Any) -> str: def check_output(cmd: list[str], *args: Any, **kwargs: Any) -> str:
if "sys.version_info[:3]" in cmd: # cmd is a list, like ["python", "-c", "do stuff"]
python_cmd = cmd[2]
if "sys.version_info[:3]" in python_cmd:
return version.text return version.text
elif "sys.version_info[:2]" in cmd: elif "sys.version_info[:2]" in python_cmd:
return f"{version.major}.{version.minor}" return f"{version.major}.{version.minor}"
elif '-c "import sys; print(sys.executable)"' in cmd: elif "import sys; print(sys.executable)" in python_cmd:
return f"/usr/bin/{cmd.split()[0]}" return f"/usr/bin/{cmd[0]}"
else: else:
assert "import sys; print(sys.prefix)" in python_cmd
return str(Path("/prefix")) return str(Path("/prefix"))
return check_output return check_output
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment