Support array args on windows

Implements correct quoting and escaping of arguments on windows.

That means that this works right now:

```ruby
filename = "c:\program files"
shell_out("dir", filename);
```

So all the defensive coding around quotes-around-filepaths -- which were
all actually buggy even when they worked (trailing backslashes would
fail) -- are unnecessary and arguments can just be passed in as an array
and this code will sort it out.

We rely on the existing determination of if metacharacters mean it needs
to run under cmd and if it needs the ^ quoting which seems to be well
tested, and which all runs after this does.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

Rakefile

@@ -3,7 +3,7 @@ require "rspec/core/rake_task"
 
 Bundler::GemHelper.install_tasks name: "mixlib-shellout"
 
-task default: [:style, :spec]
+task default: [:spec, :style]
 
 desc "Run specs"
 RSpec::Core::RakeTask.new(:spec) do |spec|

lib/mixlib/shellout/windows.rb

@@ -66,7 +66,7 @@ def run_command
           #
           # Set cwd, environment, appname, etc.
           #
-          app_name, command_line = command_to_run(command)
+          app_name, command_line = command_to_run(combine_args(*command))
           create_process_args = {
             app_name: app_name,
             command_line: command_line,
@@ -196,6 +196,33 @@ def consume_output(open_streams, stdout_read, stderr_read)
         true
       end
 
+      # Use to support array passing semantics on windows
+      #
+      # 1.  strings with whitespace or quotes in them need quotes around them.
+      # 2.  interior quotes need to get backslash escaped (parser needs to know when it really ends).
+      # 3.  random backlsashes in paths themselves remain untouched.
+      # 4.  if the argument must be quoted by #1 and terminates in a sequence of backslashes then all the backlashes must themselves
+      #     be backslash excaped (doubling the backslashes)
+      # 5.  if an interior quote that must be escaped by #2 has a sequence of backslashes before it then all the backslashes must
+      #     themself be backslash excaped along with the backslash ecape of the interior quote
+      # see: https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
+      #
+      # @api private
+      # @param args [Array<String>] array of command arguments
+      # @return String
+      def combine_args(*args)
+        return args[0] if args.length == 1
+        args.map do |arg|
+          if arg =~ /[ \t\n\v"]/
+            arg = arg.gsub(/(\\*)"/, '\\1\\1\"') # fix interior quotes and any preceding backslashes
+            arg = arg.sub(/(\\+)$/, '\\1\\1') # handle terminating backslashes before the quotes
+            "\"#{arg}\""
+          else
+            arg
+          end
+        end.join(" ")
+      end
+
       def command_to_run(command)
         return run_under_cmd(command) if should_run_under_cmd?(command)
 

spec/mixlib/shellout/windows_spec.rb

@@ -307,4 +307,88 @@ def self.with_command(cmd, filename: nil, search: false, directory: false, &exam
       end
     end
   end
+
+  context "#combine_args" do
+    let(:shell_out) { Mixlib::ShellOut.new }
+    subject { shell_out.send(:combine_args, *largs) }
+
+    def self.with_args(*args, &example)
+      context "with command #{args}" do
+        let(:largs) { args }
+        it(&example)
+      end
+    end
+
+    with_args("echo", "%PATH%") do
+      is_expected.to eql(%q{echo %PATH%})
+    end
+
+    with_args("echo %PATH%") do
+      is_expected.to eql(%q{echo %PATH%})
+    end
+
+    # Note carefully for the following that single quotes in ruby support '\\' as an escape sequence for a single
+    # literal backslash.  It is not mandatory to always use this since '\a' does not esacpe the 's' and is literally
+    # a backlash followed by an 'a'.  However, in the following all backslashes are escaped for consistency.  Otherwise
+    # it becomes prohibitively confusing to track when you need and do not need the escape the backslash (particularly
+    # when the literal string has a trailing backslash such that '\\' must be used instead of '\' which would escape
+    # the intended terminating single quote or %q{\} which escapes the terminating delimiter).
+
+    with_args("child.exe", "argument1", "argument 2", '\\some\\path with\\spaces') do
+      is_expected.to eql(%q{child.exe argument1 "argument 2" "\\some\\path with\\spaces"})
+    end
+
+    with_args("child.exe", "argument1", 'she said, "you had me at hello"', '\\some\\path with\\spaces') do
+      is_expected.to eql(%q{child.exe argument1 "she said, \\"you had me at hello\\"" "\\some\\path with\\spaces"})
+    end
+
+    with_args("child.exe", "argument1", 'argument\\\\"2\\\\"', "argument3", "argument4") do
+      is_expected.to eql(%q{child.exe argument1 "argument\\\\\\\\\\"2\\\\\\\\\\"" argument3 argument4})
+    end
+
+    with_args("child.exe", '\\some\\directory with\\spaces\\', "argument2") do
+      is_expected.to eql(%q{child.exe "\\some\\directory with\\spaces\\\\" argument2})
+    end
+
+    with_args("child.exe", '\\some\\directory with\\\\\\spaces\\\\\\', "argument2") do
+      is_expected.to eql(%q{child.exe "\\some\\directory with\\\\\\spaces\\\\\\\\\\\\" argument2})
+    end
+  end
+
+  context "#run_command" do
+    let(:shell_out) { Mixlib::ShellOut.new(*largs) }
+    subject { shell_out.send(:run_command) }
+
+    def self.with_args(*args, &example)
+      context "with command #{args}" do
+        let(:largs) { args }
+        it(&example)
+      end
+    end
+
+    with_args("echo", "FOO") do
+      is_expected.not_to be_error
+    end
+
+    # Test box is going to have to have c:\program files on it, which is perhaps brittle in principle, but
+    # I don't know enough windows to come up with a less brittle test.  Fix it if you've got a better idea.
+    # The tests need to fail though if the argument is not quoted correctly so using `echo` would be poor
+    # because `echo FOO BAR` and `echo "FOO BAR"` aren't any different.
+
+    with_args('dir c:\program files') do
+      is_expected.to be_error
+    end
+
+    with_args('dir "c:\program files"') do
+      is_expected.not_to be_error
+    end
+
+    with_args("dir", 'c:\program files') do
+      is_expected.not_to be_error
+    end
+
+    with_args("dir", 'c:\program files\\') do
+      is_expected.not_to be_error
+    end
+  end
 end