Permalink
Browse files

Fixes recursive interpolation bug

  • Loading branch information...
1 parent e9e136d commit 5ede63e2d0bbbee84e2a1346122560bbde49a555 @jyurek jyurek committed Oct 22, 2013
Showing with 17 additions and 2 deletions.
  1. +12 −2 lib/cocaine/command_line.rb
  2. +5 −0 spec/cocaine/command_line_spec.rb
View
14 lib/cocaine/command_line.rb
@@ -122,11 +122,21 @@ def environment
end
def interpolate(pattern, interpolations)
- interpolations.inject(pattern) do |command_string, (key, value)|
- command_string.gsub(/:\{?#{key}\b\}?/) { shell_quote(value) }
+ interpolations = stringify_keys(interpolations)
+ pattern.gsub(/:\{?(\w+)\b\}?/) do |match|
+ key = match.tr(":{}", "")
+ interpolations.key?(key) ? shell_quote(interpolations[key]) : match
end
@sshaw
sshaw added a line comment Oct 27, 2013

This change breaks interpolation that had worked with older versions (though maybe it shouldn't have...):

cmd = Cocaine::CommandLine.new("convert", ":one :{two} :{three}")
cmd.command("one" => "a.jpg", "two" => "b.png", "{three}" => "c.png") # convert 'a.jpg' 'b.png' {three}

This make some sense as one must prepend : to denote an interpolation in the command string but strings are accepted as a hash keys making ":key" superfluous and instead written as "key".

@jyurek
thoughtbot, inc. member
jyurek added a line comment Nov 12, 2013

That is not an intended means of interpolation. I'm not interested in changing this new code to re-support it, but thank you for bringing it up. I didn't even realize that was possible earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
+ def stringify_keys(hash)
+ hash = hash.dup
+ hash.keys.each do |key|
+ hash[key.to_s] = hash.delete(key)
+ end
+ hash
+ end
+
def shell_quote(string)
return "" if string.nil?
if unix?
View
5 spec/cocaine/command_line_spec.rb
@@ -104,6 +104,11 @@
command_string.should == "convert '`rm -rf`.jpg' 'ha'\\''ha.png'"
end
+ it 'cannot recursively introduce a place where user-supplied commands can run' do
+ cmd = Cocaine::CommandLine.new('convert', ':foo :bar')
+ cmd.command(:foo => ':bar', :bar => '`rm -rf`').should == 'convert \':bar\' \'`rm -rf`\''
+ end
+
it "can quote and interpolate dangerous variables even on windows" do
on_windows!
cmd = Cocaine::CommandLine.new("convert",

0 comments on commit 5ede63e

Please sign in to comment.