Don't HTML escape ActionMailer plaintext templates #6943

Closed
wants to merge 1 commit into
from
@@ -79,6 +79,7 @@ def call(template)
self.class.erb_implementation.new(
erb,
+ :escape => template.identifier !~ /\.html/, # only escape HTML templates
@josevalim
josevalim Jul 3, 2012 Ruby on Rails member

I think we should explicitly check for text instead of everything that is not an html. We should probably do it via a configuration option too. So people have a better control.

@mguterl
mguterl Jul 3, 2012

Thanks for the feedback @josevalim. Are you thinking we should provide an option of which formats that should be escaped or a list of those which should not be escaped?

Do you have any preference for what the configuration option should be called?

@steveklabnik
steveklabnik Jul 3, 2012 Ruby on Rails member

@josevalim when I was working on this, I had the same feeling, then I remembered that... you'd only want to HTML escape an HTML template. So I think this is actually better than only not escaping text.

:trim => (self.class.erb_trim_mode == "-")
).src
end
@@ -64,6 +64,18 @@ def test_basic_template
assert_equal "Hello", render
end
+ def test_basic_template_does_not_html_escape
+ @template = new_template("<% array.each do |item| -%><%= item %><% end -%>")
+ @template.locals = [:array]
+ assert_equal '"', render(:array => ['"'])
+ end
+
+ def test_basic_html_template_does_html_escape
+ @template = ActionView::Template.new("<% array.each do |item| -%><%= item %><% end -%>", ".html.erb", ERBHandler, {})
+ @template.locals = [:array]
+ assert_equal '&quot;', render(:array => ['"'])
+ end
+
def test_raw_template
@template = new_template("<%= hello %>", :handler => ActionView::Template::Handlers::Raw.new)
assert_equal "<%= hello %>", render