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

[WebProfiler] Remove 'none' when appending CSP tokens #36786

Merged
merged 1 commit into from May 13, 2020

Conversation

ndench
Copy link
Contributor

@ndench ndench commented May 12, 2020

Q A
Branch? 3.4, 4.4, 5.0
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #36645
License MIT
Doc PR n/a

@nicolas-grekas asked me to to have a look at this after #36678.

If a user has a CSP policy of default-src 'none', then the WebProfiler copies 'none' to script-src and style-src then adds other sources. This creates an invalid policy since 'none' is only allowed when it's the only item in the source list.

This will probably need to be merged into 3.4 first, I started on 4.4 so I can test in my current symfony project which requires 4.4.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented May 12, 2020

Thank you @ndench
Is this ready for 4.4? Do your local tests work as expected?

@ndench
Copy link
Contributor Author

ndench commented May 12, 2020

Yep, seems to work as expected for me. I'm unsure why aappveyor is failing though.

The only issue I can see is if someone specifies default-src: 'none' <some other source> at which point we won't remove the 'none'. However, that means their CSP was invalid before the WebProfiler touches it, so I don't think that's a problem.

@fabpot
Copy link
Member

fabpot commented May 13, 2020

Thank you @ndench.

@fabpot fabpot merged commit 3150104 into symfony:4.4 May 13, 2020
@fabpot fabpot mentioned this pull request May 16, 2020
This was referenced May 31, 2020
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

4 participants