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

Prevent html: escaping in broadcast later actions #362

Merged
merged 1 commit into from Jul 19, 2022

Conversation

afcapel
Copy link
Contributor

@afcapel afcapel commented Jul 19, 2022

hotwire-rails documents using an html: argument to broadcast HTML directly, bypassing the rendering mechanism.

However, there's a catch: when using html: with a broadcast later action and a queue system, the html_safe flag of
the html argument might be lost in the serialization process. Specifically, the flag remains with queue backends that work in-process (like inline or async) but it's lost in others that serialize the payload to JSON (like resque
or sidekiq).

Also, render escapes the html: param when it isn't html_safe.

The resulting effect is that the broadcast action works fine in development, but then fails when it's deployed to a production system.

This problem seems to have tripped a few people already.

The solution is giving the html: argument the same treatment as the existing content: param. This works because the we are bypassing render and hence there's no escaping.

broadcast_action_to accepts a content: argument to set the
turbo-stream template content directly, bypassing any rendering. Other
params, like partial: or locals:  are delegated to render.

Since the arguments are passed directly to render, you might expect you can
also use html: as an argument here. However, there's a catch: when using
a queue system to broadcast the action later, the html_safe flag of
the html param might be lost in the serialization process. Specifically,
the flag remains with queue backends that work in process (like `inline` or `async`)
but it's lost in other that serialize the payload to JSON (like `resque`
or `sidekiq`).

The resulting effect is that the broadcast action works fine in
development, but then fails when it's deployed to a production system.

This problem seems to have tripped a few people:

hotwired#284

Treating html: the same as content: solves the issue.

assert_broadcast_on "stream", turbo_stream_action_tag("prepend", targets: ".message", template: "<span>test</span>") do
Turbo::StreamsChannel.broadcast_action_to "stream", action: "prepend", targets: ".message", html: "<span>test</span>"
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion fails without the change to broadcast_action_to.

@afcapel afcapel changed the title Give html: rendering param the same treatment as content: Prevent html: escaping in broadcast later actions Jul 19, 2022
@dhh dhh merged commit 5040246 into hotwired:main Jul 19, 2022
dhh added a commit that referenced this pull request Aug 3, 2022
* main:
  Improve upon test suite flakiness (#327)
  Give html: rendering param the same treatment as content: (#362)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants