Remove a bit of inline CSS. Add CSP nonce where it might be required and is available. by juspence · Pull Request #8783 · encode/django-rest-framework (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation7 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

(Copied from #7960, but nonce removed to avoid conflicts with user-defined policies)

Remove a few instances of inline CSS which could trigger Content Security Policies (CSPs) and replace with classes where required.

Part of #6069.

I've left JavaScript alone as it's covered by #5740 and #7016 (which I think are duplicates of each other?).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clear: both was already in request-info.

@auvipy This is the same as #7960, but with the CSP nonce bit removed to avoid problems.

I will leave this open a few days. Please let me know if you'd like to review again or if I should merge.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you mind sharing the screenshots please after this changes?

@auvipy Sorry for the delayed response. Here's before:

And after:

@juspence @auvipy
In #7960 there was support for nonce.

<style{% if request.csp_nonce %} nonce="{{request.csp_nonce}}"{% endif %}>{{ code_style }}</style>

I do not understand the reason for removing this support in this PR

(Copied from #7960, but nonce removed to avoid conflicts with user-defined policies)

If there is no request.csp_nonce there is no conflict because it does not add nonce attr. But if the project follows the strictest CSP with nonce BrowserableAPI is still broken.
Also, the PR and release notes include

Add CSP nonce where it might be required and is available.

Which is a lie.

This PR uses a bootstrap class for floats (pull-left) but the clear:both is included in bootstrap-tweaks.css.

This means if the user overrides the default theme as documented, the clear is gone, and all content boxes get stacked horizontally.

Solution: Description and paginator should be wrapped in a <div class="clearfix">.

Screenshot of the bug using "Flatly" Bootstrap theme:

Would you mind sending a PR to fix this?