Skip to content

Commit

Permalink
token_list: Guard Stimulus' data-action from multiple escapes
Browse files Browse the repository at this point in the history
Prior to this commit, chaining more than one `token_list` calls with a
[data-action][] attribute value would result in one too many HTML
escapes. Additional subsequent calls would compound the problem.

For example, the following calls would result in an invalid descriptor
that's escaped too many times to be parsed.

```ruby
first   = "click->controller#action1"
second  = "click->controller#action2"
third   = "click->controller#action3"
fourth  = "click->controller#action4"

value = token_list(first, token_list(second, token_list(third)))

CGI.unescape_html value.to_s
 # => "click->controller#action1 click->controller#action2 click->controller#action3 click->controller#action4"
```

By [CGI.unescape_html][] each `String` value before passing it to
[token_list][] (which re-escapes the value), we can preserve a lossless
concatenation process while also preserving the HTML safety.

After this commit, the previous example works as expected:

```ruby
first   = "click->controller#action1"
second  = "click->controller#action2"
third   = "click->controller#action3"
fourth  = "click->controller#action4"

value = token_list(first, token_list(second, token_list(third)))

CGI.unescape_html value.to_s
 # => "click->controller#action1 click->controller#action2 click->controller#action3 click->controller#action4"
```

[unescaping]: https://ruby-doc.org/stdlib-2.5.3/libdoc/cgi/rdoc/CGI/Util.html#method-i-unescape_html
[token_list]:
https://edgeapi.rubyonrails.org/classes/ActionView/Helpers/TagHelper.html#method-i-token_list
[data-action]: https://stimulus.hotwired.dev/reference/actions
  • Loading branch information
seanpdoyle committed Feb 8, 2023
1 parent d77c3fa commit 812e50b
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 1 deletion.
4 changes: 4 additions & 0 deletions actionview/CHANGELOG.md
@@ -1,3 +1,7 @@
* Guard `token_list` calls from escaping HTML too often

*Sean Doyle*

* `select` can now be called with a single hash containing options and some HTML options

Previously this would not work as expected:
Expand Down
2 changes: 1 addition & 1 deletion actionview/lib/action_view/helpers/tag_helper.rb
Expand Up @@ -363,7 +363,7 @@ def content_tag(name, content_or_options_with_block = nil, options = nil, escape
# token_list(nil, false, 123, "", "foo", { bar: true })
# # => "123 foo bar"
def token_list(*args)
tokens = build_tag_values(*args).flat_map { |value| value.to_s.split(/\s+/) }.uniq
tokens = build_tag_values(*args).flat_map { |value| CGI.unescape_html(value.to_s).split(/\s+/) }.uniq

safe_join(tokens, " ")
end
Expand Down
23 changes: 23 additions & 0 deletions actionview/test/template/tag_helper_test.rb
Expand Up @@ -448,6 +448,29 @@ def test_token_list_and_class_names
end
end

def test_token_list_and_class_names_returns_an_html_safe_string
assert_predicate token_list("a value"), :html_safe?
assert_predicate class_names("a value"), :html_safe?
end

def test_token_list_and_class_names_only_html_escape_once
[:token_list, :class_names].each do |helper_method|
helper = ->(*arguments) { public_send(helper_method, *arguments) }

tokens = %w[
click->controller#action1
click->controller#action2
click->controller#action3
click->controller#action4
]

token_list = tokens.reduce(&helper)

assert_predicate token_list, :html_safe?
assert_equal tokens, (CGI.unescape_html(token_list)).split
end
end

def test_content_tag_with_data_attributes
assert_dom_equal '<p data-number="1" data-string="hello" data-string-with-quotes="double&quot;quote&quot;party&quot;">limelight</p>',
content_tag("p", "limelight", data: { number: 1, string: "hello", string_with_quotes: 'double"quote"party"' })
Expand Down

0 comments on commit 812e50b

Please sign in to comment.