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

[WebProfilerBundle] [VarDumper] Inject "unsafe-eval" into the CSP if the VarDumper was used #29155

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@monojp

monojp commented Nov 9, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #29084
License MIT
Doc PR -

Adapts the VarDumper so that it accepts a callable which is called after something has been dumped. This can be used by the ContentSecurityPolicyHandler in the WebProfilerBundle to modify CSPs to allow unsafe-eval which is needed for the profiler toolbar to evaluate injected JS.
This is kind of a small feature/extension to #18568. I ommited the changelog and doc changes because of that but they can be added of course if needed.

@monojp

This comment has been minimized.

monojp commented Nov 9, 2018

Travis seems to be failing because it is not using my modified VarDumper. Should this be separated?

@nicolas-grekas nicolas-grekas added this to the next milestone Nov 9, 2018

@monojp

This comment has been minimized.

monojp commented Dec 3, 2018

Hello, what do you think of this change? @nicolas-grekas maybe? :)

@monojp monojp force-pushed the monojp:feature_29084 branch 2 times, most recently from f6e78f3 to b9f5b37 Dec 3, 2018

@stof

This comment has been minimized.

Member

stof commented Dec 4, 2018

The static property which is never reset will not play well with cases like php-pm, which reuse the same process for multiple HTTP requests (dumping once will authorize unsafe-eval for all subsequent requests in that case with this PR)

@monojp

This comment has been minimized.

monojp commented Dec 4, 2018

Interesting, I have apparently not thought about that. I am not familiar with php-pm, do you have any suggestions to avoid that problem or when to clear the property?

@monojp monojp force-pushed the monojp:feature_29084 branch from b9f5b37 to c1d1010 Dec 4, 2018

@monojp

This comment has been minimized.

monojp commented Dec 4, 2018

I have pushed a new solution which seems cleaner to me and I guess it should handle the php-pm problems better. Basically I extended the VarDumper so that it can call functions after dumping and use it to enable unsafe-eval in the CSP. This is set up in a request listener in WebDebugToolbarListener. I'm waiting for your input :)

@monojp monojp force-pushed the monojp:feature_29084 branch from c1d1010 to d4bf852 Dec 6, 2018

@monojp monojp force-pushed the monojp:feature_29084 branch from d4bf852 to 3d17963 Dec 6, 2018

@monojp

This comment has been minimized.

monojp commented Dec 6, 2018

Adapted a few small things and added more CSP edge cases. @stof What do you think?

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