From 716ebb0bf0538b83b1dfc2de157263195c8ee9d7 Mon Sep 17 00:00:00 2001 From: Pavel Minaev Date: Wed, 3 Aug 2022 17:29:19 -0700 Subject: [PATCH] Fix #1001: Enable controlling shell expansion via "argsCanBeInterpretedByShell" Fix #357: "argsExpansion" does not do what it says in VSCode Treat non-array "args" in debug config as a request to prevent shell argument escaping and allow shell expansion. Remove "argsExpansion". --- src/debugpy/adapter/clients.py | 40 +++++++++++++++++++++----------- src/debugpy/adapter/launchers.py | 15 ++++-------- src/debugpy/launcher/handlers.py | 8 +++---- tests/debug/config.py | 1 - tests/debug/runners.py | 7 ++++-- tests/debug/session.py | 7 +++++- tests/debug/targets.py | 18 ++++++++++---- tests/debugpy/test_args.py | 18 +++++++------- 8 files changed, 68 insertions(+), 46 deletions(-) diff --git a/src/debugpy/adapter/clients.py b/src/debugpy/adapter/clients.py index 20a450a72..bc4bc50ab 100644 --- a/src/debugpy/adapter/clients.py +++ b/src/debugpy/adapter/clients.py @@ -29,6 +29,7 @@ class Capabilities(components.Capabilities): "supportsVariablePaging": False, "supportsRunInTerminalRequest": False, "supportsMemoryReferences": False, + "supportsArgsCanBeInterpretedByShell": False, } class Expectations(components.Capabilities): @@ -364,20 +365,6 @@ def property_or_debug_option(prop_name, flag_name): '"program", "module", and "code" are mutually exclusive' ) - # Propagate "args" via CLI if and only if shell expansion is requested. - args_expansion = request( - "argsExpansion", json.enum("shell", "none", optional=True) - ) - if args_expansion == "shell": - args += request("args", json.array(str)) - request.arguments.pop("args", None) - - cwd = request("cwd", str, optional=True) - if cwd == (): - # If it's not specified, but we're launching a file rather than a module, - # and the specified path has a directory in it, use that. - cwd = None if program == () else (os.path.dirname(program) or None) - console = request( "console", json.enum( @@ -389,6 +376,30 @@ def property_or_debug_option(prop_name, flag_name): ) console_title = request("consoleTitle", json.default("Python Debug Console")) + # Propagate "args" via CLI so that shell expansion can be applied if requested. + target_args = request("args", json.array(str, vectorize=True)) + args += target_args + + # If "args" was a single string rather than an array, shell expansion must be applied. + shell_expand_args = len(target_args) > 0 and isinstance( + request.arguments["args"], str + ) + if shell_expand_args: + if not self.capabilities["supportsArgsCanBeInterpretedByShell"]: + raise request.isnt_valid( + 'Shell expansion in "args" is not supported by the client' + ) + if console == "internalConsole": + raise request.isnt_valid( + 'Shell expansion in "args" is not available for "console":"internalConsole"' + ) + + cwd = request("cwd", str, optional=True) + if cwd == (): + # If it's not specified, but we're launching a file rather than a module, + # and the specified path has a directory in it, use that. + cwd = None if program == () else (os.path.dirname(program) or None) + sudo = bool(property_or_debug_option("sudo", "Sudo")) if sudo and sys.platform == "win32": raise request.cant_handle('"sudo":true is not supported on Windows.') @@ -412,6 +423,7 @@ def property_or_debug_option(prop_name, flag_name): launcher_path, adapter_host, args, + shell_expand_args, cwd, console, console_title, diff --git a/src/debugpy/adapter/launchers.py b/src/debugpy/adapter/launchers.py index cb8fc65d5..444e54dd9 100644 --- a/src/debugpy/adapter/launchers.py +++ b/src/debugpy/adapter/launchers.py @@ -7,7 +7,7 @@ import sys from debugpy import adapter, common -from debugpy.common import json, log, messaging, sockets +from debugpy.common import log, messaging, sockets from debugpy.adapter import components, servers @@ -70,6 +70,7 @@ def spawn_debuggee( launcher_path, adapter_host, args, + shell_expand_args, cwd, console, console_title, @@ -119,16 +120,6 @@ def on_launcher_connected(sock): if console == "internalConsole": log.info("{0} spawning launcher: {1!r}", session, cmdline) try: - for i, arg in enumerate(cmdline): - try: - cmdline[i] = arg - except UnicodeEncodeError as exc: - raise start_request.cant_handle( - "Invalid command line argument {0}: {1}", - json.repr(arg), - exc, - ) - # If we are talking to the client over stdio, sys.stdin and sys.stdout # are redirected to avoid mangling the DAP message stream. Make sure # the launcher also respects that. @@ -154,6 +145,8 @@ def on_launcher_connected(sock): } if cwd is not None: request_args["cwd"] = cwd + if shell_expand_args: + request_args["argsCanBeInterpretedByShell"] = True try: # It is unspecified whether this request receives a response immediately, or only # after the spawned command has completed running, so do not block waiting for it. diff --git a/src/debugpy/launcher/handlers.py b/src/debugpy/launcher/handlers.py index 32dcaeebf..213a5b9d8 100644 --- a/src/debugpy/launcher/handlers.py +++ b/src/debugpy/launcher/handlers.py @@ -10,6 +10,7 @@ from debugpy.common import json from debugpy.launcher import debuggee + def launch_request(request): debug_options = set(request("debugOptions", json.array(str))) @@ -67,12 +68,9 @@ def property_or_debug_option(prop_name, flag_name): debugpy_args = request("debugpyArgs", json.array(str)) cmdline += debugpy_args - # Further arguments can come via two channels: the launcher's own command line, or - # "args" in the request; effective arguments are concatenation of these two in order. - # Arguments for debugpy (such as -m) always come via CLI, but those specified by the - # user via "args" are passed differently by the adapter depending on "argsExpansion". + # Use the copy of arguments that was propagated via the command line rather than + # "args" in the request itself, to allow for shell expansion. cmdline += sys.argv[1:] - cmdline += request("args", json.array(str)) process_name = request("processName", sys.executable) diff --git a/tests/debug/config.py b/tests/debug/config.py index 22428f47b..c6ec3035d 100644 --- a/tests/debug/config.py +++ b/tests/debug/config.py @@ -49,7 +49,6 @@ class DebugConfig(MutableMapping): "type": (), # Launch "args": [], - "argsExpansion": "shell", "code": (), "console": "internal", "cwd": (), diff --git a/tests/debug/runners.py b/tests/debug/runners.py index 23a957261..71f985f75 100644 --- a/tests/debug/runners.py +++ b/tests/debug/runners.py @@ -325,9 +325,12 @@ def spawn_debuggee(occ): attach_listen.host = "127.0.0.1" attach_listen.port = net.get_test_server_port(5478, 5600) -all_launch_terminal = [launch["integratedTerminal"], launch["externalTerminal"]] +all_launch_terminal = [ + launch.with_options(console="integratedTerminal"), + launch.with_options(console="externalTerminal"), +] -all_launch = [launch["internalConsole"]] + all_launch_terminal +all_launch = [launch.with_options(console="internalConsole")] + all_launch_terminal all_attach_listen = [attach_listen["api"], attach_listen["cli"]] diff --git a/tests/debug/session.py b/tests/debug/session.py index af242877b..2e58c1a4c 100644 --- a/tests/debug/session.py +++ b/tests/debug/session.py @@ -490,7 +490,11 @@ def run_in_terminal(self, args, cwd, env): def _process_request(self, request): self.timeline.record_request(request, block=False) if request.command == "runInTerminal": - args = request("args", json.array(str)) + args = request("args", json.array(str, vectorize=True)) + if len(args) > 0 and request("argsCanBeInterpretedByShell", False): + # The final arg is a string that contains multiple actual arguments. + last_arg = args.pop() + args += last_arg.split() cwd = request("cwd", ".") env = request("env", json.object(str)) try: @@ -557,6 +561,7 @@ def _start_channel(self, stream): "columnsStartAt1": True, "supportsVariableType": True, "supportsRunInTerminalRequest": True, + "supportsArgsCanBeInterpretedByShell": True, }, ) diff --git a/tests/debug/targets.py b/tests/debug/targets.py index 80299df8c..25b06629d 100644 --- a/tests/debug/targets.py +++ b/tests/debug/targets.py @@ -45,6 +45,14 @@ def cli(self, env): """ raise NotImplementedError + @property + def argslist(self): + args = self.args + if isinstance(args, str): + return [args] + else: + return list(args) + @property def co_filename(self): """co_filename of code objects created at runtime from the source that this @@ -121,9 +129,11 @@ def configure(self, session): def cli(self, env): if self._cwd: - return [self._get_relative_program()] + list(self.args) + cli = [self._get_relative_program()] else: - return [self.filename.strpath] + list(self.args) + cli = [self.filename.strpath] + cli += self.argslist + return cli class Module(Target): @@ -150,7 +160,7 @@ def configure(self, session): def cli(self, env): if self.filename is not None: env.prepend_to("PYTHONPATH", self.filename.dirname) - return ["-m", self.name] + list(self.args) + return ["-m", self.name] + self.argslist class Code(Target): @@ -176,7 +186,7 @@ def configure(self, session): session.config["args"] = self.args def cli(self, env): - return ["-c", self.code] + list(self.args) + return ["-c", self.code] + self.argslist @property def co_filename(self): diff --git a/tests/debugpy/test_args.py b/tests/debugpy/test_args.py index 03341f243..263325528 100644 --- a/tests/debugpy/test_args.py +++ b/tests/debugpy/test_args.py @@ -34,8 +34,11 @@ def code_to_debug(): @pytest.mark.parametrize("target", targets.all) @pytest.mark.parametrize("run", runners.all_launch) -@pytest.mark.parametrize("expansion", ["", "none", "shell"]) +@pytest.mark.parametrize("expansion", ["preserve", "expand"]) def test_shell_expansion(pyfile, target, run, expansion): + if expansion == "expand" and run.console == "internalConsole": + pytest.skip('Shell expansion is not supported for "internalConsole"') + @pyfile def code_to_debug(): import sys @@ -46,6 +49,8 @@ def code_to_debug(): backchannel.send(sys.argv) def expand(args): + if expansion != "expand": + return log.info("Before expansion: {0}", args) for i, arg in enumerate(args): if arg.startswith("$"): @@ -57,17 +62,14 @@ def run_in_terminal(self, args, cwd, env): expand(args) return super().run_in_terminal(args, cwd, env) - args = ["0", "$1", "2"] + argslist = ["0", "$1", "2"] + args = argslist if expansion == "preserve" else " ".join(argslist) with Session() as session: - if expansion: - session.config["argsExpansion"] = expansion - backchannel = session.open_backchannel() with run(session, target(code_to_debug, args=args)): pass argv = backchannel.receive() - if session.config["console"] != "internalConsole" and expansion != "none": - expand(args) - assert argv == [some.str] + args + expand(argslist) + assert argv == [some.str] + argslist