-
Notifications
You must be signed in to change notification settings - Fork 89
fix: validate next/image params #1661
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
Conversation
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-hp-edge-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-rsc-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-canary ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
67257af
to
f7acb8a
Compare
f7acb8a
to
7ea4282
Compare
@ascorbic This was a breaking change for me. Needed to do very small update for a old client project and all images broke. |
@Plucks77 Sorry to hear that! It certainly wasn't meant to be a breaking change. Could you give an example of a broken image URL? |
@@ -10,5 +10,6 @@ export const handler: Handler = createIPXHandler({ | |||
domains, | |||
remotePatterns, | |||
responseHeaders, | |||
localPrefix: '/_next/static/media/', |
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.
We are noticing a breaking change as well. Is it possible that this is expecting images to be inside a media
folder?
Summary
This PR includes two fixes to validate next/image prarams. First, the ipx edge function ensures that the passed props are valid, returning a 400 error if not. Second, it upgrades
@netlify/ipx
and passes the newlocalPrefix
option, to enforce local image sources that start with/_next/static/
.Draft because it's waiting to release netlify/netlify-ipx#83
Test plan
/image
in the default site preview. Open the first image and modify any of the params to be non-numeric, and check for the error messageRelevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.