Permalink
Browse files

Fixes interpolation on SafeBuffer

Interpolation was untested and did not work with hash arguments.

Adds
- support for interpolation with hash argument
- tests for the above
- tests for safe/unsafe interpolation
  • Loading branch information...
1 parent 12affbe commit a764938ad0ddb0aa73bb86215626f24b980e3f55 @mezis mezis committed Dec 13, 2013
@@ -183,15 +183,14 @@ def +(other)
end
def %(args)
- args = Array(args).map do |arg|
- if !html_safe? || arg.html_safe?
- arg
- else
- ERB::Util.h(arg)
- end
+ case args
+ when Hash
@plentz

plentz Dec 22, 2013

Contributor

why use case instead of a simpler if/else?

@mezis

mezis via email Dec 22, 2013

Contributor
@dmathieu

dmathieu Dec 22, 2013

Contributor

In cases where there are only a few possibilities, like here, I also believe if to be a better and more readable option.

@mezis

mezis Dec 22, 2013

Contributor

Besides my previous argument (or lack thereof), switching on types using case felt closer to what's prevalent in the Rails codebase, and in well-journeyed styleguides like Github's.

This said, if you feel strongly about it, again you're most welcome to change it.

@dmathieu

dmathieu Dec 22, 2013

Contributor

It doesn't matter very much. And committing only this would fall under the cosmetic change category 😉

+ escaped_args = Hash[args.map { |k,arg| [k, html_escape_interpolated_argument(arg)] }]
+ else
+ escaped_args = Array(args).map { |arg| html_escape_interpolated_argument(arg) }
end
- self.class.new(super(args))
+ self.class.new(super(escaped_args))
end
def html_safe?
@@ -224,6 +223,12 @@ def #{unsafe_method}!(*args) # def capitalize!(*args)
EOT
end
end
+
+ private
+
+ def html_escape_interpolated_argument(arg)
+ (!html_safe? || arg.html_safe?) ? arg : ERB::Util.h(arg)
+ end
end
end
@@ -140,4 +140,29 @@ def test_titleize
# should still be unsafe
assert !y.html_safe?, "should not be safe"
end
+
+ test 'Should work with interpolation (array argument)' do
+ x = 'foo %s bar'.html_safe % ['qux']
+ assert_equal 'foo qux bar', x
+ end
+
+ test 'Should work with interpolation (hash argument)' do
+ x = 'foo %{x} bar'.html_safe % { x: 'qux' }
+ assert_equal 'foo qux bar', x
+ end
+
+ test 'Should escape unsafe interpolated args' do
+ x = 'foo %{x} bar'.html_safe % { x: '<br/>' }
+ assert_equal 'foo &lt;br/&gt; bar', x
+ end
+
+ test 'Should not escape safe interpolated args' do
+ x = 'foo %{x} bar'.html_safe % { x: '<br/>'.html_safe }
+ assert_equal 'foo <br/> bar', x
+ end
+
+ test 'Should interpolate to a safe string' do
+ x = 'foo %{x} bar'.html_safe % { x: 'qux' }
+ assert x.html_safe?, 'should be safe'
+ end
end

0 comments on commit a764938

Please sign in to comment.