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

Fix generation of empty content security policy #32054

Merged
merged 3 commits into from
Feb 19, 2018

Conversation

pixeltrix
Copy link
Contributor

This reverts the changes in #32045 and fixes the issue with accidentally creating an empty policy when setting up the request. The decision to generate an empty policy if it's set was an explicit one to show the difference between not having a policy set and an empty policy which just uses the browser defaults. It also removes the trailing semi-colon so that an empty policy doesn't generate a console warning.

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.
Setting up the request environment was accidentally creating a CSP
as a consequence of accessing the option - only set the instance
variable if a block is passed.
Although the spec[1] is defined in such a way that a trailing semi-colon
is valid it also doesn't allow a semi-colon by itself to indicate an
empty policy. Therefore it's easier (and valid) just to omit it rather
than to detect whether the policy is empty or not.

[1]: https://www.w3.org/TR/CSP2/#policy-syntax
@pixeltrix pixeltrix merged commit dc6185b into master Feb 19, 2018
@pixeltrix pixeltrix deleted the fix-generation-of-empty-csp branch February 19, 2018 14:55
pixeltrix added a commit that referenced this pull request Feb 19, 2018
Fix generation of empty content security policy

(cherry picked from commit dc6185b)
@pixeltrix
Copy link
Contributor Author

Backported to 5-2-stable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants