Repository navigation
Escape unsafe OutputBuffer appends without the super dispatch - #59570
Open
thomasklemm wants to merge 1 commit into
Open
thomasklemm wants to merge 1 commit into
thomasklemm wants to merge 1 commit into
Conversation
OutputBuffer#<< calls ERB::Util.unwrapped_html_escape for every value that isn't html_safe, which reaches ERB::Escape.html_escape through an alias and super. Every <%= %> in an ERB template appends this way. Run the same steps inline instead: call to_s on the converted value, check html_safe? again, and otherwise call ERB::Escape.html_escape, the method that super reaches. Every to_s and html_safe? call stays as it was.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every
<%= %>appends throughActionView::OutputBuffer#<<. For a value that isn't html_safe, it callsto_sandhtml_safe?, thenERB::Util.unwrapped_html_escape, which reachesERB::Escape.html_escapethrough an alias and asuperonERB::Util's singleton class. YJIT can't specialize thatsuper, so each append is a dynamic send.This PR runs the same steps inline:
to_son the converted value,html_safe?again, thenERB::Escape.html_escape, the methodsuperreaches. Everyto_sandhtml_safe?call stays.There is one difference. Template appends no longer go through
ERB::Util, so a module prepended to its singleton class that overridesunwrapped_html_escape, orhtml_escapebefore Active Support loads, no longer sees them. Every other caller still goes through it.unwrapped_html_escapeis:nodoc:, and #46573 chosesuperso that such prepends keep working; this keeps that everywhere except template appends.On Ruby 4.0.7, a
<%= %>of a String that isn't html_safe takes 110 ns instead of 141 ns on main in the interpreter, and 52 ns instead of 68 ns with YJIT.On ruby-bench's erubi-rails this runs 5.8% fewer instructions per iteration in the interpreter and 5–6% fewer with YJIT. A page in Fizzy or Lobsters makes 130 to 920
<%= %>appends.