Skip to content

Commit

Permalink
Fix ActionText::ContentHelper allowed tags and attrs
Browse files Browse the repository at this point in the history
which were being set to the HTML4 defaults before the sanitizer
configuration could be applied.

Also, backfill some light tests for sanitization.

Related to rails#48644
  • Loading branch information
flavorjones committed Jul 17, 2023
1 parent 0164f1f commit e8137c5
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 3 deletions.
6 changes: 6 additions & 0 deletions actiontext/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
fall back to `Rails::HTML4::Sanitizer`. Previous configurations default to
`Rails::HTML4::Sanitizer`.

As a result of this change, the defaults for `ActionText::ContentHelper.allowed_tags` and
`.allowed_attributes` are applied at runtime, so the value of these attributes is now 'nil'
unless set by the application. You may call `sanitizer_allowed_tags` or
`sanitizer_allowed_attributes` to inspect the tags and attributes being allowed by the
sanitizer.

*Mike Dalessio*

* Attachables now can override default attachment missing template.
Expand Down
19 changes: 16 additions & 3 deletions actiontext/app/helpers/action_text/content_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
module ActionText
module ContentHelper
mattr_accessor(:sanitizer, default: Rails::HTML4::Sanitizer.safe_list_sanitizer.new)
mattr_accessor(:allowed_tags) { sanitizer.class.allowed_tags + [ ActionText::Attachment.tag_name, "figure", "figcaption" ] }
mattr_accessor(:allowed_attributes) { sanitizer.class.allowed_attributes + ActionText::Attachment::ATTRIBUTES }
mattr_accessor(:allowed_tags)
mattr_accessor(:allowed_attributes)
mattr_accessor(:scrubber)

def render_action_text_content(content)
Expand All @@ -15,7 +15,12 @@ def render_action_text_content(content)
end

def sanitize_action_text_content(content)
sanitizer.sanitize(content.to_html, tags: allowed_tags, attributes: allowed_attributes, scrubber: scrubber).html_safe
sanitizer.sanitize(
content.to_html,
tags: sanitizer_allowed_tags,
attributes: sanitizer_allowed_attributes,
scrubber: scrubber,
).html_safe
end

def render_action_text_attachments(content)
Expand Down Expand Up @@ -48,5 +53,13 @@ def render_action_text_attachment(attachment, locals: {}) # :nodoc:

render(**options).chomp
end

def sanitizer_allowed_tags
allowed_tags || (sanitizer.class.allowed_tags + [ ActionText::Attachment.tag_name, "figure", "figcaption" ])
end

def sanitizer_allowed_attributes
allowed_attributes || (sanitizer.class.allowed_attributes + ActionText::Attachment::ATTRIBUTES)
end
end
end
26 changes: 26 additions & 0 deletions actiontext/test/unit/content_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,32 @@ class ActionText::ContentTest < ActiveSupport::TestCase
assert_not defined?(::ApplicationController)
end

test "does basic sanitization" do
html = "<div onclick='action()'>safe<script>unsafe</script></div>"
rendered = content_from_html(html).to_rendered_html_with_layout

assert_not_includes rendered, "<script>"
assert_not_includes rendered, "action"
end

test "does custom tag sanitization" do
old_tags = ActionText::ContentHelper.allowed_tags
old_attrs = ActionText::ContentHelper.allowed_attributes
ActionText::ContentHelper.allowed_tags = ["div"] # not 'span'
ActionText::ContentHelper.allowed_attributes = ["size"] # not 'class'

html = "<div size='large' class='high'>safe<span>unsafe</span></div>"
rendered = content_from_html(html).to_rendered_html_with_layout

assert_includes rendered, "<div"
assert_not_includes rendered, "<span"
assert_includes rendered, "large"
assert_not_includes rendered, "high"
ensure
ActionText::ContentHelper.allowed_tags = old_tags
ActionText::ContentHelper.allowed_attributes = old_attrs
end

test "renders with layout when in a new thread" do
html = "<h1>Hello world</h1>"
rendered = nil
Expand Down

0 comments on commit e8137c5

Please sign in to comment.