safe_concat should not work on dirty buffers.

2 files changed, 20 insertions, 4 deletions

diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb
index c56ac16203..71f3879e49 100644
--- a/activesupport/lib/active_support/core_ext/string/output_safety.rb
+++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -77,10 +77,19 @@ module ActiveSupport #:nodoc:
   class SafeBuffer < String
     UNSAFE_STRING_METHODS = ["capitalize", "chomp", "chop", "delete", "downcase", "gsub", "lstrip", "next", "reverse", "rstrip", "slice", "squeeze", "strip", "sub", "succ", "swapcase", "tr", "tr_s", "upcase"].freeze
 
-    # TODO: Should safe_concat check if the current buffer is dirty or not?
-    # We should probably raise as it would mean we are adding concatenating
-    # to something that is safe but it actually isn't.
-    alias safe_concat concat
+    alias_method :original_concat, :concat
+    private :original_concat
+
+    class SafeConcatError < StandardError
+      def initialize
+        super "Could not concatenate to the buffer because it is not html safe."
+      end
+    end
+
+    def safe_concat(value)
+      raise SafeConcatError if dirty?
+      original_concat(value)
+    end
 
     def initialize(*)
       @dirty = false
diff --git a/activesupport/test/safe_buffer_test.rb b/activesupport/test/safe_buffer_test.rb
index 9c37458c93..a8b39d2c38 100644
--- a/activesupport/test/safe_buffer_test.rb
+++ b/activesupport/test/safe_buffer_test.rb
@@ -97,4 +97,11 @@ class SafeBufferTest < ActiveSupport::TestCase
     @buffer.gsub!('', '<>')
     assert !@buffer.dup.html_safe?
   end
+
+  test "Should raise an error when safe_concat is called on dirty buffers" do
+    @buffer.gsub!('', '<>')
+    assert_raise ActiveSupport::SafeBuffer::SafeConcatError do
+      @buffer.safe_concat "BUSTED"
+    end
+  end
 end