diff --git a/Rakefile b/Rakefile index dd326ab6..40ab182b 100644 --- a/Rakefile +++ b/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| diff --git a/lib/mixlib/shellout/windows.rb b/lib/mixlib/shellout/windows.rb index 27adbbd0..cde795b4 100644 --- a/lib/mixlib/shellout/windows.rb +++ b/lib/mixlib/shellout/windows.rb @@ -2,7 +2,7 @@ # Author:: Daniel DeLeo () # Author:: John Keiser () # Author:: Ho-Sheng Hsiao () -# Copyright:: Copyright (c) 2011-2016 Chef Software, Inc. +# Copyright:: Copyright (c) 2011-2019, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -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,46 @@ 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 (double 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 + # themselves be backslash excaped along with the backslash ecape of the interior quote (double plus one backslashes). + # + # And to restate. We are constructing a string which will be parsed by the windows parser into arguments, and we want those + # arguments to match the *args array we are passed here. So call the windows parser operation A then we need to apply A^-1 to + # our args to construct the string so that applying A gives windows back our *args. + # + # And when the windows parser sees a series of backslashes followed by a double quote, it has to determine if that double quote + # is terminating or not, and how many backslashes to insert in the args. So what it does is divide it by two (rounding down) to + # get the number of backslashes to insert. Then if it is even the double quotes terminate the argument. If it is even the + # double quotes are interior double quotes (the extra backslash quotes the double quote). + # + # We construct the inverse operation so interior double quotes preceeded by N backslashes get 2N+1 backslashes in front of the quote, + # while trailing N backslashes get 2N backslashes in front of the quote that terminates the argument. + # + # see: https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/ + # + # @api private + # @param args [Array] 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\"') # interior quotes with N preceeding backslashes need 2N+1 backslashes + arg = arg.sub(/(\\+)$/, '\1\1') # trailing N backslashes need to become 2N backslashes + "\"#{arg}\"" + else + arg + end + end.join(" ") + end + def command_to_run(command) return run_under_cmd(command) if should_run_under_cmd?(command) diff --git a/spec/mixlib/shellout/windows_spec.rb b/spec/mixlib/shellout/windows_spec.rb index d4edabc5..10115608 100644 --- a/spec/mixlib/shellout/windows_spec.rb +++ b/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 '\d' does not escape the 'd' and is literally + # a backlash followed by an 'd'. 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('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('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('child.exe argument1 "argument\\\\\\\\\\"2\\\\\\\\\\"" argument3 argument4') + end + + with_args("child.exe", '\\some\\directory with\\spaces\\', "argument2") do + is_expected.to eql('child.exe "\\some\\directory with\\spaces\\\\" argument2') + end + + with_args("child.exe", '\\some\\directory with\\\\\\spaces\\\\\\', "argument2") do + is_expected.to eql('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