phlex-ruby / phlex-rails

An object-oriented alternative to ActionView for Ruby on Rails.
https://www.phlex.fun
MIT License
222 stars 35 forks source link

ActiveSupport::SafeBuffer values as attributes are HTML-escaped #212

Closed nielsslot closed 1 month ago

nielsslot commented 3 months ago

If an ActiveSupport::SafeBuffer value is used as an attribute it gets HTML-escaped, possibly resulting in a double escaped attribute in the output.

For example, I would expect the following two statements to render the same HTML:

div(class: token_list("[&_div]"))
div(class: "[&_div]")

But this renders the following:

<div class="[&amp;amp;_div]"></div>
<div class="[&amp;_div]"></div>

The ampersand in the return value from token_list is escaped again and is not rendered as &amp; but as &amp;amp;.

token_list here is the Rails helper also known as class_names and it returns an ActiveSupport::SafeBuffer.

Could phlex-rails include special handling for attributes passed as ActiveSupport::SafeBuffer so that those values aren't escaped again?

I did also see that the new version of Phlex might include similar features to token_list. That might make the use of token_list within Phlex unneeded, but this issue should apply to any ActiveSupport::SafeBuffer used as an attribute.

joeldrapper commented 1 month ago

Yeah, I agree we should probably respect ActiveSupport::SafeBuffer here.

joeldrapper commented 1 month ago

I believe this is already fixed in main.

joeldrapper commented 1 month ago

Hey @nielsslot, turns out Phlex isn’t HTML-escaping this. It’s returned from Rails already escaped. I would say you can use the tokens helper except we HTML escape too. That is something I can fix. It is already fixed in the alpha of Phlex 2.

joeldrapper commented 1 month ago

Hey @nielsslot, looking into this further, I don’t think we can fix this in v1. If we did, we would need to make srcdoc an unsafe attribute. Since there’s currently no way to override Phlex’ attribute safety, marking srcdoc as unsafe would cause problems.

Good news is this is not an issue in v2 and you can try the alpha now. https://github.com/orgs/phlex-ruby/discussions/769