fix: handle absolute rewrite URLs #1325
Conversation
✅ 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 netlify-plugin-nextjs-static-root-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-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-rsc-demo canceled.
|
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
❌ Deploy Preview for netlify-plugin-nextjs-next-auth-demo failed.
|
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
| @@ -5,6 +5,19 @@ export interface FetchEventResult { | |||
| waitUntil: Promise<any> | |||
| } | |||
|
|
|||
| /** | |||
| * This is how Next handles rewritten URLs. | |||
There was a problem hiding this comment.
[dust] Might be handy to include the same link that was provided in the pull request summary here so it makes it easier to folks to find the relevant code that's being referred to here
| /** | ||
| * This is how Next handles rewritten URLs. | ||
| */ | ||
| export function relativizeURL(url: string | string, base: string | URL) { |
There was a problem hiding this comment.
[boulder] Is it a mistake that the url parameter's type is string | string or was it meant to be string | URL?
| export function relativizeURL(url: string | string, base: string | URL) { | |
| export function relativizeURL(url: string | URL, base: string | URL) { |
There was a problem hiding this comment.
Oh, missed this before adding automerge! That's copied directly from the Next.js source and certainly looks like a mistake!
Summary
When rewriting a URL in middleware, it attaches an
x-middleware-rewriteheader with the target URL. This is used both for server-side rewrites, but also for client-side preflights. The returned value can be either a relative or absolute URL. Previously we passed-through the header unchanged. This caused problems for two reasons. First, if it is an absolute URL it is treated as external by the router on the client, even if it is not, which caused Next to get into an infinite reload loop as it tried to redirect to itself. For actual external URLs it was failing because Netlify Edge Functions don't support rewriting to external URLs.This fixes these problems in two parts:
context.rewrite()it instead callsfetch(), adds the appropriate headers to the response, and returns the response.Test plan
x-middleware-rewriteheaders is relativex-middleware-rewriteheader is "http://example.com".Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Fixes issue mentioned in this comment
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.