-
Notifications
You must be signed in to change notification settings - Fork 183
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
MPP-3835: Workaround next/image's inline style for SVG images #4820
Conversation
When converting to django-csp 4.x, the 'unsafe-inline' rule was applied to script-src instead of style-src.
Image from next/image adds an inline style="color:transparent" to many SVG images. This causes CSP violations in production, and the style is ignored. This PR replaces Image with a local version that turns the inline style into a className with the color:transparent style. It only handles this variant, which was tested to be the only inline style emitted by next.js.
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.
Thanks for the changes!
Not required before merging but I get this warning when running a production build,
⚠ For production Image Optimization with Next.js, the optional 'sharp' package is strongly recommended. Run 'npm i sharp', and Next.js will use it automatically for Image Optimization. Read more: https://nextjs.org/docs/messages/sharp-missing-in-production
Wondering if we should install it or try and remove the warning, as we're not using Image Optimization.
@@ -0,0 +1,3 @@ | |||
.transparent { | |||
color: transparent; |
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.
👍
import { ComponentProps } from "react"; | ||
import styles from "./Image.module.scss"; | ||
|
||
export default function Image(props: ComponentProps<typeof NextImage>) { |
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.
LGTM, tested it out and the build outputs html <img>'s with no inline styles.
// Work around next.js adding inline style color:transparent | ||
// This breaks our CSP rules about inline styles. | ||
// More context and some suggested solutions from: | ||
// https://0xdbe.github.io/NextJS-CSP-NextImage/ |
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.
Was a good read, learned some more about CSP and the Image issue with Next.js 👍
I read https://nextjs.org/docs/messages/sharp-missing-in-production, but I don't see a way to remove the warning, or an existing issue. This will need some more research. It may be worth installing |
The output is identical with |
Next.js's
Image
fromnext/image
adds an inline stylestyle="color:transparent"
to some images, such as the logo image:This is a known issue, with a good explanation by an issue writer at https://0xdbe.github.io/NextJS-CSP-NextImage/ .
Since we do not use image optimization, we can replace this with an
<img />
element. This PR extracts the alt text and image parameters from the<Image>
declaration, turns an inlinestyle="color:transparent"
into aclassName
with an equivalent CSS rule, and returns the new<img />
element.How to test:
To see the current behavior
In
main
or PR #4819, runcd frontend; npm run build
. Look for<img
orstyle="color:transparent"
in that document. If desired, make a copy for diff later.In Firefox, open the developer tools, console, and load / reload https://relay.firefox.com/. See many Content-Security-Policy violations.
To see the new behavior
In this branch, run
cd frontend; npm run build
. The string "color:transparent" should not be in that document. Some of the<img
tags will have a newclassName
entry, likeImage_transparent__sZTo9
. A visual diff with the old version should show that thealt=
tag is still present, and thedata-nimg="1"
element is dropped.