Skip to content

Commit

Permalink
Merge pull request #182 from chef/lcg/win-array-args
Browse files Browse the repository at this point in the history
Support array args on windows WIP
  • Loading branch information
lamont-granquist authored Jun 6, 2019
2 parents a9146e5 + 9b36f24 commit 5c86a66
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 3 deletions.
2 changes: 1 addition & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
44 changes: 42 additions & 2 deletions lib/mixlib/shellout/windows.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Author:: Daniel DeLeo (<dan@chef.io>)
# Author:: John Keiser (<jkeiser@chef.io>)
# Author:: Ho-Sheng Hsiao (<hosh@chef.io>)
# 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");
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<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\"') # 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)

Expand Down
84 changes: 84 additions & 0 deletions spec/mixlib/shellout/windows_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 5c86a66

Please sign in to comment.