Ensure Active Record error related view helpers escape the message [#1280 state:resolved] [Inge Jørgensen, Dan Barry]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>

2 files changed, 36 insertions, 2 deletions

diff --git a/actionpack/lib/action_view/helpers/active_record_helper.rb b/actionpack/lib/action_view/helpers/active_record_helper.rb
index 8b56d241ae..541899ea6a 100644
--- a/actionpack/lib/action_view/helpers/active_record_helper.rb
+++ b/actionpack/lib/action_view/helpers/active_record_helper.rb
@@ -121,7 +121,7 @@ module ActionView
         if (obj = (object.respond_to?(:errors) ? object : instance_variable_get("@#{object}"))) &&
           (errors = obj.errors.on(method))
           content_tag("div",
-            "#{options[:prepend_text]}#{errors.is_a?(Array) ? errors.first : errors}#{options[:append_text]}",
+            "#{options[:prepend_text]}#{ERB::Util.html_escape(errors.is_a?(Array) ? errors.first : errors)}#{options[:append_text]}",
             :class => options[:css_class]
           )
         else
@@ -198,7 +198,7 @@ module ActionView
               locale.t :header, :count => count, :model => object_name
             end
             message = options.include?(:message) ? options[:message] : locale.t(:body)
-            error_messages = objects.sum {|object| object.errors.full_messages.map {|msg| content_tag(:li, msg) } }.join
+            error_messages = objects.sum {|object| object.errors.full_messages.map {|msg| content_tag(:li, ERB::Util.html_escape(msg)) } }.join
 
             contents = ''
             contents << content_tag(options[:header_tag] || :h2, header_message) unless header_message.blank?
diff --git a/actionpack/test/template/active_record_helper_test.rb b/actionpack/test/template/active_record_helper_test.rb
index e46f95d18b..83c028b5f2 100644
--- a/actionpack/test/template/active_record_helper_test.rb
+++ b/actionpack/test/template/active_record_helper_test.rb
@@ -19,6 +19,30 @@ class ActiveRecordHelperTest < ActionView::TestCase
     Column = Struct.new("Column", :type, :name, :human_name)
   end
 
+  class DirtyPost
+    class Errors
+      def empty?
+        false
+      end
+
+      def count
+        1
+      end
+
+      def full_messages
+        ["Author name can't be <em>empty</em>"]
+      end
+
+      def on(field)
+        "can't be <em>empty</em>"
+      end
+    end
+
+    def errors
+      Errors.new
+    end
+  end
+
   def setup_post
     @post = Post.new
     def @post.errors
@@ -195,10 +219,20 @@ class ActiveRecordHelperTest < ActionView::TestCase
     assert_equal %(<div class="errorDeathByClass"><h1>1 error prohibited this post from being saved</h1><p>There were problems with the following fields:</p><ul><li>Author name can't be empty</li></ul></div>), error_messages_for("post", :class => "errorDeathByClass", :id => nil, :header_tag => "h1")
   end
 
+  def test_error_messages_for_escapes_html
+    @dirty_post = DirtyPost.new
+    assert_dom_equal %(<div class="errorExplanation" id="errorExplanation"><h2>1 error prohibited this dirty post from being saved</h2><p>There were problems with the following fields:</p><ul><li>Author name can't be &lt;em&gt;empty&lt;/em&gt;</li></ul></div>), error_messages_for("dirty_post")
+  end
+
   def test_error_messages_for_handles_nil
     assert_equal "", error_messages_for("notthere")
   end
 
+  def test_error_message_on_escapes_html
+    @dirty_post = DirtyPost.new
+    assert_dom_equal "<div class=\"formError\">can't be &lt;em&gt;empty&lt;/em&gt;</div>", error_message_on(:dirty_post, :author_name)
+  end
+
   def test_error_message_on_handles_nil
     assert_equal "", error_message_on("notthere", "notthere")
   end