Skip to content
This repository
Browse code

Ensure that the strings returned by SafeBuffer#gsub and friends aren'…

…t considered html_safe?

Also make sure that the versions of those methods which modify a string in place such as gsub! can't be called on safe buffers at all.

Conflicts:

	activesupport/test/safe_buffer_test.rb
  • Loading branch information...
commit 1300c034775a5d52ad9141fdf5bbdbb9159df96a 1 parent 7d1782a
Michael Koziarski authored June 08, 2011 tenderlove committed June 07, 2011
13  activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -74,6 +74,7 @@ def html_safe?
74 74
 
75 75
 module ActiveSupport #:nodoc:
76 76
   class SafeBuffer < String
  77
+    UNSAFE_STRING_METHODS = ["capitalize", "chomp", "chop", "delete", "downcase", "gsub", "lstrip", "next", "reverse", "rstrip", "slice", "squeeze", "strip", "sub", "succ", "swapcase", "tr", "tr_s", "upcase"].freeze
77 78
     alias safe_concat concat
78 79
 
79 80
     def concat(value)
@@ -110,6 +111,18 @@ def to_yaml(*args)
110 111
 
111 112
       to_str.to_yaml(*args)
112 113
     end
  114
+
  115
+    for unsafe_method in UNSAFE_STRING_METHODS
  116
+      class_eval <<-EOT, __FILE__, __LINE__
  117
+        def #{unsafe_method}(*args)
  118
+          super.to_str
  119
+        end
  120
+
  121
+        def #{unsafe_method}!(*args)
  122
+          raise TypeError, "Cannot modify SafeBuffer in place"
  123
+        end
  124
+      EOT
  125
+    end
113 126
   end
114 127
 end
115 128
 
12  activesupport/test/safe_buffer_test.rb
@@ -60,4 +60,16 @@ def test_nested
60 60
     yaml = YAML.dump data
61 61
     assert_equal({'str' => str}, YAML.load(yaml))
62 62
   end
  63
+
  64
+  test "Should not return safe buffer from gsub" do
  65
+    altered_buffer = @buffer.gsub('', 'asdf')
  66
+    assert_equal 'asdf', altered_buffer
  67
+    assert !altered_buffer.html_safe?
  68
+  end
  69
+
  70
+  test "Should not allow gsub! on safe buffers" do
  71
+    assert_raise TypeError do
  72
+      @buffer.gsub!('', 'asdf')
  73
+    end
  74
+  end
63 75
 end

2 notes on commit 1300c03

Josh Goebel

How would capitalize, strip, downcase, upcase, etc. change a safe buffer in such a way that it was no longer safe? gsub and some others I can perfectly understand, but...?

Steve Klabnik
Collaborator

Proving that it's not possible isn't simple. And it has to be not possible.

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