Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Restore performance of ERB::Util.html_escape

Revert html_escape to do a single gsub again, but add the "n" flag (no
language, i.e. not multi-byte) to protect against XSS via invalid utf8

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information...
commit 1583dabf66b68a71ad0fa48d31c27d6bc040d0b4 1 parent 2512192
@jenseng jenseng authored josevalim committed
View
2  activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -20,7 +20,7 @@ def html_escape(s)
if s.html_safe?
s
else
- s.gsub(/&/, "&amp;").gsub(/\"/, "&quot;").gsub(/>/, "&gt;").gsub(/</, "&lt;").html_safe
+ s.gsub(/[&"><]/n) { |special| HTML_ESCAPE[special] }.html_safe
end
end
View
23 activesupport/test/core_ext/string_ext_test.rb
@@ -13,12 +13,6 @@
class StringInflectionsTest < Test::Unit::TestCase
include InflectorTestCases
- def test_erb_escape
- string = [192, 60].pack('CC')
- expected = 192.chr + "&lt;"
- assert_equal expected, ERB::Util.html_escape(string)
- end
-
def test_strip_heredoc_on_an_empty_string
assert_equal '', ''.strip_heredoc
end
@@ -468,6 +462,23 @@ def to_s
assert string.html_safe?
assert !string.to_param.html_safe?
end
+
+ test "ERB::Util.html_escape should escape unsafe characters" do
+ string = '<>&"'
+ expected = '&lt;&gt;&amp;&quot;'
+ assert_equal expected, ERB::Util.html_escape(string)
+ end
+
+ test "ERB::Util.html_escape should correctly handle invalid UTF-8 strings" do
+ string = [192, 60].pack('CC')
+ expected = 192.chr + "&lt;"
+ assert_equal expected, ERB::Util.html_escape(string)
+ end
+
+ test "ERB::Util.html_escape should not escape safe strings" do
+ string = "<b>hello</b>".html_safe
+ assert_equal string, ERB::Util.html_escape(string)
+ end
end
class StringExcludeTest < ActiveSupport::TestCase

1 comment on commit 1583dab

@leehambley

Does this not cause many (thousands, in my case) warnings matching the following?

activesupport-3.1.4/lib/active_support/core_ext/string/output_safety.rb:24: warning: regexp match /.../n against to UTF-8 string

It is important to be able to escape I18n localized strings in European languages too. What's the suggested fix/workaround? (for example html_escaping something like Bestätigungs-E-Mail anfordern, done by I18n, when that string exists in a translation file)

Please sign in to comment.
Something went wrong with that request. Please try again.