Basic Argument Forwarding and Escaping Code Additions/Changes

src/Microsoft.DotNet.Cli.Utils/ArgumentEscaper.cs

@@ -0,0 +1,216 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Microsoft.DotNet.Cli.Utils
+{
+    public static class ArgumentEscaper
+    {
+        /// <summary>
+        /// Undo the processing which took place to create string[] args in Main,
+        /// so that the next process will receive the same string[] args
+        /// 
+        /// See here for more info:
+        /// http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
+        /// </summary>
+        /// <param name="args"></param>
+        /// <returns></returns>
+        public static string EscapeAndConcatenateArgArray(IEnumerable<string> args, bool cmd=false)
+        { 
+            var sb = new StringBuilder();
+            var first = false;
+
+            foreach (var arg in args)
+            {
+                if (first)
+                {
+                    first = false;
+                }
+                else
+                {
+                    sb.Append(' ');
+                }
+                sb.Append(EscapeArg(arg, cmd));
+            }
+
+            return sb.ToString();
+        }
+
+        public static string EscapeAndConcatenateArgArrayForBash(IEnumerable<string> args)
+        {
+            return EscapeAndConcatenateArgArray(EscapeArgArrayForBash(args));
+        }
+
+        /// <summary>
+        /// Undo the processing which took place to create string[] args in Main,
+        /// so that the next process will receive the same string[] args
+        /// 
+        /// See here for more info:
+        /// http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
+        /// </summary>
+        /// <param name="args"></param>
+        /// <returns></returns>
+        public static string EscapeAndConcatenateArgArrayForCmd(IEnumerable<string> args)
+        {
+            return EscapeAndConcatenateArgArray(EscapeArgArrayForCmd(args), true);
+        }
+
+        /// <summary>
+        /// Undo the processing which took place to create string[] args in Main,
+        /// so that the next process will receive the same string[] args
+        /// 
+        /// See here for more info:
+        /// http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
+        /// </summary>
+        /// <param name="args"></param>
+        /// <returns></returns>
+        public static IEnumerable<string> EscapeArgArray(IEnumerable<string> args)
+        {
+            var escapedArgs = new List<string>();
+
+            foreach (var arg in args)
+            {
+                escapedArgs.Add(EscapeArg(arg));
+            }
+
+            return escapedArgs;
+        }
+
+        public static IEnumerable<string> EscapeArgArrayForBash(IEnumerable<string> arguments)
+        {
+            var escapedArgs = new List<string>();
+
+            foreach (var arg in arguments)
+            {
+                escapedArgs.Add(EscapeArgForBash(arg));
+            }
+
+            return escapedArgs;
+        }
+
+        /// <summary>
+        /// This prefixes every character with the '^' character to force cmd to
+        /// interpret the argument string literally. An alternative option would 
+        /// be to do this only for cmd metacharacters.
+        /// 
+        /// See here for more info:
+        /// http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
+        /// </summary>
+        /// <param name="args"></param>
+        /// <returns></returns>
+        public static IEnumerable<string> EscapeArgArrayForCmd(IEnumerable<string> arguments)
+        {
+            var escapedArgs = new List<string>();
+
+            foreach (var arg in arguments)
+            {
+                escapedArgs.Add(EscapeArgForCmd(arg));
+            }
+
+            return escapedArgs;
+        }
+
+        private static string EscapeArg(string arg, bool cmd=false)
+        {
+            var sb = new StringBuilder();
+
+            // Always quote beginning and end to account for possible spaces
+            if (cmd) sb.Append('^');
+            sb.Append('"');
+
+            if (!cmd)
+            {
+                for (int i = 0; i < arg.Length; ++i)
+                {
+                    var backslashCount = 0;
+
+                    // Consume All Backslashes
+                    while (i < arg.Length && arg[i] == '\\')
+                    {
+                        backslashCount++;
+                        i++;
+                    }
+
+                    // Escape any backslashes at the end of the arg
+                    // This ensures the outside quote is interpreted as
+                    // an argument delimiter
+                    if (i == arg.Length)
+                    {
+                        sb.Append('\\', 2 * backslashCount);
+                    }
+
+                    // Escape any preceding backslashes and the quote
+                    else if (arg[i] == '"')
+                    {
+                        sb.Append('\\', (2 * backslashCount) + 1);
+                        sb.Append('"');
+                    }
+
+                    // Output any consumed backslashes and the character
+                    else
+                    {
+                        sb.Append('\\', backslashCount);
+                        sb.Append(arg[i]);
+                    }
+                }
+            }
+            else
+            {
+                for (int i = 0; i < arg.Length; ++i)
+                {
+                    if (arg[i] == '"')
+                    {
+                        sb.Append('"');
+                        sb.Append('^');
+                        sb.Append(arg[i]);
+                    }
+                    else
+                    {
+                        sb.Append(arg[i]);
+                    }
+                }
+            }
+            
+            if (cmd) sb.Append('^');
+            sb.Append('"');
+
+            return sb.ToString();
+        }
+
+        private static string EscapeArgForBash(string arguments)
+        {
+            throw new NotImplementedException();
+        }
+
+        /// <summary>
+        /// Prepare as single argument to 
+        /// roundtrip properly through cmd.
+        /// 
+        /// This prefixes every character with the '^' character to force cmd to
+        /// interpret the argument string literally. An alternative option would 
+        /// be to do this only for cmd metacharacters.
+        /// 
+        /// See here for more info:
+        /// http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
+        /// </summary>
+        /// <param name="args"></param>
+        /// <returns></returns>
+        private static string EscapeArgForCmd(string arguments)
+        {
+            var sb = new StringBuilder();
+
+            foreach (var character in arguments)
+            {
+                sb.Append('^');
+                sb.Append(character);
+            }
+
+            return sb.ToString();
+        }
+    }
+}

src/Microsoft.DotNet.Cli.Utils/Command.cs

@@ -39,20 +39,25 @@ namespace Microsoft.DotNet.Cli.Utils
             ResolutionStrategy = commandSpec.ResolutionStrategy;
         }
 
-        public static Command Create(string commandName, IEnumerable<string> args, NuGetFramework framework = null)
+        /// <summary>
+        /// Create a command with the specified arg array. Args will be 
+        /// escaped properly to ensure that exactly the strings in this
+        /// array will be present in the corresponding argument array
+        /// in the command's process.
+        /// </summary>
+        /// <param name="commandName"></param>
+        /// <param name="args"></param>
+        /// <param name="framework"></param>
+        /// <returns></returns>
+        public static Command Create(string commandName, IEnumerable<string> args, NuGetFramework framework = null, bool useComSpec = false)
         {
-            return Create(commandName, string.Join(" ", args), framework);
-        }
-
-        public static Command Create(string commandName, string args, NuGetFramework framework = null)
-        {
-            var commandSpec = CommandResolver.TryResolveCommandSpec(commandName, args, framework);
+            var commandSpec = CommandResolver.TryResolveCommandSpec(commandName, args, framework, useComSpec=useComSpec);
 
             if (commandSpec == null)
             {
                 throw new CommandUnknownException(commandName);
             }
-            
+
             var command = new Command(commandSpec);
 
             return command;
@@ -60,6 +65,7 @@ namespace Microsoft.DotNet.Cli.Utils
         
         public CommandResult Execute()
         {
+
             Reporter.Verbose.WriteLine($"Running {_process.StartInfo.FileName} {_process.StartInfo.Arguments}");
 
             ThrowIfRunning();

src/Microsoft.DotNet.Cli.Utils/CommandResolver.cs

@@ -12,45 +12,56 @@ namespace Microsoft.DotNet.Cli.Utils
 {
     internal static class CommandResolver
     {
-        public static CommandSpec TryResolveCommandSpec(string commandName, string args, NuGetFramework framework = null)
+        public static CommandSpec TryResolveCommandSpec(string commandName, IEnumerable<string> args, NuGetFramework framework = null, bool useComSpec = false)
         {
-            return ResolveFromRootedCommand(commandName, args) ??
-                   ResolveFromProjectDependencies(commandName, args, framework) ??
-                   ResolveFromProjectTools(commandName, args) ??
-                   ResolveFromAppBase(commandName, args) ??
-                   ResolveFromPath(commandName, args);
+            return ResolveFromRootedCommand(commandName, args, useComSpec) ??
+                   ResolveFromProjectDependencies(commandName, args, framework, useComSpec) ??
+                   ResolveFromProjectTools(commandName, args, useComSpec) ??
+                   ResolveFromAppBase(commandName, args, useComSpec) ??
+                   ResolveFromPath(commandName, args, useComSpec);
         }
 
-        private static CommandSpec ResolveFromPath(string commandName, string args)
+        private static CommandSpec ResolveFromPath(string commandName, IEnumerable<string> args, bool useComSpec = false)
         {
             var commandPath = Env.GetCommandPath(commandName);
-
+            if (commandPath != null) Console.WriteLine("path?");
             return commandPath == null 
                 ? null 
-                : CreateCommandSpecPreferringExe(commandName, args, commandPath, CommandResolutionStrategy.Path);
+                : CreateCommandSpecPreferringExe(commandName, args, commandPath, CommandResolutionStrategy.Path, useComSpec);
         }
 
-        private static CommandSpec ResolveFromAppBase(string commandName, string args)
+        private static CommandSpec ResolveFromAppBase(string commandName, IEnumerable<string> args, bool useComSpec = false)
         {
             var commandPath = Env.GetCommandPathFromAppBase(AppContext.BaseDirectory, commandName);
-
+            if (commandPath != null) Console.WriteLine("appbase?");
             return commandPath == null 
                 ? null 
-                : CreateCommandSpecPreferringExe(commandName, args, commandPath, CommandResolutionStrategy.BaseDirectory);
+                : CreateCommandSpecPreferringExe(commandName, args, commandPath, CommandResolutionStrategy.BaseDirectory, useComSpec);
         }
 
-        private static CommandSpec ResolveFromRootedCommand(string commandName, string args)
+        private static CommandSpec ResolveFromRootedCommand(string commandName, IEnumerable<string> args, bool useComSpec = false)
         {
             if (Path.IsPathRooted(commandName))
             {
-                return new CommandSpec(commandName, args, CommandResolutionStrategy.Path);
+                Console.WriteLine("rooted?");
+
+                if (useComSpec)
+                {
+                    return CreateComSpecCommandSpec(commandName, args, CommandResolutionStrategy.Path);
+                }
+                else
+                {
+                    var escapedArgs = ArgumentEscaper.EscapeAndConcatenateArgArray(args);
+                    return new CommandSpec(commandName, escapedArgs, CommandResolutionStrategy.Path);
+                }
+                
             }
 
             return null;
         }
 
-        public static CommandSpec ResolveFromProjectDependencies(string commandName, string args,
-            NuGetFramework framework)
+        public static CommandSpec ResolveFromProjectDependencies(string commandName, IEnumerable<string> args,
+            NuGetFramework framework, bool useComSpec = false)
         {
             if (framework == null) return null;
 
@@ -64,7 +75,7 @@ namespace Microsoft.DotNet.Cli.Utils
 
             var depsPath = GetDepsPath(projectContext, Constants.DefaultConfiguration);
 
-            return ConfigureCommandFromPackage(commandName, args, commandPackage, projectContext, depsPath);
+            return ConfigureCommandFromPackage(commandName, args, commandPackage, projectContext, depsPath, useComSpec);
         }
 
         private static ProjectContext GetProjectContext(NuGetFramework framework)
@@ -93,7 +104,7 @@ namespace Microsoft.DotNet.Cli.Utils
                               e == FileNameSuffixes.DotNet.DynamicLib));
         }
 
-        public static CommandSpec ResolveFromProjectTools(string commandName, string args)
+        public static CommandSpec ResolveFromProjectTools(string commandName, IEnumerable<string> args, bool useComSpec = false)
         {
             var context = GetProjectContext(FrameworkConstants.CommonFrameworks.DnxCore50);
 
@@ -129,7 +140,7 @@ namespace Microsoft.DotNet.Cli.Utils
                 : null;
         }
 
-        private static CommandSpec ConfigureCommandFromPackage(string commandName, string args, string packageDir)
+        private static CommandSpec ConfigureCommandFromPackage(string commandName, IEnumerable<string> args, string packageDir)
         {
             var commandPackage = new PackageFolderReader(packageDir);
 
@@ -138,8 +149,8 @@ namespace Microsoft.DotNet.Cli.Utils
             return ConfigureCommandFromPackage(commandName, args, files, packageDir);
         }
 
-        private static CommandSpec ConfigureCommandFromPackage(string commandName, string args,
-            PackageDescription commandPackage, ProjectContext projectContext, string depsPath = null)
+        private static CommandSpec ConfigureCommandFromPackage(string commandName, IEnumerable<string> args,
+            PackageDescription commandPackage, ProjectContext projectContext, string depsPath = null, bool useComSpec = false)
         {
             var files = commandPackage.Library.Files;
 
@@ -149,11 +160,11 @@ namespace Microsoft.DotNet.Cli.Utils
 
             var packageDir = Path.Combine(packageRoot, packagePath);
 
-            return ConfigureCommandFromPackage(commandName, args, files, packageDir, depsPath);
+            return ConfigureCommandFromPackage(commandName, args, files, packageDir, depsPath, useComSpec);
         }
 
-        private static CommandSpec ConfigureCommandFromPackage(string commandName, string args,
-            IEnumerable<string> files, string packageDir, string depsPath = null)
+        private static CommandSpec ConfigureCommandFromPackage(string commandName, IEnumerable<string> args,
+            IEnumerable<string> files, string packageDir, string depsPath = null, bool useComSpec = false)
         {
             var fileName = string.Empty;
 
@@ -169,21 +180,35 @@ namespace Microsoft.DotNet.Cli.Utils
 
                 fileName = CoreHost.HostExePath;
 
-                var depsArg = string.Empty;
+                var additionalArgs = new List<string>();
+                additionalArgs.Add(dllPath);
 
                 if (depsPath != null)
                 {
-                    depsArg = $"\"--depsfile:{depsPath}\" ";
+                    additionalArgs.Add("--depsfile");
+                    additionalArgs.Add(depsPath);
                 }
 
-                args = $"\"{dllPath}\" {depsArg}{args}";
+                args = additionalArgs.Concat(args);
             }
             else
             {
                 fileName = Path.Combine(packageDir, commandPath);
             }
 
-            return new CommandSpec(fileName, args, CommandResolutionStrategy.NugetPackage);
+            
+
+            if (useComSpec)
+            {
+                return CreateComSpecCommandSpec(fileName, args, CommandResolutionStrategy.NugetPackage);
+            }
+            else
+            {
+                var escapedArgs = ArgumentEscaper.EscapeAndConcatenateArgArray(args);
+                return new CommandSpec(fileName, escapedArgs, CommandResolutionStrategy.NugetPackage);
+            }
+
+            
         }
 
         private static string GetDepsPath(ProjectContext context, string buildConfiguration)
