-
Notifications
You must be signed in to change notification settings - Fork 601
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
Eliminate the use of inline styles, making it compatible with strict CSP for style. #634
Conversation
Hey there - and thanks for this! The approach though will cause a redraw which we wouldn't want especially for a heavy page, is there any other way around the policy you're setting? I think if doing this, we'd want to make it opt-in for the performance impact if possible - thoughts? |
Unfortunately due to the inline styles being dynamically generated by concatenating pixel widths, it is impossible to workaround using even unsafe-hashes. The big issue I see actually is that the script relies on HTML string concatenation instead of manipulating the dom directly and adding elements node by node. I don't know another way, but I was pretty much forced to do it like this. The nature of applications I am working on prohibit the allow of inline script and style completely. A second possible workaround would be the use of the attr CSS function to set the value of a CSS variable from an attribute, and then using the CSS file set the padding and margin using this variable, but if I remember correctly, this is not well supported. https://caniuse.com/?search=attr While it maybe possible to somehow enable inline style only when MiniProfiler is running, this will create a situation where the developer is running in a different permission environment than the actual live system which will lead to slipping of inline styles and introduction of bugs, and thus is not a viable (productive) workaround. The redraws might be possible to eliminate if the miniprofiler HTML is set on a non-rendered element first, then the styles are manipulated, and then finally inserted into the actual visible container, do you think that would solve the redraw problem ? |
@NickCraver would this latest change address the redraw issue or mitigate it ? |
I have tested this on many heavy pages, and with the mitigation I have applied (using document.createElement to created a detached element, manipulate that then insert the nodes directly (not as html) to their final parent) I don't have any redraw issues. |
@rwasef1830 Sorry I've been swamped at work, going to try and spin this up the next few days (took some time off to catch up on things) - trying to get everything updated for builds to pass here first so we have better checks running in #637. |
Miniprofiler list page also needs work, uses inline stuff. |
@NickCraver This PR should be complete now (rebased on main). I introduced a new NonceGetter property on MiniProfilerOptions that allows the consumer to set the value per request to use for the nonce. This value is also used as a fallback for the tag helper if the tag helper property is not set. The "share" full result page is now working with no CSP errors. Breaking changes: Requesting review, feedback and hopefully a merge after everything is made whole. |
Spaces > tabs :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made some additional changes to support all scenarios, thanks for all the work here! Apologies I haven't had time until vacation this week to get some OSS backlog done.
Hello, @NickCraver do you have any ETA for a new release on NuGet.org with this change ? Thanks |
…CSP for style. (MiniProfiler#634) Thank you for making this project easy to build and fork. With this PR MiniProfiler will run under a strict CSP that disallows inline styles and scripts (by using nonce) with zero errors. It accomplish this by putting dynamically generated style tag values as data attributes, and then later after appending the miniprofiler html, it queries for them and manipulates the style object on Element directly, thus eliminating the need for inline style. Co-authored-by: Nick Craver <nrcraver@gmail.com>
Dear MiniProfiler team,
Thank you for making this project easy to build and fork.
With this PR MiniProfiler will run under a strict CSP that disallows inline styles and scripts (by using nonce) with zero errors.
It accomplish this by putting dynamically generated style tag values as data attributes, and then later after appending the miniprofiler html, it queries for them and manipulates the style object on Element directly, thus eliminating the need for inline style.