Uh oh!
There was an error while loading. Please reload this page.
Avoid calling content_security_policy_nonce internally - #389
Conversation
Rails 5.2 adds support for configuring a Content-Security-Policy header, including adding nonces to tags produced by the `javascript_tag` helper. Unfortunately, Rails and this gem now both define a helper named `content_security_policy_nonce`: https://github.com/rails/rails/blob/v5.2.0.rc2/actionpack/lib/action_controller/metal/content_security_policy.rb#L44https://github.com/twitter/secureheaders/blob/v5.0.5/lib/secure_headers/view_helper.rb#L69 The Rails helper wins over the Secure Headers one, and helpers like `nonced_javascript_tag` currently raise this error on Rails 5.2: ArgumentError: wrong number of arguments (given 1, expected 0) By using a method with a different name internally, we avoid clashing with the Rails implementation, and `nonced_javascript_tag` works again.
jacobbednarz
left a comment
There was a problem hiding this comment.
Nice find! 🏆
I'm not usually a fan of the underscore prefixed methods but it seems reasonable here.
eugeneius
commented
Mar 26, 2018
I have no strong opinion on the particular method name, and will happily change it to anything else. 🙂 |
oreoshake
commented
Mar 26, 2018
This is going to break things 😢. While people should use the tag helpers, I'm sure someone is using the raw value for something. Luckily, 6.0 hasn't been released so we can sneak this in there. It was meant to be a private API to begin with. The name collision is unfortunate but it's a polluted namespace. |
oreoshake
commented
Mar 26, 2018
Released in v6.0.0.alpha02. |
eugeneius
commented
Mar 26, 2018
Thanks @oreoshake! I think you may have missed this line though: |
paulfri
commented
Mar 26, 2018
@eugeneius We were using |
eugeneius
commented
Mar 26, 2018
I'm not saying that Rails didn't break Secure Headers; I'm saying that this patch makes Secure Headers mostly compatible with Rails 5.2, without affecting its compatibility with earlier versions. As such I think it's not a breaking change, and could be backported to 5.0.x so that there's a non-prerelease version available that (mostly) works with Rails 5.2.0 when it's out in a couple of weeks. |
paulfri
commented
Mar 26, 2018
It may be backwards-compatible with Rails (in that |
eugeneius
commented
Mar 27, 2018
|
paulfri
commented
Mar 27, 2018
I completely missed that, sorry! We already updated to use the tag helpers (and 6.0.0.alpha02) but it looks like it's indeed backwards-compatible and could be backported to 5.x. |
Rails 5.2 adds support for configuring a Content-Security-Policy header, including adding nonces to tags produced by the
javascript_taghelper.Unfortunately, Rails and this gem now both define a helper named
content_security_policy_nonce:https://github.com/rails/rails/blob/v5.2.0.rc2/actionpack/lib/action_controller/metal/content_security_policy.rb#L44
https://github.com/twitter/secureheaders/blob/v5.0.5/lib/secure_headers/view_helper.rb#L69
The Rails helper wins over the Secure Headers one, and helpers like
nonced_javascript_tagcurrently raise this error on Rails 5.2:By using a method with a different name internally, we avoid clashing with the Rails implementation, and
nonced_javascript_tagworks again.