Fix PowerShell wildcard escaping in debug paths (#765)

* Fix PowerShell wildcard escaping

* Add quoting for debug arguments

* Add obsolete attributes to APIs that should not be public

* Add escaping tests

* Add dot source tests for strange paths

Author: rjmholt

src/PowerShellEditorServices.Protocol/Server/DebugAdapter.cs

@@ -259,7 +259,6 @@ protected async Task HandleLaunchRequest(
                 // the path exists and is a directory.
                 if (!string.IsNullOrEmpty(workingDir))
                 {
-                    workingDir = PowerShellContext.UnescapePath(workingDir);
                     try
                     {
                         if ((File.GetAttributes(workingDir) & FileAttributes.Directory) != FileAttributes.Directory)
@@ -303,7 +302,16 @@ protected async Task HandleLaunchRequest(
             string arguments = null;
             if ((launchParams.Args != null) && (launchParams.Args.Length > 0))
             {
-                arguments = string.Join(" ", launchParams.Args);
+                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)
+                    {
+                        sb.Append(' ');
+                    }
+                }
+                arguments = sb.ToString();
                 Logger.Write(LogLevel.Verbose, "Script arguments are: " + arguments);
             }
 

src/PowerShellEditorServices/Debugging/DebugService.cs

@@ -178,7 +178,7 @@ public async Task<BreakpointDetails[]> SetLineBreakpoints(
             // Fix for issue #123 - file paths that contain wildcard chars [ and ] need to
             // quoted and have those wildcard chars escaped.
             string escapedScriptPath =
-                PowerShellContext.EscapePath(scriptPath, escapeSpaces: false);
+                PowerShellContext.WildcardEscapePath(scriptPath);
 
             if (dscBreakpoints == null || !dscBreakpoints.IsDscResourcePath(escapedScriptPath))
             {

src/PowerShellEditorServices/Properties/AssemblyInfo.cs

@@ -5,6 +5,7 @@
 
 using System.Runtime.CompilerServices;
 
+[assembly: InternalsVisibleTo("Microsoft.PowerShell.EditorServices.Protocol")]
 [assembly: InternalsVisibleTo("Microsoft.PowerShell.EditorServices.Test")]
 [assembly: InternalsVisibleTo("Microsoft.PowerShell.EditorServices.Test.Shared")]
 

src/PowerShellEditorServices/Session/PowerShellContext.cs

@@ -23,6 +23,7 @@ namespace Microsoft.PowerShell.EditorServices
     using System.Management.Automation.Runspaces;
     using Microsoft.PowerShell.EditorServices.Session.Capabilities;
     using System.IO;
+    using System.ComponentModel;
 
     /// <summary>
     /// Manages the lifetime and usage of a PowerShell session.
@@ -768,7 +769,7 @@ await this.ExecuteCommand<object>(
         /// <returns>A Task that can be awaited for completion.</returns>
         public async Task ExecuteScriptWithArgs(string script, string arguments = null, bool writeInputToHost = false)
         {
-            string launchedScript = script;
+            var escapedScriptPath = new StringBuilder(PowerShellContext.WildcardEscapePath(script));
             PSCommand command = new PSCommand();
 
             if (arguments != null)
@@ -796,21 +797,24 @@ public async Task ExecuteScriptWithArgs(string script, string arguments = null,
                 if (File.Exists(script) || File.Exists(scriptAbsPath))
                 {
                     // Dot-source the launched script path
-                    script = ". " + EscapePath(script, escapeSpaces: true);
+                    string escapedFilePath = escapedScriptPath.ToString();
+                    escapedScriptPath = new StringBuilder(". ").Append(QuoteEscapeString(escapedFilePath));
                 }
 
-                launchedScript = script + " " + arguments;
-                command.AddScript(launchedScript, false);
+                // Add arguments
+                escapedScriptPath.Append(' ').Append(arguments);
+
+                command.AddScript(escapedScriptPath.ToString(), false);
             }
             else
             {
-                command.AddCommand(script, false);
+                command.AddCommand(escapedScriptPath.ToString(), false);
             }
 
             if (writeInputToHost)
             {
                 this.WriteOutput(
-                    launchedScript + Environment.NewLine,
+                    script + Environment.NewLine,
                     true);
             }
 
@@ -1113,30 +1117,145 @@ public async Task SetWorkingDirectory(string path, bool isPathAlreadyEscaped)
             {
                 if (!isPathAlreadyEscaped)
                 {
-                    path = EscapePath(path, false);
+                    path = WildcardEscapePath(path);
                 }
 
                 runspaceHandle.Runspace.SessionStateProxy.Path.SetLocation(path);
             }
         }
 
+        /// <summary>
+        /// Fully escape a given path for use in PowerShell script.
+        /// Note: this will not work with PowerShell.AddParameter()
+        /// </summary>
+        /// <param name="path">The path to escape.</param>
+        /// <returns>An escaped version of the path that can be embedded in PowerShell script.</returns>
+        internal static string FullyPowerShellEscapePath(string path)
+        {
+            string wildcardEscapedPath = WildcardEscapePath(path);
+            return QuoteEscapeString(wildcardEscapedPath);
+        }
+
+        /// <summary>
+        /// Wrap a string in quotes to make it safe to use in scripts.
+        /// </summary>
+        /// <param name="escapedPath">The glob-escaped path to wrap in quotes.</param>
+        /// <returns>The given path wrapped in quotes appropriately.</returns>
+        internal static string QuoteEscapeString(string escapedPath)
+        {
+            var sb = new StringBuilder(escapedPath.Length + 2); // Length of string plus two quotes
+            sb.Append('\'');
+            if (!escapedPath.Contains('\''))
+            {
+                sb.Append(escapedPath);
+            }
+            else
+            {
+                foreach (char c in escapedPath)
+                {
+                    if (c == '\'')
+                    {
+                        sb.Append("''");
+                        continue;
+                    }
+
+                    sb.Append(c);
+                }
+            }
+            sb.Append('\'');
+            return sb.ToString();
+        }
+
+        /// <summary>
+        /// Return the given path with all PowerShell globbing characters escaped,
+        /// plus optionally the whitespace.
+        /// </summary>
+        /// <param name="path">The path to process.</param>
+        /// <param name="escapeSpaces">Specify True to escape spaces in the path, otherwise False.</param>
+        /// <returns>The path with [ and ] escaped.</returns>
+        internal static string WildcardEscapePath(string path, bool escapeSpaces = false)
+        {
+            var sb = new StringBuilder();
+            for (int i = 0; i < path.Length; i++)
+            {
+                char curr = path[i];
+                switch (curr)
+                {
+                    // Escape '[', ']', '?' and '*' with '`'
+                    case '[':
+                    case ']':
+                    case '*':
+                    case '?':
+                    case '`':
+                        sb.Append('`').Append(curr);
+                        break;
+
+                    default:
+                        // Escape whitespace if required
+                        if (escapeSpaces && char.IsWhiteSpace(curr))
+                        {
+                            sb.Append('`').Append(curr);
+                            break;
+                        }
+                        sb.Append(curr);
+                        break;
+                }
+            }
+
+            return sb.ToString();
+        }
+
         /// <summary>
         /// Returns the passed in path with the [ and ] characters escaped. Escaping spaces is optional.
         /// </summary>
         /// <param name="path">The path to process.</param>
         /// <param name="escapeSpaces">Specify True to escape spaces in the path, otherwise False.</param>
         /// <returns>The path with [ and ] escaped.</returns>
+        [EditorBrowsable(EditorBrowsableState.Never)]
+        [Obsolete("This API is not meant for public usage and should not be used.")]
         public static string EscapePath(string path, bool escapeSpaces)
         {
-            string escapedPath = Regex.Replace(path, @"(?<!`)\[", "`[");
-            escapedPath = Regex.Replace(escapedPath, @"(?<!`)\]", "`]");
+            return WildcardEscapePath(path, escapeSpaces);
+        }
 
-            if (escapeSpaces)
+        internal static string UnescapeWildcardEscapedPath(string wildcardEscapedPath)
+        {
+            // Prevent relying on my implementation if we can help it
+            if (!wildcardEscapedPath.Contains('`'))
             {
-                escapedPath = Regex.Replace(escapedPath, @"(?<!`) ", "` ");
+                return wildcardEscapedPath;
             }
 
-            return escapedPath;
+            var sb = new StringBuilder(wildcardEscapedPath.Length);
+            for (int i = 0; i < wildcardEscapedPath.Length; i++)
+            {
+                // If we see a backtick perform a lookahead
+                char curr = wildcardEscapedPath[i];
+                if (curr == '`' && i + 1 < wildcardEscapedPath.Length)
+                {
+                    // If the next char is an escapable one, don't add this backtick to the new string
+                    char next = wildcardEscapedPath[i + 1];
+                    switch (next)
+                    {
+                        case '[':
+                        case ']':
+                        case '?':
+                        case '*':
+                            continue;
+
+                        default:
+                            if (char.IsWhiteSpace(next))
+                            {
+                                continue;
+                            }
+                            break;
+                    }
+                }
+
+                sb.Append(curr);
+            }
+
+            return sb.ToString();
         }
 
         /// <summary>
@@ -1145,14 +1264,11 @@ public static string EscapePath(string path, bool escapeSpaces)
         /// </summary>
         /// <param name="path">The path to unescape.</param>
         /// <returns>The path with the ` character before [, ] and spaces removed.</returns>
+        [EditorBrowsable(EditorBrowsableState.Never)]
+        [Obsolete("This API is not meant for public usage and should not be used.")]
         public static string UnescapePath(string path)
         {
-            if (!path.Contains("`"))
-            {
-                return path;
-            }
-
-            return Regex.Replace(path, @"`(?=[ \[\]])", "");
+            return UnescapeWildcardEscapedPath(path);
         }
 
         #endregion

src/PowerShellEditorServices/Workspace/Workspace.cs

@@ -373,7 +373,7 @@ internal string ResolveFilePath(string filePath)
                 // Clients could specify paths with escaped space, [ and ] characters which .NET APIs
                 // will not handle.  These paths will get appropriately escaped just before being passed
                 // into the PowerShell engine.
-                filePath = PowerShellContext.UnescapePath(filePath);
+                filePath = PowerShellContext.UnescapeWildcardEscapedPath(filePath);
 
                 // Get the absolute file path
                 filePath = Path.GetFullPath(filePath);

test/PowerShellEditorServices.Test.Shared/Debugging/Debug With Params [Test].ps1 renamed to test/PowerShellEditorServices.Test.Shared/Debugging/Debug W&ith Params [Test].ps1

@@ -0,0 +1,4 @@
+function Hello
+{
+    "Bye"
+}

test/PowerShellEditorServices.Test.Shared/scriptassets/Bad&name4script.ps1

@@ -0,0 +1 @@
+Write-Output "Windows won't let me put * or ? in the name of this file..."

test/PowerShellEditorServices.Test.Shared/scriptassets/NormalScript.ps1

@@ -105,7 +105,7 @@ public async Task DebuggerAcceptsScriptArgs(string[] args)
             // it should not escape already escaped chars.
             ScriptFile debugWithParamsFile =
                 this.workspace.GetFile(
-                    @"..\..\..\..\PowerShellEditorServices.Test.Shared\Debugging\Debug` With Params `[Test].ps1");
+                    @"..\..\..\..\PowerShellEditorServices.Test.Shared\Debugging\Debug` W&ith Params `[Test].ps1");
 
             await this.debugService.SetLineBreakpoints(
                 debugWithParamsFile,