-
Notifications
You must be signed in to change notification settings - Fork 86
fix: cookies set in middleware accessible during the same request #2847
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
📊 Package size report 0.09%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
f73ce83
to
ab19472
Compare
ab19472
to
c8379d6
Compare
93c8766
to
45fb6dd
Compare
45fb6dd
to
bc25218
Compare
edge-runtime/lib/response.ts
Outdated
request.headers.set('x-middleware-rewrite', target) | ||
return addMiddlewareHeaders(context.rewrite(target), res) | ||
return addMiddlewareHeaders(context.rewrite(target), edgeResponse) |
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.
I'm not sure, but possibly here we would also need to handle middleware cookies as well? next.js test fixture doesn't seem to have case for setting cookies for rewrites, so maybe we should not touch it in this PR - just flagging it (so this comment act as potential paper trail in case there was some future report about cookies not being handled for rewrites)
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.
Recreated the bug quite easily with the following middleware, so decided to fix in this PR as well
const nextRes = NextResponse.rewrite(url)
nextRes.cookies.set("nextjs-cookie", "rewrite");
return nextRes;
Description
These changes make it possible to set a cookie in middleware and have that value be accessible at render time during the same request. This should also address a couple e2e tests failures we see in the report.
Part of FRB-1387
Fixes FRB-1748
Fixes #2669
Tests
Used variants of the following setup: