Fix #876, Correct handling of quotation marks in commands

- don't strip quotes surrounding command-line arguments when used in `scripts`
 - prevented command paths and grouping arguments containing spaces
 - pass `preserveSurroundingQuotes` into `CommandGrammar` to control stripping
- support expected shell capabilities such as quoting command and redirections
 - use `cmd /s /c` when executing `scripts` on Windows
 - use `/bin/bash -c` when executing `scripts` on Linux
- add functional tests of commands and scripts (together with `dnu restore`)
 - no `dnu restore` for .NET Core due to extra time required

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nits:
- let VS do its thing with test services
- make `CommandGrammar` constructor private; used only in `Process()` method
- update a few `ScriptExecutor` comments, long lines, et cetera

src/Microsoft.Framework.ApplicationHost/Impl/Syntax/CommandGrammar.cs

@@ -9,12 +9,12 @@ namespace Microsoft.Framework.ApplicationHost.Impl.Syntax
 {
     internal class CommandGrammar : Grammar
     {
-        public CommandGrammar(Func<string, string> variable)
+        private CommandGrammar(Func<string, string> variable, bool preserveSurroundingQuotes)
         {
             var environmentVariablePiece = Ch('%').And(Rep(Ch().Not(Ch('%')))).And(Ch('%')).Left().Down().Str()
                 .Build(key => variable(key) ?? "%" + key + "%");
 
-            var escapeSequencePiece = 
+            var escapeSequencePiece =
                 Ch('%').And(Ch('%')).Build(_=>"%")
                     .Or(Ch('^').And(Ch('^')).Build(_ => "^"))
                     .Or(Ch('\\').And(Ch('\\')).Build(_ => "\\"))
@@ -30,6 +30,12 @@ public CommandGrammar(Func<string, string> variable)
             var unquotedTerm = Rep1(unquotedPiece.Or(specialPiece)).Str();
 
             var quotedTerm = Ch('\"').And(Rep(quotedPiece.Or(specialPiece)).Str()).And(Ch('\"')).Left().Down();
+            if (preserveSurroundingQuotes)
+            {
+                // Str() value assigned to quotedTerm does not include quotation marks surrounding the quoted or
+                // special piece. Add those quotes back if requested.
+                quotedTerm = quotedTerm.Build(str => "\"" + str + "\"");
+            }
 
             var whitespace = Rep(Ch(' '));
 
@@ -40,9 +46,9 @@ public CommandGrammar(Func<string, string> variable)
 
         public readonly Parser<IList<string>> Parse;
 
-        public static string[] Process(string text, Func<string, string> variables)
+        public static string[] Process(string text, Func<string, string> variables, bool preserveSurroundingQuotes)
         {
-            var grammer = new CommandGrammar(variables);
+            var grammer = new CommandGrammar(variables, preserveSurroundingQuotes);
             var cursor = new Cursor(text, 0, text.Length);
 
             var result = grammer.Parse(cursor);

src/Microsoft.Framework.ApplicationHost/Program.cs

@@ -50,9 +50,11 @@ public Task<int> Main(string[] args)
             string replacementCommand;
             if (host.Project.Commands.TryGetValue(lookupCommand, out replacementCommand))
             {
-                var replacementArgs = CommandGrammar.Process(
-                    replacementCommand,
-                    GetVariable).ToArray();
+                // preserveSurroundingQuotes: false to imitate a shell. Shells remove quotation marks before calling
+                // Main methods. Here however we are invoking Main() without involving a shell.
+                var replacementArgs = CommandGrammar
+                    .Process(replacementCommand, GetVariable, preserveSurroundingQuotes: false)
+                    .ToArray();
                 options.ApplicationName = replacementArgs.First();
                 programArgs = replacementArgs.Skip(1).Concat(programArgs).ToArray();
             }

src/Microsoft.Framework.PackageManager/Scripts/ScriptExecutor.cs

@@ -34,51 +34,81 @@ public bool Execute(Runtime.Project project, string scriptName, Func<string, str
 
             foreach (var scriptCommandLine in scriptCommandLines)
             {
+                // Preserve quotation marks around arguments since command is about to be passed to a shell. May need
+                // the quotes to ensure the shell groups arguments correctly.
                 var scriptArguments = CommandGrammar.Process(
                     scriptCommandLine,
-                    GetScriptVariable(project, getVariable));
+                    GetScriptVariable(project, getVariable),
+                    preserveSurroundingQuotes: true);
 
-                // Ensure the array won't be empty and the first element won't be null or empty string.
+                // 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();
-
                 if (scriptArguments.Length == 0)
                 {
                     continue;
                 }
 
                 if (!PlatformHelper.IsMono)
                 {
-                    // Forward-slash is used in script blocked only. Replace them with back-slash to correctly
-                    // locate the script. The directory separator is platform-specific. 
-                    scriptArguments[0] = scriptArguments[0].Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar);
-
-                    // Command-lines on Windows are executed via "cmd /C" in order
-                    // to support batch files, &&, built-in commands like echo, etc.
+                    // Only forward slashes are used in script blocks. Replace with backslashes to correctly
+                    // locate the script. The directory separator is platform-specific.
+                    scriptArguments[0] = scriptArguments[0].Replace(
+                        Path.AltDirectorySeparatorChar,
+                        Path.DirectorySeparatorChar);
+
+                    // Command-lines on Windows are executed via "cmd /S /C" in order to support batch files, &&,
+                    // built-in commands like echo, et cetera. /S allows quoting the command as well as the arguments.
                     // ComSpec is Windows-specific, and contains the full path to cmd.exe
                     var comSpec = Environment.GetEnvironmentVariable("ComSpec");
                     if (!string.IsNullOrEmpty(comSpec))
                     {
                         scriptArguments =
-                            new[] { comSpec, "/C", "\"" }
+                            new[] { comSpec, "/S", "/C", "\"" }
                             .Concat(scriptArguments)
                             .Concat(new[] { "\"" })
                             .ToArray();
                     }
                 }
                 else
                 {
-                    var scriptCandiate = scriptArguments[0] + ".sh";
-                    if (File.Exists(scriptCandiate))
+                    // 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);
+                    }
+
+                    if (!scriptCandidate.EndsWith(".sh", StringComparison.Ordinal))
                     {
-                        scriptArguments[0] = scriptCandiate;
-                        scriptArguments = new[] { "/bin/bash" }.Concat(scriptArguments).ToArray();
+                        scriptCandidate = scriptCandidate + ".sh";
                     }
+
+                    if (File.Exists(Path.Combine(project.ProjectDirectory, scriptCandidate)))
+                    {
+                        // 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 }";
+                    }
+
+                    // Always use /bin/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[] { "/bin/bash", "-c", "\"" }
+                        .Concat(scriptArguments.Select(argument => argument.Replace("\"", "\\\"")))
+                        .Concat(new[] { "\"" })
+                        .ToArray();
                 }
 
                 var startInfo = new ProcessStartInfo
                 {
                     FileName = scriptArguments.FirstOrDefault(),
-                    Arguments = String.Join(" ", scriptArguments.Skip(1)),
+                    Arguments = string.Join(" ", scriptArguments.Skip(1)),
                     WorkingDirectory = project.ProjectDirectory,
 #if DNX451
                     UseShellExecute = false

test/Microsoft.Framework.ApplicationHost.FunctionalTests/AppHostTests.cs

@@ -1,12 +1,12 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Runtime.Versioning;
 using Bootstrapper.FunctionalTests;
 using Microsoft.Framework.CommonTestUtils;
+using Microsoft.Framework.PackageManager;
 using Microsoft.Framework.Runtime;
 using Microsoft.Framework.Runtime.Common.Impl;
 using NuGet;
@@ -170,5 +170,136 @@ public void AppHostShowsErrorWhenCurrentTargetFrameworkWasNotFoundInProjectJson(
                 Assert.Contains(expectedErrorMsg, stdErr);
             }
         }
+
+        public static IEnumerable<object[]> ClrRuntimeComponentsAndCommandsSet
+        {
+            get
+            {
+                // command name -> expected output
+                var commands = new Dictionary<string, string>
+                {
+                    {
+                        "one",
+@"0: 'one'
+1: 'two'
+2: 'extra'
+"
+                    },
+                    {
+                        "two",
+@"0: '^>three'
+1: '&&>>^""'
+2: 'extra'
+"
+                    },
+                    {
+                        "three",
+@"0: 'four'
+1: 'argument five'
+2: 'extra'
+"
+                    },
+                    {
+                        "run",
+@"0: 'extra'
+"
+                    },
+                };
+
+                var data = new TheoryData<object, object, object, string, string>();
+                foreach (var component in TestUtils.GetClrRuntimeComponents())
+                {
+                    foreach (var command in commands)
+                    {
+                        data.Add(component[0], component[1], component[2], command.Key, command.Value);
+                    }
+                }
+
+                return data;
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(ClrRuntimeComponentsAndCommandsSet))]
+        public void AppHost_ExecutesCommands(
+            string flavor,
+            string os,
+            string architecture,
+            string command,
+            string expectedOutput)
+        {
+            var environment = new Dictionary<string, string>
+            {
+                { "DNX_TRACE", "0" },
+            };
+
+            var projectName = "Project Name";
+            var projectStructure =
+$@"{{
+  '.': ['Program.cs', '{ Project.ProjectFileName }']
+}}";
+            var programContents =
+@"using System;
+
+namespace Project_Name
+{
+    public class Program
+    {
+        public void Main(string[] arguments)
+        {
+            for (var i = 0; i < arguments.Length; i++)
+            {
+                var argument = arguments[i];
+                if (!string.IsNullOrWhiteSpace(argument))
+                {
+                    Console.WriteLine($""{ i }: '{ argument }'"");
+                }
+            }
+        }
+    }
+}";
+            var projectJsonContents =
+$@"{{
+  ""commands"": {{
+    ""one"": ""\""{ projectName }\"" one two"",
+    ""two"": ""\""{ projectName }\"" ^>three &&>>^\"""",
+    ""three"": ""\""{ projectName }\"" four \""argument five\""""
+  }},
+  ""frameworks"" : {{
+    ""dnx451"": {{ }}
+  }}
+}}";
+
+            using (var applicationRoot = TestUtils.CreateTempDir())
+            {
+                var projectPath = Path.Combine(applicationRoot, projectName);
+                DirTree.CreateFromJson(projectStructure)
+                    .WithFileContents("Program.cs", programContents)
+                    .WithFileContents(Project.ProjectFileName, projectJsonContents)
+                    .WriteTo(projectPath);
+                var runtimeHomePath = _fixture.GetRuntimeHomeDir(flavor, os, architecture);
+
+                var exitCode = DnuTestUtils.ExecDnu(
+                    runtimeHomePath,
+                    subcommand: "restore",
+                    arguments: null,
+                    environment: environment,
+                    workingDir: projectPath);
+                Assert.Equal(0, exitCode); // Guard
+
+                string output;
+                string error;
+                exitCode = BootstrapperTestUtils.ExecBootstrapper(
+                    runtimeHomePath,
+                    arguments: $@"""{ projectPath }"" { command } extra",
+                    stdOut: out output,
+                    stdErr: out error,
+                    environment: environment);
+
+                Assert.Equal(0, exitCode);
+                Assert.Empty(error);
+                Assert.Equal(expectedOutput, output);
+            }
+        }
     }
 }