fix: preserve set-cookie headers during SSR redirects#1744
Conversation
Adopt upstream Nuxt's `ssrContext['~renderResponse']` pattern to defer redirect responses, ensuring cookies set via useCookie are included in redirect response headers.
commit: |
📝 WalkthroughWalkthroughThe PR modifies the SSR redirect mechanism to preserve cookies set via Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bridge/src/runtime/composables/router.ts`:
- Around line 154-167: The meta-refresh body uses encodedLoc which only replaces
quotes and can allow HTML injection (in function redirect); escape the location
for HTML context before interpolating into the meta tag: add or reuse an
HTML-escape utility (e.g., escapeHtml) and apply it to the location (or
encodedLoc) when building the body string assigned to
nuxtApp.ssrContext!['~renderResponse'].body, while keeping the existing
encodeURL usage for the Location header (encodedHeader) and preserving
sanitizeStatusCode for the statusCode.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/bridge/src/runtime/composables/router.tspackages/bridge/src/runtime/nitro/renderer.tsplayground/pages/cookie-with-redirect.vuetest/bridge.test.ts
| const isExternalHost = hasProtocol(toPath, { acceptRelative: true }) | ||
|
|
||
| const redirect = async function (response: any) { | ||
| // @ts-expect-error | ||
| await nuxtApp.callHook('app:redirected') | ||
|
|
||
| await sendRedirect(nuxtApp.ssrContext!.event, location, options?.redirectCode || 302) | ||
| const encodedLoc = location.replace(URL_QUOTE_RE, '%22') | ||
| const encodedHeader = encodeURL(location, isExternalHost) | ||
|
|
||
| nuxtApp.ssrContext!['~renderResponse'] = { | ||
| statusCode: sanitizeStatusCode(options?.redirectCode || 302, 302), | ||
| body: `<!DOCTYPE html><html><head><meta http-equiv="refresh" content="0; url=${encodedLoc}"></head></html>`, | ||
| headers: { location: encodedHeader } | ||
| } |
There was a problem hiding this comment.
Escape the meta‑refresh URL to prevent HTML injection.
Only quotes are escaped today, so a location containing " (or </>) can break the attribute and inject markup. Please HTML‑escape the value used in the meta refresh body.
🛡️ Suggested fix
- const encodedLoc = location.replace(URL_QUOTE_RE, '%22')
- const encodedHeader = encodeURL(location, isExternalHost)
+ const encodedHeader = encodeURL(location, isExternalHost)
+ const encodedLoc = encodedHeader
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"')
nuxtApp.ssrContext!['~renderResponse'] = {
statusCode: sanitizeStatusCode(options?.redirectCode || 302, 302),
body: `<!DOCTYPE html><html><head><meta http-equiv="refresh" content="0; url=${encodedLoc}"></head></html>`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isExternalHost = hasProtocol(toPath, { acceptRelative: true }) | |
| const redirect = async function (response: any) { | |
| // @ts-expect-error | |
| await nuxtApp.callHook('app:redirected') | |
| await sendRedirect(nuxtApp.ssrContext!.event, location, options?.redirectCode || 302) | |
| const encodedLoc = location.replace(URL_QUOTE_RE, '%22') | |
| const encodedHeader = encodeURL(location, isExternalHost) | |
| nuxtApp.ssrContext!['~renderResponse'] = { | |
| statusCode: sanitizeStatusCode(options?.redirectCode || 302, 302), | |
| body: `<!DOCTYPE html><html><head><meta http-equiv="refresh" content="0; url=${encodedLoc}"></head></html>`, | |
| headers: { location: encodedHeader } | |
| } | |
| const isExternalHost = hasProtocol(toPath, { acceptRelative: true }) | |
| const redirect = async function (response: any) { | |
| // `@ts-expect-error` | |
| await nuxtApp.callHook('app:redirected') | |
| const encodedHeader = encodeURL(location, isExternalHost) | |
| const encodedLoc = encodedHeader | |
| .replace(/&/g, '&') | |
| .replace(/</g, '<') | |
| .replace(/>/g, '>') | |
| .replace(/"/g, '"') | |
| nuxtApp.ssrContext!['~renderResponse'] = { | |
| statusCode: sanitizeStatusCode(options?.redirectCode || 302, 302), | |
| body: `<!DOCTYPE html><html><head><meta http-equiv="refresh" content="0; url=${encodedLoc}"></head></html>`, | |
| headers: { location: encodedHeader } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bridge/src/runtime/composables/router.ts` around lines 154 - 167,
The meta-refresh body uses encodedLoc which only replaces quotes and can allow
HTML injection (in function redirect); escape the location for HTML context
before interpolating into the meta tag: add or reuse an HTML-escape utility
(e.g., escapeHtml) and apply it to the location (or encodedLoc) when building
the body string assigned to nuxtApp.ssrContext!['~renderResponse'].body, while
keeping the existing encodeURL usage for the Location header (encodedHeader) and
preserving sanitizeStatusCode for the statusCode.
|
I'm planning to release 3.6.2, including this fix. (We were going to release v4, but it's been delayed.) |
1 participant
🔗 Linked issue
Fixes: #1737
❓ Type of change
📚 Description
When
useCookieis called beforenavigateToin a middleware during SSR, theset-cookieheader was not included in the redirect response.navigateTocalledsendRedirect()immediately, which sent the HTTP response before theapp:renderedhook (whereuseCookiewrites cookies) could fire.Changed to use
ssrContext['~renderResponse']to defer the redirect response, the same approach as upstream Nuxt:navigateTonow setsssrContext['~renderResponse']instead of callingsendRedirect()directly, deferring the response.app:renderedhook (which triggers cookie writing) before checking for~renderResponse.set-cookie.📝 Checklist