@@ -192,23 +217,59 @@ namespace Microsoft.DotNet.Cli.Utils
                 context.ProjectFile.Name + FileNameSuffixes.Deps);
         }
 
-        private static CommandSpec CreateCommandSpecPreferringExe(string commandName, string args, string commandPath,
-            CommandResolutionStrategy resolutionStrategy)
+        private static CommandSpec CreateCommandSpecPreferringExe(
+            string commandName, 
+            IEnumerable<string> args, 
+            string commandPath,
+            CommandResolutionStrategy resolutionStrategy, 
+            bool useComSpec = false)
         {
             if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) &&
                 Path.GetExtension(commandPath).Equals(".cmd", StringComparison.OrdinalIgnoreCase))
             {
                 var preferredCommandPath = Env.GetCommandPath(commandName, ".exe");
 
-                if (preferredCommandPath != null)
+                // Use cmd if we can't find an exe
+                if (preferredCommandPath == null)
                 {
-                    commandPath = Environment.GetEnvironmentVariable("ComSpec");
-
-                    args = $"/S /C \"\"{preferredCommandPath}\" {args}\"";
+                    useComSpec = true;   
+                }
+                else
+                {
+                    commandPath = preferredCommandPath;
                 }
             }
 
-            return new CommandSpec(commandPath, args, resolutionStrategy);
+            if (useComSpec)
+            {
+                return CreateComSpecCommandSpec(commandPath, args, resolutionStrategy);
+            }
+            else
+            {
+                var escapedArgs = ArgumentEscaper.EscapeAndConcatenateArgArray(args);
+                return new CommandSpec(commandPath, escapedArgs, resolutionStrategy);
+            }
+        }
+
+        private static CommandSpec CreateComSpecCommandSpec(
+            string command, 
+            IEnumerable<string> args, 
+            CommandResolutionStrategy resolutionStrategy)
+        {
+
+            // To prevent Command Not Found, comspec gets passed in as
+            // the command already in some cases
+            var comSpec = Environment.GetEnvironmentVariable("ComSpec");
+            if (command.Equals(comSpec, StringComparison.OrdinalIgnoreCase))
+            {
+                command = args.FirstOrDefault();
+                args = args.Skip(1);
+            }
+            var cmdEscapedArgs = ArgumentEscaper.EscapeAndConcatenateArgArrayForCmd(args);
+            var escapedArgString = $"/s /c \"\"{command}\" {cmdEscapedArgs}\"";
+
+            return new CommandSpec(comSpec, escapedArgString, resolutionStrategy);
         }
     }
 }
