From 15c7308ca9fbd85f231e5c11e950a39fd068b078 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sun, 1 Apr 2018 17:23:50 +0800 Subject: [PATCH] Proper script argument escaping I chose to make Script.parse to always operate in POSIX mode so it is much easier to write commands compatible on all platforms. Script.cmdify is the important part on Windows. It ensures the argument line is always escaped and joined properly on Windows, not just for the subset that works both under POSIX and Windows (as is the case of shlex.escape). --- pipenv/cmdparse.py | 60 ++++++++++++++++++++++++++++++++++++++++++ pipenv/core.py | 36 +++++++------------------ pipenv/project.py | 20 +++++++++++--- tests/test_cmdparse.py | 27 +++++++++++++++++++ tests/test_pipenv.py | 19 ++++++------- 5 files changed, 123 insertions(+), 39 deletions(-) create mode 100644 pipenv/cmdparse.py create mode 100644 tests/test_cmdparse.py diff --git a/pipenv/cmdparse.py b/pipenv/cmdparse.py new file mode 100644 index 00000000..1f6bc9e6 --- /dev/null +++ b/pipenv/cmdparse.py @@ -0,0 +1,60 @@ +import re +import shlex + +import six + + +class Script(object): + """Parse a script line (in Pipfile's [scripts] section). + + This always works in POSIX mode, even on Windows. + """ + def __init__(self, parts): + if not parts: + raise ValueError('invalid script') + self._parts = parts + + + @classmethod + def parse(cls, value): + if isinstance(value, six.text_type): + value = shlex.split(value) + return cls(value) + + def __repr__(self): + return 'Script({0!r})'.format(self._parts) + + @property + def command(self): + return self._parts[0] + + @property + def args(self): + return self._parts[1:] + + def extend(self, extra_args): + self._parts.extend(extra_args) + + def cmdify(self): + """Encode into a cmd-executable string. + + This re-implements CreateProcess's quoting logic to turn a list of + arguments into one single string for the shell to interpret. + + * All double quotes are escaped with a backslash. + * Existing backslashes before a quote are doubled, so they are all + escaped properly. + * Backslashes elsewhere are left as-is; cmd will interpret them + literally. + + The result is then quoted into a pair of double quotes to be grouped. + + The intended use of this function is to pre-process an argument list + before passing it into ``subprocess.Popen(..., shell=True)``. + + See also: https://docs.python.org/3/library/subprocess.html#converting-argument-sequence + """ + return ' '.join( + '"{0}"'.format(re.sub(r'(\\*)"', r'\1\1\\"', arg)) + for arg in self._parts + ) diff --git a/pipenv/core.py b/pipenv/core.py index 1a0deca8..b42892eb 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -2199,46 +2199,28 @@ def inline_activate_virtualenv(): def do_run_nt(command, args): """Run command by appending space-joined args to it!""" import subprocess - try: - from shlex import quote as shellquote - except ImportError: - from pipenv.vendor.backports.shlex import quote as shellquote - command = project.scripts.get(command, command) - command = [command] + [shellquote(a) for a in args] - - # if you've passed something with crazy quoting... - # ...just don't. (or put it in a script!) - p = subprocess.Popen(command, shell=True, universal_newlines=True) + p = subprocess.Popen( + project.build_script(command, args).cmdify(), + shell=True, universal_newlines=True, + ) p.communicate() sys.exit(p.returncode) -def _get_command_posix(project, command, args): - """Fully bake command into executable and args, based upon project""" - # Script was found… - if command in project.scripts: - command = project.scripts[command] - parsed_command = shlex.split(command) - executable = parsed_command[0] - # prepend arguments - args = list(parsed_command[1:]) + list(args) - return executable, args - - def do_run_posix(command, args): """Attempt to run command either pulling from project or interpreting as executable. Args are appended to the command in [scripts] section of project if found. """ - executable, args = _get_command_posix(project, command, args) - command_path = system_which(executable) + script = project.build_script(command, args) + command_path = system_which(script.command) if not command_path: - if command in project.scripts: + if project.has_script(command): click.echo( '{0}: the command {1} (from {2}) could not be found within {3}.' ''.format( crayons.red('Error', bold=True), - crayons.red(executable), + crayons.red(script.command), crayons.normal(command, bold=True), crayons.normal('PATH', bold=True), ), @@ -2256,7 +2238,7 @@ def do_run_posix(command, args): err=True, ) sys.exit(1) - os.execl(command_path, command_path, *args) + os.execl(command_path, command_path, *script.args) def do_run(command, args, three=None, python=False): diff --git a/pipenv/project.py b/pipenv/project.py index 9da86876..7ca2a916 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -16,6 +16,7 @@ import pipfile.api import toml from pip9 import ConfigOptionParser +from .cmdparse import Script from .utils import ( mkdir_p, convert_deps_from_pip, @@ -388,9 +389,22 @@ class Project(object): """A dictionary of the settings added to the Pipfile.""" return self.parsed_pipfile.get('pipenv', {}) - @property - def scripts(self): - return dict(self.parsed_pipfile.get('scripts', {})) + def has_script(self, name): + try: + return name in self.parsed_pipfile['scripts'] + except KeyError: + return False + + def build_script(self, name, extra_args=None): + try: + script = Script.parse(self.parsed_pipfile['scripts'][name]) + except KeyError: + script = Script([name]) + except ValueError: + raise ValueError('invalid script entry {0!r}'.format(name)) + if extra_args: + script.extend(extra_args) + return script def update_settings(self, d): settings = self.settings diff --git a/tests/test_cmdparse.py b/tests/test_cmdparse.py new file mode 100644 index 00000000..776ef61f --- /dev/null +++ b/tests/test_cmdparse.py @@ -0,0 +1,27 @@ +import textwrap + +from pipenv.cmdparse import Script + + +def test_parse(): + script = Script.parse(['python', '-c', "print('hello')"]) + assert script.command == 'python' + assert script.args == ['-c', "print('hello')"], script + + +def test_cmdify(): + script = Script.parse(['python', '-c', "print('hello')"]) + cmd = script.cmdify(['--verbose']) + assert cmd == '"python" "-c" "print(\'hello\')" "--verbose"', script + + +def test_cmdify_complex(): + script = Script.parse(' '.join([ + '"C:\\Program Files\\Python36\\python.exe" -c', + """ "print(\'Double quote: \\\"\')" """.strip(), + ])) + assert script.cmdify([]) == ' '.join([ + '"C:\\Program Files\\Python36\\python.exe"', + '"-c"', + """ "print(\'Double quote: \\\"\')" """.strip(), + ]), script diff --git a/tests/test_pipenv.py b/tests/test_pipenv.py index d9262f9b..0e34bf70 100644 --- a/tests/test_pipenv.py +++ b/tests/test_pipenv.py @@ -5,7 +5,7 @@ import shutil import json import pytest import warnings -from pipenv.core import activate_virtualenv, _get_command_posix +from pipenv.core import activate_virtualenv from pipenv.utils import ( temp_environ, get_windows_path, mkdir_p, normalize_drive, TemporaryDirectory ) @@ -794,9 +794,8 @@ requests = {version = "*"} # If we can do this we can theoretically make a subshell # This test doesn't work on *nix if os.name == 'nt': - args = ['pewtwo', 'in', '.venv', 'pip', 'freeze'] process = subprocess.Popen( - args, + 'pewtwo in .venv pip freeze', shell=True, universal_newlines=True, stdin=subprocess.PIPE, @@ -1261,12 +1260,14 @@ multicommand = "bash -c \"cd docs && make html\"" assert c.out == '' assert 'Error' in c.err assert 'randomthingtotally (from notfoundscript)' in c.err - executable, argv = _get_command_posix(Project(), 'multicommand', []) - assert executable == 'bash' - assert argv == ['-c', 'cd docs && make html'] - executable, argv = _get_command_posix(Project(), 'appendscript', ['a', 'b']) - assert executable == 'cmd' - assert argv == ['arg1', 'a', 'b'] + + project = Project() + script = project.build_script('multicommand') + assert script.command == 'bash' + assert script.args == ['-c', 'cd docs && make html'] + script = project.build_script('appendscript', ['a', 'b']) + assert script.command == 'cmd' + assert script.args == ['arg1', 'a', 'b'] @pytest.mark.lock @pytest.mark.complex