Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speedup ActionView::OutputBuffer #45614

Merged
merged 1 commit into from Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 48 additions & 8 deletions actionview/lib/action_view/buffers.rb
Expand Up @@ -18,24 +18,64 @@ module ActionView
# sbuf << 5
# puts sbuf # => "hello\u0005"
#
class OutputBuffer < ActiveSupport::SafeBuffer # :nodoc:
def initialize(*)
super
encode!
class OutputBuffer # :nodoc:
def initialize(buffer = "")
@buffer = String.new(buffer)
@buffer.encode!
end

delegate :length, :blank?, :encoding, :encode!, :force_encoding, to: :@buffer

def to_s
@buffer.html_safe
end
alias_method :html_safe, :to_s

def to_str
@buffer.dup
end

def html_safe?
true
end

def <<(value)
return self if value.nil?
super(value.to_s)
unless value.nil?
casperisfine marked this conversation as resolved.
Show resolved Hide resolved
value = value.to_s
@buffer << if value.html_safe?
value
else
CGI.escapeHTML(value)
end
end
self
end
alias :append= :<<

def safe_concat(value)
@buffer << value
self
end
alias :safe_append= :safe_concat

def safe_expr_append=(val)
return self if val.nil?
safe_concat val.to_s
@buffer << val.to_s
self
end

alias :safe_append= :safe_concat
def initialize_copy(other)
@buffer = other.to_str
end

# Don't use this
def slice!(range)
@buffer.slice!(range)
end

def ==(other)
other.class == self.class && @buffer == other.to_str
end
end

class StreamingBuffer # :nodoc:
Expand Down
3 changes: 3 additions & 0 deletions actionview/lib/action_view/helpers/cache_helper.rb
Expand Up @@ -285,6 +285,9 @@ def write_fragment_for(name, options)
pos = output_buffer.length
yield
output_safe = output_buffer.html_safe?
# We need to modify the buffer in place to be deal with the view handlers
# like `builder` that don't access the buffer through `@output_buffer` but
# keep the initial reference.
fragment = output_buffer.slice!(pos..-1)
if output_safe
self.output_buffer = output_buffer.class.new(output_buffer)
Expand Down
10 changes: 8 additions & 2 deletions actionview/lib/action_view/helpers/capture_helper.rb
Expand Up @@ -43,8 +43,14 @@ module CaptureHelper
def capture(*args)
value = nil
buffer = with_output_buffer { value = yield(*args) }
if (string = buffer.presence || value) && string.is_a?(String)
ERB::Util.html_escape string

case string = buffer.presence || value
when OutputBuffer
string.to_s
when ActiveSupport::SafeBuffer
string
when String
ERB::Util.html_escape(string)
end
end

Expand Down
2 changes: 1 addition & 1 deletion actionview/lib/action_view/renderer/abstract_renderer.rb
Expand Up @@ -142,7 +142,7 @@ class RenderedTemplate # :nodoc:
attr_reader :body, :template

def initialize(body, template)
@body = body
@body = body.to_s
@template = template
end

Expand Down
31 changes: 31 additions & 0 deletions actionview/test/output_buffer_test.rb
@@ -0,0 +1,31 @@
# frozen_string_literal: true

require "abstract_unit"

class TestOutputBuffer < ActiveSupport::TestCase
setup do
@buffer = ActionView::OutputBuffer.new
end

test "#<< maintains HTML safety" do
@buffer << "<script>alert('pwned!')</script>"
assert_predicate @buffer, :html_safe?
assert_predicate @buffer.to_s, :html_safe?
assert_equal "&lt;script&gt;alert(&#39;pwned!&#39;)&lt;/script&gt;", @buffer.to_s
end

test "#safe_append= bypasses HTML safety" do
@buffer.safe_append = "<p>This is fine</p>"
assert_predicate @buffer, :html_safe?
assert_predicate @buffer.to_s, :html_safe?
assert_equal "<p>This is fine</p>", @buffer.to_s
end

test "can be duped" do
@buffer << "Hello"
copy = @buffer.dup
copy << " World!"
assert_equal "Hello World!", copy.to_s
assert_equal "Hello", @buffer.to_s
end
end
6 changes: 3 additions & 3 deletions actionview/test/template/capture_helper_test.rb
Expand Up @@ -182,7 +182,7 @@ def test_with_output_buffer_swaps_the_output_buffer_given_no_argument
buffer = @av.with_output_buffer do
@av.output_buffer << "."
end
assert_equal ".", buffer
assert_equal ".", buffer.to_s
assert_nil @av.output_buffer
end

Expand All @@ -192,7 +192,7 @@ def test_with_output_buffer_swaps_the_output_buffer_with_an_argument
@av.with_output_buffer(buffer) do
@av.output_buffer << "."
end
assert_equal "..", buffer
assert_equal "..", buffer.to_s
assert_nil @av.output_buffer
end

Expand All @@ -219,7 +219,7 @@ def test_with_output_buffer_sets_proper_encoding

def test_with_output_buffer_does_not_assume_there_is_an_output_buffer
assert_nil @av.output_buffer
assert_equal "", @av.with_output_buffer { }
assert_equal "", @av.with_output_buffer { }.to_s
end

def alt_encoding(output_buffer)
Expand Down