Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Shift SafeBuffer#concat responsibility over to rails_xss

  • Loading branch information...
commit a815f0c5a3a873aefca76f459ce05ddde73080db 1 parent 9da7ff8
@jeremy jeremy authored
View
9 activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -64,15 +64,6 @@ module ActiveSupport #:nodoc:
class SafeBuffer < String
alias safe_concat concat
- def concat(value)
- if value.html_safe?
- super(value)
- else
- super(ERB::Util.h(value))
- end
- end
- alias << concat
-
def +(other)
dup.concat(other)
end
View
27 activesupport/test/core_ext/string_ext_test.rb
@@ -287,11 +287,6 @@ def test_bytesize
class OutputSafetyTest < ActiveSupport::TestCase
def setup
@string = "hello"
- @object = Class.new(Object) do
- def to_s
- "other"
- end
- end.new
end
test "A string is unsafe by default" do
@@ -316,15 +311,7 @@ def to_s
end
test "An object is unsafe by default" do
- assert !@object.html_safe?
- end
-
- test "Adding an object to a safe string returns a safe string" do
- string = @string.html_safe
- string << @object
-
- assert_equal "helloother", string
- assert string.html_safe?
+ assert !Object.new.html_safe?
end
test "Adding a safe string to another safe string returns a safe string" do
@@ -336,12 +323,12 @@ def to_s
assert @combination.html_safe?
end
- test "Adding an unsafe string to a safe string escapes it and returns a safe string" do
+ test "Adding an unsafe string to a safe string doesn't escape it without rails_xss but returns a safe string" do
@other_string = "other".html_safe
@combination = @other_string + "<foo>"
@other_combination = @string + "<foo>"
- assert_equal "other&lt;foo&gt;", @combination
+ assert_equal "other<foo>", @combination
assert_equal "hello<foo>", @other_combination
assert @combination.html_safe?
@@ -356,10 +343,10 @@ def to_s
assert !@other_string.html_safe?
end
- test "Concatting unsafe onto safe yields escaped safe" do
+ test "Concatting unsafe onto safe yields safe by not escaped without rails_xss" do
@matchu
matchu added a note

Should be "but" instead of "by"? Or am I just tired?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@other_string = "other".html_safe
string = @other_string.concat("<foo>")
- assert_equal "other&lt;foo&gt;", string
+ assert_equal "other<foo>", string
assert string.html_safe?
end
@@ -379,10 +366,10 @@ def to_s
assert !@other_string.html_safe?
end
- test "Concatting unsafe onto safe with << yields escaped safe" do
+ test "Concatting unsafe onto safe with << yields safe but not escaped without rails_xss" do
@other_string = "other".html_safe
string = @other_string << "<foo>"
- assert_equal "other&lt;foo&gt;", string
+ assert_equal "other<foo>", string
assert string.html_safe?
end
View
4 activesupport/test/safe_buffer_test.rb
@@ -10,9 +10,9 @@ def setup
assert_equal "", @buffer
end
- test "Should escape a raw string which is passed to them" do
+ test "Should not escape a raw string unless using rails_xss" do
@buffer << "<script>"
- assert_equal "&lt;script&gt;", @buffer
+ assert_equal "<script>", @buffer
end
test "Should NOT escape a safe value passed to it" do

2 comments on commit a815f0c

@nicolasblanco

Great fix! Is there a workaround for all helpers that concat strings or should we wait for 2.3.9?
Thanks.

@matchu

Should be "but" instead of "by"? Or am I just tired?

@johndouthat

+1 Thanks!

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