From 70da88a89a452ee5cc1c26995f9c9533859e6705 Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sun, 18 Nov 2018 14:36:29 -0700 Subject: [PATCH 1/2] Fix Pester CodeLens run/debug by not quoting params/already quoted args Requires a corresponding PR in vscode-powershell. We should look this one over good as it has potential to break folks. I think the odds are low. Essentially we added the QuoteEscapString method because we get raw paths from VSCode when debugging the current file. Those paths are quoted because they may contain spaces and because we use single quotes, existing single quotes have to be doubled e.g C:\Don'tUseQuotesInPaths to 'C:\Don''tUseQuotesInPaths'. This unfortunately was always being invoked on args. Now we check if the arg is a parameter (starts with '-') or is already quoted. The thinking is if the users is specifying a quoted arg, it's on them to handle any necessary escaping. BTW is there another char that can act as the start of a parameter - like the longer dash - whatever that is called. Longer term, we need a better solution for how to handle paths from VSCode. Maybe we handle them there and eliminate the escaping logic in PSES? --- .../Server/DebugAdapter.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs b/src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs index f35e9998e..0c8dfea03 100644 --- a/src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs +++ b/src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs @@ -305,12 +305,22 @@ protected async Task HandleLaunchRequest( var sb = new StringBuilder(); for (int i = 0; i < launchParams.Args.Length; i++) { - sb.Append(PowerShellContext.QuoteEscapeString(launchParams.Args[i])); - if (i < launchParams.Args.Length - 1) + string arg = launchParams.Args[i]; + if (arg.StartsWith("-") || arg.StartsWith("'") || arg.StartsWith("\"")) + { + sb.Append(arg); + } + else + { + sb.Append(PowerShellContext.QuoteEscapeString(arg)); + } + + if (i < (launchParams.Args.Length - 1)) { sb.Append(' '); } } + arguments = sb.ToString(); Logger.Write(LogLevel.Verbose, "Script arguments are: " + arguments); } From 30b2f718d051394c1a3e3c4f66bd2fe04b4eac7e Mon Sep 17 00:00:00 2001 From: Keith Hill Date: Sun, 18 Nov 2018 21:24:39 -0700 Subject: [PATCH 2/2] Revert to not escape/quoting args or script value when not a file path --- .../Server/DebugAdapter.cs | 21 +------------ .../Session/PowerShellContext.cs | 31 +++++++++++++------ 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs b/src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs index 0c8dfea03..a43f5a7d3 100644 --- a/src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs +++ b/src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs @@ -302,26 +302,7 @@ protected async Task HandleLaunchRequest( string arguments = null; if ((launchParams.Args != null) && (launchParams.Args.Length > 0)) { - var sb = new StringBuilder(); - for (int i = 0; i < launchParams.Args.Length; i++) - { - string arg = launchParams.Args[i]; - if (arg.StartsWith("-") || arg.StartsWith("'") || arg.StartsWith("\"")) - { - sb.Append(arg); - } - else - { - sb.Append(PowerShellContext.QuoteEscapeString(arg)); - } - - if (i < (launchParams.Args.Length - 1)) - { - sb.Append(' '); - } - } - - arguments = sb.ToString(); + arguments = string.Join(" ", launchParams.Args); Logger.Write(LogLevel.Verbose, "Script arguments are: " + arguments); } diff --git a/src/PowerShellEditorServices/Session/PowerShellContext.cs b/src/PowerShellEditorServices/Session/PowerShellContext.cs index 772892bb5..cf1206b0f 100644 --- a/src/PowerShellEditorServices/Session/PowerShellContext.cs +++ b/src/PowerShellEditorServices/Session/PowerShellContext.cs @@ -769,7 +769,6 @@ await this.ExecuteCommand( /// A Task that can be awaited for completion. public async Task ExecuteScriptWithArgs(string script, string arguments = null, bool writeInputToHost = false) { - var escapedScriptPath = new StringBuilder(PowerShellContext.WildcardEscapePath(script)); PSCommand command = new PSCommand(); if (arguments != null) @@ -791,24 +790,38 @@ public async Task ExecuteScriptWithArgs(string script, string arguments = null, "Could not determine current filesystem location:\r\n\r\n" + e.ToString()); } - // If we don't escape wildcard characters in a path to a script file, the script can - // fail to execute if say the script filename was foo][.ps1. + var strBld = new StringBuilder(); + + // The script parameter can refer to either a "script path" or a "command name". If it is a + // script path, we can determine that by seeing if the path exists. If so, we always single + // quote that path in case it includes special PowerShell characters like ', &, (, ), [, ] and + // . Any embedded single quotes are escaped. + // If the provided path is already quoted, then File.Exists will not find it. + // This keeps us from quoting an already quoted path. // Related to issue #123. if (File.Exists(script) || File.Exists(scriptAbsPath)) { - // Dot-source the launched script path - string escapedFilePath = escapedScriptPath.ToString(); - escapedScriptPath = new StringBuilder(". ").Append(QuoteEscapeString(escapedFilePath)); + // Dot-source the launched script path and single quote the path in case it includes + strBld.Append(". ").Append(QuoteEscapeString(script)); + } + else + { + strBld.Append(script); } // Add arguments - escapedScriptPath.Append(' ').Append(arguments); + strBld.Append(' ').Append(arguments); + + var launchedScript = strBld.ToString(); + this.logger.Write(LogLevel.Verbose, $"Launch script is: {launchedScript}"); - command.AddScript(escapedScriptPath.ToString(), false); + command.AddScript(launchedScript, false); } else { - command.AddCommand(escapedScriptPath.ToString(), false); + // AddCommand can handle script paths including those with special chars e.g.: + // ".\foo & [bar]\foo.ps1" and it can handle arbitrary commands, like "Invoke-Pester" + command.AddCommand(script, false); } if (writeInputToHost)