Handle invalid UTF-8 strings when HTML escaping

Use `ActiveSupport::Multibyte::Unicode.tidy_bytes` to handle invalid UTF-8 strings in `ERB::Util.unwrapped_html_escape` and `ERB::Util.html_escape_once`. Prevents user-entered input passed from a querystring into a form field from causing invalid byte sequence errors.
rails · Jun 8, 2015 · 05a2a6a · 05a2a6a
1 parent a69e0a5
commit 05a2a6a
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 4 deletions.
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
@@ -1,3 +1,12 @@
+*   Handle invalid UTF-8 strings when HTML escaping
+
+    Use `ActiveSupport::Multibyte::Unicode.tidy_bytes` to handle invalid UTF-8
+    strings in `ERB::Util.unwrapped_html_escape` and `ERB::Util.html_escape_once`.
+    Prevents user-entered input passed from a querystring into a form field from
+    causing invalid byte sequence errors.
+
+    *Grey Baker*
+
 *   Fix a range of values for parameters of the Time#change
 
     *Nikolay Kondratyev*

diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -37,7 +37,7 @@ def unwrapped_html_escape(s) # :nodoc:
       if s.html_safe?
         s
       else
-        s.gsub(HTML_ESCAPE_REGEXP, HTML_ESCAPE)
+        ActiveSupport::Multibyte::Unicode.tidy_bytes(s).gsub(HTML_ESCAPE_REGEXP, HTML_ESCAPE)
       end
     end
     module_function :unwrapped_html_escape
@@ -50,7 +50,7 @@ def unwrapped_html_escape(s) # :nodoc:
     #   html_escape_once('&lt;&lt; Accept & Checkout')
     #   # => "&lt;&lt; Accept &amp; Checkout"
     def html_escape_once(s)
-      result = s.to_s.gsub(HTML_ESCAPE_ONCE_REGEXP, HTML_ESCAPE)
+      result = ActiveSupport::Multibyte::Unicode.tidy_bytes(s.to_s).gsub(HTML_ESCAPE_ONCE_REGEXP, HTML_ESCAPE)
       s.html_safe? ? result.html_safe : result
     end
 

diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb
@@ -782,8 +782,8 @@ def to_s
   end
 
   test "ERB::Util.html_escape should correctly handle invalid UTF-8 strings" do
-    string = [192, 60].pack('CC')
-    expected = 192.chr + "&lt;"
+    string = "\251 <"
+    expected = "© &lt;"
     assert_equal expected, ERB::Util.html_escape(string)
   end
 
@@ -799,6 +799,12 @@ def to_s
     assert_equal escaped_string, ERB::Util.html_escape_once(string)
     assert_equal escaped_string, ERB::Util.html_escape_once(escaped_string)
   end
+
+  test "ERB::Util.html_escape_once should correctly handle invalid UTF-8 strings" do
+    string = "\251 <"
+    expected = "© &lt;"
+    assert_equal expected, ERB::Util.html_escape_once(string)
+  end
 end
 
 class StringExcludeTest < ActiveSupport::TestCase