+

src/Microsoft.DotNet.Cli.Utils/ScriptExecutor.cs

@@ -22,7 +22,7 @@ namespace Microsoft.DotNet.Cli.Utils
             var scriptArguments = CommandGrammar.Process(
                 scriptCommandLine,
                 GetScriptVariable(project, getVariable),
-                preserveSurroundingQuotes: true);
+                preserveSurroundingQuotes: false);
 
             // Ensure the array won't be empty and the elements won't be null or empty strings.
             scriptArguments = scriptArguments.Where(argument => !string.IsNullOrEmpty(argument)).ToArray();
@@ -31,6 +31,8 @@ namespace Microsoft.DotNet.Cli.Utils
                 return null;
             }
 
+            var useComSpec = false;
+
             if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
             {
                 // Only forward slashes are used in script blocks. Replace with backslashes to correctly
@@ -45,24 +47,24 @@ namespace Microsoft.DotNet.Cli.Utils
                 var comSpec = Environment.GetEnvironmentVariable("ComSpec");
                 if (!string.IsNullOrEmpty(comSpec))
                 {
-                    scriptArguments =
-                        new[] { comSpec, "/S", "/C", "\"" }
-                        .Concat(scriptArguments)
-                        .Concat(new[] { "\"" })
-                        .ToArray();
+                    useComSpec=true;
+
+                    List<string> concatenatedArgs = new List<string>();
+                    concatenatedArgs.Add(comSpec);
+                    concatenatedArgs.AddRange(scriptArguments);
+
+                    scriptArguments = concatenatedArgs.ToArray();
                 }
             }
             else
             {
                 // Special-case a script name that, perhaps with added .sh, matches an existing file.
-                var surroundWithQuotes = false;
                 var scriptCandidate = scriptArguments[0];
                 if (scriptCandidate.StartsWith("\"", StringComparison.Ordinal) &&
                     scriptCandidate.EndsWith("\"", StringComparison.Ordinal))
                 {
                     // Strip surrounding quotes; they were required in project.json to keep the script name
                     // together but confuse File.Exists() e.g. "My Script", lacking ./ prefix and .sh suffix.
-                    surroundWithQuotes = true;
                     scriptCandidate = scriptCandidate.Substring(1, scriptCandidate.Length - 2);
                 }
 
@@ -76,19 +78,25 @@ namespace Microsoft.DotNet.Cli.Utils
                     // scriptCandidate may be a path relative to the project root. If so, likely will not be found
                     // in the $PATH; add ./ to let bash know where to look.
                     var prefix = Path.IsPathRooted(scriptCandidate) ? string.Empty : "./";
-                    var quote = surroundWithQuotes ? "\"" : string.Empty;
-                    scriptArguments[0] = $"{ quote }{ prefix }{ scriptCandidate }{ quote }";
+                    scriptArguments[0] = $"{prefix}{scriptCandidate}";
                 }
 
                 // Always use /usr/bin/env bash -c in order to support redirection and so on; similar to Windows case.
                 // Unlike Windows, must escape quotation marks within the newly-quoted string.
-                scriptArguments = new[] { "/usr/bin/env", "bash", "-c", "\"" }
-                    .Concat(scriptArguments.Select(argument => argument.Replace("\"", "\\\"")))
-                    .Concat(new[] { "\"" })
-                    .ToArray();
+                
+                // TODO change this back to original, not doing anything special for bash
+                var bashArgs = ArgumentEscaper.EscapeArgArrayForBash(scriptArguments);
+
+                List<string> concatenatedArgs = new List<string>();
+                concatenatedArgs.Add("/usr/bin/env");
+                concatenatedArgs.Add("bash");
+                concatenatedArgs.Add("-c");
+                concatenatedArgs.AddRange(bashArgs);
+                    
+                scriptArguments = concatenatedArgs.ToArray();
             }
 
-            return Command.Create(scriptArguments.FirstOrDefault(), string.Join(" ", scriptArguments.Skip(1)))
+            return Command.Create(scriptArguments.FirstOrDefault(), scriptArguments.Skip(1), useComSpec: useComSpec)
                 .WorkingDirectory(project.ProjectDirectory);
         }
         private static Func<string, string> WrapVariableDictionary(IDictionary<string, string> contextVariables)