Skip to content

Conversation

flavorjones
Copy link
Member

@flavorjones flavorjones commented May 20, 2023

Motivation / Background

This PR updates the behavior of ERB::Util.html_escape_once to always return an html_safe string.

This method previously maintained the html_safe? property of a string on the return value. Because this string has been escaped, however, not marking it as html_safe causes entities to be double-escaped.

As an example, take this view snippet:

<p><%= html_escape_once("this & that &amp; the other") %></p>

Before this change, that would be double-escaped and render as:

<p>this &amp;amp; that &amp;amp; the other</p>

After this change, it renders correctly as:

<p>this &amp; that &amp; the other</p>

Fixes #48256

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@flavorjones flavorjones requested review from matthewd and jhawthorn May 20, 2023 17:37
@flavorjones flavorjones changed the title ERB::Util.html_escape_once always returns an html_safe string update ERB::Util.html_escape_once to always returns an html_safe string May 22, 2023
@flavorjones flavorjones changed the title update ERB::Util.html_escape_once to always returns an html_safe string update ERB::Util.html_escape_once to always return an html_safe string May 22, 2023
This method previously maintained the `html_safe?` property of a string on the return
value. Because this string has been escaped, however, not marking it as `html_safe` causes
entities to be double-escaped.

As an example, take this view snippet:

  ```html
  <p><%= html_escape_once("this & that &amp; the other") %></p>
  ```

Before this change, that would be double-escaped and render as:

  ```html
  <p>this &amp;amp; that &amp;amp; the other</p>
  ```

After this change, it renders correctly as:

  ```html
  <p>this &amp; that &amp; the other</p>
  ```

[Fix rails#48256]
@byroot byroot force-pushed the flavorjones-html-escape-once-is-html-safe branch from 6bf7887 to aea8849 Compare May 22, 2023 10:02
@byroot
Copy link
Member

byroot commented May 22, 2023

That just make sense. html_escape returns a safe string, html_escape_once should be consistent with it.

@byroot byroot merged commit 1c69f50 into rails:main May 22, 2023
@flavorjones flavorjones deleted the flavorjones-html-escape-once-is-html-safe branch May 22, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

html_escape_once decodes items flagged html_safe
2 participants