Handle more unsafe String methods (#33990)

* Handle more unsafe String methods

* Fix codeclimate issue

* Revert stylistic change

[Janosch Müller + Rafael Mendonça França]

2 files changed, 82 insertions, 9 deletions

diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb
index 81299e5b58..a26473dc84 100644
--- a/activesupport/test/core_ext/string_ext_test.rb
+++ b/activesupport/test/core_ext/string_ext_test.rb
@@ -892,6 +892,54 @@ class OutputSafetyTest < ActiveSupport::TestCase
     assert_predicate string, :html_safe?
   end
 
+  test "Inserting safe into safe yields safe" do
+    string = "foo".html_safe
+    string.insert(0, "<b>".html_safe)
+
+    assert_equal "<b>foo", string
+    assert_predicate string, :html_safe?
+  end
+
+  test "Inserting unsafe into safe yields escaped safe" do
+    string = "foo".html_safe
+    string.insert(0, "<b>")
+
+    assert_equal "&lt;b&gt;foo", string
+    assert_predicate string, :html_safe?
+  end
+
+  test "Replacing safe with safe yields safe" do
+    string = "foo".html_safe
+    string.replace("<b>".html_safe)
+
+    assert_equal "<b>", string
+    assert_predicate string, :html_safe?
+  end
+
+  test "Replacing safe with unsafe yields escaped safe" do
+    string = "foo".html_safe
+    string.replace("<b>")
+
+    assert_equal "&lt;b&gt;", string
+    assert_predicate string, :html_safe?
+  end
+
+  test "Replacing index of safe with safe yields safe" do
+    string = "foo".html_safe
+    string[0] = "<b>".html_safe
+
+    assert_equal "<b>oo", string
+    assert_predicate string, :html_safe?
+  end
+
+  test "Replacing index of safe with unsafe yields escaped safe" do
+    string = "foo".html_safe
+    string[0] = "<b>"
+
+    assert_equal "&lt;b&gt;oo", string
+    assert_predicate string, :html_safe?
+  end
+
   test "emits normal string yaml" do
     assert_equal "foo".to_yaml, "foo".html_safe.to_yaml(foo: 1)
   end
diff --git a/activesupport/test/safe_buffer_test.rb b/activesupport/test/safe_buffer_test.rb
index 70dec6b3d2..49a3951623 100644
--- a/activesupport/test/safe_buffer_test.rb
+++ b/activesupport/test/safe_buffer_test.rb
@@ -75,16 +75,41 @@ class SafeBufferTest < ActiveSupport::TestCase
     assert_equal "my_test", str
   end
 
-  test "Should not return safe buffer from gsub" do
-    altered_buffer = @buffer.gsub("", "asdf")
-    assert_equal "asdf", altered_buffer
-    assert_not_predicate altered_buffer, :html_safe?
-  end
+  {
+    capitalize: nil,
+    chomp: nil,
+    chop: nil,
+    delete: "foo",
+    delete_prefix: "foo",
+    delete_suffix: "foo",
+    downcase: nil,
+    gsub: ["foo", "bar"],
+    lstrip: nil,
+    next: nil,
+    reverse: nil,
+    rstrip: nil,
+    slice: "foo",
+    squeeze: nil,
+    strip: nil,
+    sub: ["foo", "bar"],
+    succ: nil,
+    swapcase: nil,
+    tr: ["foo", "bar"],
+    tr_s: ["foo", "bar"],
+    unicode_normalize: nil,
+    upcase: nil,
+  }.each do |unsafe_method, dummy_args|
+    test "Should not return safe buffer from #{unsafe_method}" do
+      skip unless String.method_defined?(unsafe_method)
+      altered_buffer = @buffer.send(unsafe_method, *dummy_args)
+      assert_not_predicate altered_buffer, :html_safe?
+    end
 
-  test "Should not return safe buffer from gsub!" do
-    @buffer.gsub!("", "asdf")
-    assert_equal "asdf", @buffer
-    assert_not_predicate @buffer, :html_safe?
+    test "Should not return safe buffer from #{unsafe_method}!" do
+      skip unless String.method_defined?("#{unsafe_method}!")
+      @buffer.send("#{unsafe_method}!", *dummy_args)
+      assert_not_predicate @buffer, :html_safe?
+    end
   end
 
   test "Should escape dirty buffers on add" do