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

Improve AV changelog with regards to default RAW handler #24929

Merged
merged 1 commit into from
May 9, 2016

Conversation

rosenfeld
Copy link
Contributor

It highlights which kind of incompatibilities this could lead to when rendering plain JS or HTML partials.

cc/ @rafaelfranca

@rails-bot
Copy link

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

@rosenfeld
Copy link
Contributor Author

r? @rafaelfranca

handler instead of ERB. The raw handler does not flag the rendered text as html safe,
so if your application rendered plain JS or HTML files before, you'll have to replace:

<%= render '/common/analytics.js' %>
Copy link
Member

Choose a reason for hiding this comment

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

<%= render '/common/analytics.js' %>, needs code quotes, same below.

It highlights which kind of incompatibilities this could lead to when rendering plain JS or HTML partials.
@rosenfeld
Copy link
Contributor Author

I've updated the patch, @vipulnsward. Thanks

@spastorino spastorino merged commit f338bb5 into rails:master May 9, 2016
@rafaelfranca
Copy link
Member

Thank you so much for making this clear. About making raw templates html safe by default at first it seems fine but I'm worried it could introduce security problems but I don't see which problem could be caused by marking it as html safe that would not be caused by recommending people to call html_safe in the render call.

I think this needs better discussion with the security team. I'll open an issue to discuss.

@rafaelfranca
Copy link
Member

#24949 open

rafaelfranca pushed a commit to rafaelfranca/omg-rails that referenced this pull request May 10, 2016
Improve AV changelog with regards to default RAW handler
rafaelfranca pushed a commit that referenced this pull request May 10, 2016
Improve AV changelog with regards to default RAW handler
@rosenfeld
Copy link
Contributor Author

Thanks, I'll add some comments there

eileencodes added a commit that referenced this pull request Jun 21, 2016
In PR #24929 the changelog was updated to make note that while the new
template handler was changed to raw this changed the behavior when
outputting plain html or js files. Previously ERB would output the files
unescaped. Changing the default handler to RAW meant that these same
files would be rendered as escaped rather than as js or html.

Because of this change in behavior and after the discussion #24949 in we
decided to change the behavior of the Raw handler to output html_safe
strings by default.

Now files rendered with the default handler (raw) render the file
unescaped.
eileencodes added a commit that referenced this pull request Jun 21, 2016
In PR #24929 the changelog was updated to make note that while the new
template handler was changed to raw this changed the behavior when
outputting plain html or js files. Previously ERB would output the files
unescaped. Changing the default handler to RAW meant that these same
files would be rendered as escaped rather than as js or html.

Because of this change in behavior and after the discussion #24949 in we
decided to change the behavior of the Raw handler to output html_safe
strings by default.

Now files rendered with the default handler (raw) render the file
unescaped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants