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.

Sign up for GitHub

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

Jump to bottom

Skip generating empty CSP header when no policy is configured #32045

Merged
merged 1 commit into from Feb 18, 2018
Merged

Skip generating empty CSP header when no policy is configured #32045

merged 1 commit into from Feb 18, 2018

Conversation

eagletmt
Copy link
Contributor

Summary

Rails.application.config.content_security_policy is configured with no
policies by default. In this case, Content-Security-Policy header should
not be generated instead of generating the header with no directives.

Other Information

Firefox also warns "Content Security Policy: Couldn't process unknown
directive ''".

@rails-bot
Copy link

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @eileencodes (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review.

Please see the contribution instructions for more information.

@eagletmt
Skip generating empty CSP header when no policy is configured
53d863d 
`Rails.application.config.content_security_policy` is configured with no
policies by default. In this case, Content-Security-Policy header should
not be generated instead of generating the header with no directives.
Firefox also warns "Content Security Policy: Couldn't process unknown
directive ''".
@guilleiguaran guilleiguaran merged commit 86f7c26 into rails:master Feb 18, 2018
@eagletmt eagletmt deleted the skip-csp-header branch February 19, 2018 03:36
@eagletmt
Copy link
Contributor Author

Thank you for reviewing and merging! Will it be backported to 5-2-stable?

@guilleiguaran
Copy link
Member

@eagletmt of course!!! I've backported it in b88292b

pixeltrix added a commit that referenced this pull request Feb 19, 2018
@pixeltrix
Revert "Merge pull request #32045 from eagletmt/skip-csp-header"
52a1f1c 
This reverts commit 86f7c26, reversing
changes made to 5ece2e4.

If a policy is set then we should generate it even if it's empty.
However what is happening is that we're accidentally generating an
empty policy when the initializer is commented out by default.
@pixeltrix pixeltrix mentioned this pull request Feb 19, 2018
Merged
@ScottHelme
Copy link

The error message that Firefox is giving isn't very helpful and also not present in Chrome. I've bugged Firefox as it seems they're not gracefully handling a CSP header with ";" as the value.

https://bugzilla.mozilla.org/show_bug.cgi?id=1439425

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guilleiguaran guilleiguaran guilleiguaran approved these changes

Assignees

@eileencodes eileencodes

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@eagletmt @rails-bot @guilleiguaran @ScottHelme @eileencodes