diff --git a/packages/next/src/export/worker.ts b/packages/next/src/export/worker.ts index 2135cb5784e4..8e0ad41fbf14 100644 --- a/packages/next/src/export/worker.ts +++ b/packages/next/src/export/worker.ts @@ -8,6 +8,7 @@ import type { ExportPathMap, NextConfigComplete, } from '../server/config-shared' +import type { OutgoingHttpHeaders } from 'http' // `NEXT_PREBUNDLED_REACT` env var is inherited from parent process, // then override react packages here for export worker. @@ -49,6 +50,7 @@ import { NEXT_DYNAMIC_NO_SSR_CODE } from '../shared/lib/lazy-dynamic/no-ssr-erro import { mockRequest } from '../server/lib/mock-request' import { NodeNextRequest } from '../server/base-http/node' import { isAppRouteRoute } from '../lib/is-app-route-route' +import { toNodeHeaders } from '../server/web/utils' import { RouteModuleLoader } from '../server/future/helpers/module-loader/route-module-loader' loadRequireHook() @@ -98,7 +100,7 @@ interface ExportPageResults { fromBuildExportRevalidate?: number | false fromBuildExportMeta?: { status?: number - headers?: Record + headers?: OutgoingHttpHeaders } error?: boolean ssgNotFound?: boolean @@ -411,6 +413,8 @@ export default async function exportPage({ // Call the handler with the request and context from the module. const response = await module.handle(request, context) + // TODO: (wyattjoh) if cookie headers are present, should we bail? + // we don't consider error status for static generation // except for 404 // TODO: do we want to cache other status codes? @@ -423,7 +427,7 @@ export default async function exportPage({ context.staticGenerationContext.store?.revalidate || false results.fromBuildExportRevalidate = revalidate - const headers = Object.fromEntries(response.headers) + const headers = toNodeHeaders(response.headers) if (!headers['content-type'] && body.type) { headers['content-type'] = body.type diff --git a/packages/next/src/server/base-http/web.ts b/packages/next/src/server/base-http/web.ts index 1fa9df5bd852..563b79aa845b 100644 --- a/packages/next/src/server/base-http/web.ts +++ b/packages/next/src/server/base-http/web.ts @@ -1,4 +1,5 @@ import type { IncomingHttpHeaders, OutgoingHttpHeaders } from 'http' +import { toNodeHeaders } from '../web/utils' import { BaseNextRequest, BaseNextResponse } from './index' @@ -75,7 +76,7 @@ export class WebNextResponse extends BaseNextResponse { } getHeaders(): OutgoingHttpHeaders { - return Object.fromEntries(this.headers.entries()) + return toNodeHeaders(this.headers) } hasHeader(name: string): boolean { diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 14a4b1dcb586..c2bf4f2cb5e4 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -99,6 +99,7 @@ import { I18NProvider } from './future/helpers/i18n-provider' import { sendResponse } from './send-response' import { RouteKind } from './future/route-kind' import { handleInternalServerErrorResponse } from './future/route-modules/helpers/response-handlers' +import { fromNodeHeaders, toNodeHeaders } from './web/utils' export type FindComponentsResult = { components: LoadComponentsReturnType @@ -592,12 +593,15 @@ export default abstract class Server { !val.every((item, idx) => item === middlewareValue[idx]) ) { val = [ - ...(middlewareValue || []), - ...(typeof val === 'string' - ? [val] - : Array.isArray(val) - ? val - : []), + // TODO: (wyattjoh) find out why this is called multiple times resulting in duplicate cookies being added + ...new Set([ + ...(middlewareValue || []), + ...(typeof val === 'string' + ? [val] + : Array.isArray(val) + ? val + : []), + ]), ] } } @@ -1544,7 +1548,7 @@ export default abstract class Server { const blob = await response.blob() // Copy the headers from the response. - const headers = Object.fromEntries(response.headers) + const headers = toNodeHeaders(response.headers) if (!headers['content-type'] && blob.type) { headers['content-type'] = blob.type } @@ -1919,7 +1923,7 @@ export default abstract class Server { req, res, new Response(cachedData.body, { - headers: new Headers((cachedData.headers || {}) as any), + headers: fromNodeHeaders(cachedData.headers), status: cachedData.status || 200, }) ) diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index ded15c59e9f0..b1e480bbf9ab 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -1873,14 +1873,6 @@ export default class NextNodeServer extends BaseServer { onWarning: params.onWarning, }) - const allHeaders = new Headers() - - for (let [key, value] of result.response.headers) { - if (key !== 'x-middleware-next') { - allHeaders.append(key, value) - } - } - if (!this.renderOpts.dev) { result.waitUntil.catch((error) => { console.error(`Uncaught: middleware waitUntil errored`, error) @@ -1890,19 +1882,24 @@ export default class NextNodeServer extends BaseServer { if (!result) { this.render404(params.request, params.response, params.parsed) return { finished: true } - } else { - for (let [key, value] of allHeaders) { - result.response.headers.set(key, value) - - if (key.toLowerCase() === 'set-cookie') { - addRequestMeta( - params.request, - '_nextMiddlewareCookie', - splitCookiesString(value) - ) - } + } + + for (let [key, value] of result.response.headers) { + if (key.toLowerCase() !== 'set-cookie') continue + + // Clear existing header. + result.response.headers.delete(key) + + // Append each cookie individually. + const cookies = splitCookiesString(value) + for (const cookie of cookies) { + result.response.headers.append(key, cookie) } + + // Add cookies to request meta. + addRequestMeta(params.request, '_nextMiddlewareCookie', cookies) } + return result } @@ -2038,7 +2035,11 @@ export default class NextNodeServer extends BaseServer { continue } if (key !== 'content-encoding' && value !== undefined) { - res.setHeader(key, value) + if (typeof value === 'number') { + res.setHeader(key, value.toString()) + } else { + res.setHeader(key, value) + } } } @@ -2243,10 +2244,15 @@ export default class NextNodeServer extends BaseServer { params.res.statusCode = result.response.status params.res.statusMessage = result.response.statusText - result.response.headers.forEach((value: string, key) => { - // the append handling is special cased for `set-cookie` + // TODO: (wyattjoh) investigate improving this + + result.response.headers.forEach((value, key) => { + // The append handling is special cased for `set-cookie`. if (key.toLowerCase() === 'set-cookie') { - params.res.setHeader(key, value) + // TODO: (wyattjoh) replace with native response iteration when we can upgrade undici + for (const cookie of splitCookiesString(value)) { + params.res.appendHeader(key, cookie) + } } else { params.res.appendHeader(key, value) } diff --git a/packages/next/src/server/response-cache/types.ts b/packages/next/src/server/response-cache/types.ts index 3b81c4ba0d78..2ca7f700e955 100644 --- a/packages/next/src/server/response-cache/types.ts +++ b/packages/next/src/server/response-cache/types.ts @@ -1,3 +1,4 @@ +import type { OutgoingHttpHeaders } from 'http' import type RenderResult from '../render-result' export interface ResponseCacheBase { @@ -41,7 +42,7 @@ export interface CachedRouteValue { // expects that type instead of a string body: Buffer status: number - headers: Record + headers: OutgoingHttpHeaders } export interface CachedImageValue { diff --git a/packages/next/src/server/send-response.ts b/packages/next/src/server/send-response.ts index b5fe049f54b0..a722f74bcfb4 100644 --- a/packages/next/src/server/send-response.ts +++ b/packages/next/src/server/send-response.ts @@ -1,5 +1,6 @@ import type { BaseNextRequest, BaseNextResponse } from './base-http' import type { NodeNextResponse } from './base-http/node' +import { splitCookiesString } from './web/utils' /** * Sends the response on the underlying next response object. @@ -23,7 +24,10 @@ export async function sendResponse( response.headers?.forEach((value, name) => { // The append handling is special cased for `set-cookie`. if (name.toLowerCase() === 'set-cookie') { - res.setHeader(name, value) + // TODO: (wyattjoh) replace with native response iteration when we can upgrade undici + for (const cookie of splitCookiesString(value)) { + res.appendHeader(name, cookie) + } } else { res.appendHeader(name, value) } diff --git a/packages/next/src/server/server-route-utils.ts b/packages/next/src/server/server-route-utils.ts index 5be9abb43b64..9cef9bf532cd 100644 --- a/packages/next/src/server/server-route-utils.ts +++ b/packages/next/src/server/server-route-utils.ts @@ -92,7 +92,12 @@ export const createHeaderRoute = ({ key = compileNonPath(key, params) value = compileNonPath(value, params) } - res.setHeader(key, value) + + if (key.toLowerCase() === 'set-cookie') { + res.appendHeader(key, value) + } else { + res.setHeader(key, value) + } } return { finished: false } }, diff --git a/packages/next/src/server/web/next-url.ts b/packages/next/src/server/web/next-url.ts index d74dc1d58065..a0704a1db980 100644 --- a/packages/next/src/server/web/next-url.ts +++ b/packages/next/src/server/web/next-url.ts @@ -1,13 +1,15 @@ +import type { OutgoingHttpHeaders } from 'http' import type { DomainLocale, I18NConfig } from '../config-shared' +import type { I18NProvider } from '../future/helpers/i18n-provider' + import { detectDomainLocale } from '../../shared/lib/i18n/detect-domain-locale' import { formatNextPathnameInfo } from '../../shared/lib/router/utils/format-next-pathname-info' import { getHostname } from '../../shared/lib/get-hostname' import { getNextPathnameInfo } from '../../shared/lib/router/utils/get-next-pathname-info' -import type { I18NProvider } from '../future/helpers/i18n-provider' interface Options { base?: string | URL - headers?: { [key: string]: string | string[] | undefined } + headers?: OutgoingHttpHeaders forceLocale?: boolean nextConfig?: { basePath?: string diff --git a/packages/next/src/server/web/types.ts b/packages/next/src/server/web/types.ts index b5a6b07d2883..7e91248b18e4 100644 --- a/packages/next/src/server/web/types.ts +++ b/packages/next/src/server/web/types.ts @@ -3,10 +3,7 @@ import type { NextRequest } from './spec-extension/request' import type { NextFetchEvent } from './spec-extension/fetch-event' import type { NextResponse } from './spec-extension/response' import type { CloneableBody } from '../body-streams' - -export interface NodeHeaders { - [header: string]: string | string[] | undefined -} +import type { OutgoingHttpHeaders } from 'http' export interface RequestData { geo?: { @@ -16,7 +13,7 @@ export interface RequestData { latitude?: string longitude?: string } - headers: NodeHeaders + headers: OutgoingHttpHeaders ip?: string method: string nextConfig?: { diff --git a/packages/next/src/server/web/utils.ts b/packages/next/src/server/web/utils.ts index 9249d9704f16..a87a225d4920 100644 --- a/packages/next/src/server/web/utils.ts +++ b/packages/next/src/server/web/utils.ts @@ -1,13 +1,16 @@ -import type { NodeHeaders } from './types' +import type { OutgoingHttpHeaders } from 'http' -export function fromNodeHeaders(object: NodeHeaders): Headers { +export function fromNodeHeaders(object: OutgoingHttpHeaders): Headers { const headers = new Headers() for (let [key, value] of Object.entries(object)) { const values = Array.isArray(value) ? value : [value] for (let v of values) { - if (v !== undefined) { - headers.append(key, v) + if (typeof v === 'undefined') continue + if (typeof v === 'number') { + v = v.toString() } + + headers.append(key, v) } } return headers @@ -89,13 +92,19 @@ export function splitCookiesString(cookiesString: string) { return cookiesStrings } -export function toNodeHeaders(headers?: Headers): NodeHeaders { - const result: NodeHeaders = {} +export function toNodeHeaders(headers?: Headers): OutgoingHttpHeaders { + const result: OutgoingHttpHeaders = {} + const cookies: string[] = [] if (headers) { for (const [key, value] of headers.entries()) { - result[key] = value if (key.toLowerCase() === 'set-cookie') { - result[key] = splitCookiesString(value) + // We may have gotten a comma joined string of cookies, or multiple + // set-cookie headers. We need to merge them into one header array + // to represent all the cookies. + cookies.push(...splitCookiesString(value)) + result[key] = cookies + } else { + result[key] = value } } } diff --git a/packages/next/src/shared/lib/get-hostname.ts b/packages/next/src/shared/lib/get-hostname.ts index 0f642aa97eab..ff40ed42e117 100644 --- a/packages/next/src/shared/lib/get-hostname.ts +++ b/packages/next/src/shared/lib/get-hostname.ts @@ -1,3 +1,5 @@ +import type { OutgoingHttpHeaders } from 'http' + /** * Takes an object with a hostname property (like a parsed URL) and some * headers that may contain Host and returns the preferred hostname. @@ -6,9 +8,10 @@ */ export function getHostname( parsed: { hostname?: string | null }, - headers?: { [key: string]: string | string[] | undefined } + headers?: OutgoingHttpHeaders ) { return ((!Array.isArray(headers?.host) && headers?.host) || parsed.hostname) - ?.split(':')[0] + ?.toString() + .split(':')[0] .toLowerCase() } diff --git a/test/e2e/app-dir/set-cookies/app/api/app/edge/route.js b/test/e2e/app-dir/set-cookies/app/api/app/edge/route.js new file mode 100644 index 000000000000..4e93d886b94c --- /dev/null +++ b/test/e2e/app-dir/set-cookies/app/api/app/edge/route.js @@ -0,0 +1,12 @@ +export const runtime = 'edge' + +import cookies from '../../../../cookies' + +export async function GET() { + const headers = new Headers() + for (const cookie of cookies) { + headers.append('set-cookie', cookie) + } + + return new Response(null, { headers }) +} diff --git a/test/e2e/app-dir/set-cookies/app/api/app/experimental-edge/route.js b/test/e2e/app-dir/set-cookies/app/api/app/experimental-edge/route.js new file mode 100644 index 000000000000..dfce639be7ff --- /dev/null +++ b/test/e2e/app-dir/set-cookies/app/api/app/experimental-edge/route.js @@ -0,0 +1,12 @@ +export const runtime = 'experimental-edge' + +import cookies from '../../../../cookies' + +export async function GET() { + const headers = new Headers() + for (const cookie of cookies) { + headers.append('set-cookie', cookie) + } + + return new Response(null, { headers }) +} diff --git a/test/e2e/app-dir/set-cookies/app/api/app/node/route.js b/test/e2e/app-dir/set-cookies/app/api/app/node/route.js new file mode 100644 index 000000000000..d9b7f3e1e4dd --- /dev/null +++ b/test/e2e/app-dir/set-cookies/app/api/app/node/route.js @@ -0,0 +1,10 @@ +import cookies from '../../../../cookies' + +export async function GET() { + const headers = new Headers() + for (const cookie of cookies) { + headers.append('set-cookie', cookie) + } + + return new Response(null, { headers }) +} diff --git a/test/e2e/app-dir/set-cookies/cookies.mjs b/test/e2e/app-dir/set-cookies/cookies.mjs new file mode 100644 index 000000000000..34bd8323b78b --- /dev/null +++ b/test/e2e/app-dir/set-cookies/cookies.mjs @@ -0,0 +1,9 @@ +export default [ + 'foo=bar; Expires=Wed, 01 Jan 2044 00:00:00 GMT', + 'bar=foo; Expires=Wed, 01 Jan 2044 00:00:00 GMT', +] + +export const nextConfigHeaders = [ + 'fooNextConfig=bar; Expires=Wed, 01 Jan 2044 00:00:00 GMT', + 'barNextConfig=foo; Expires=Wed, 01 Jan 2044 00:00:00 GMT', +] diff --git a/test/e2e/app-dir/set-cookies/next.config.mjs b/test/e2e/app-dir/set-cookies/next.config.mjs new file mode 100644 index 000000000000..be80ea961eac --- /dev/null +++ b/test/e2e/app-dir/set-cookies/next.config.mjs @@ -0,0 +1,30 @@ +import { nextConfigHeaders } from './cookies.mjs' + +const headers = nextConfigHeaders.map((header) => ({ + key: 'Set-Cookie', + value: header, +})) + +/** + * @type {import('next').NextConfig} + */ +const config = { + experimental: { appDir: true }, + async headers() { + return [ + { + source: '/:path*', + has: [ + { + type: 'query', + key: 'next-config-headers', + value: 'true', + }, + ], + headers, + }, + ] + }, +} + +export default config diff --git a/test/e2e/app-dir/set-cookies/pages/api/pages/edge.js b/test/e2e/app-dir/set-cookies/pages/api/pages/edge.js new file mode 100644 index 000000000000..ff6b0e120919 --- /dev/null +++ b/test/e2e/app-dir/set-cookies/pages/api/pages/edge.js @@ -0,0 +1,12 @@ +export const config = { runtime: 'edge' } + +import cookies from '../../../cookies' + +export default async function handler() { + const headers = new Headers() + for (const cookie of cookies) { + headers.append('set-cookie', cookie) + } + + return new Response(null, { headers }) +} diff --git a/test/e2e/app-dir/set-cookies/pages/api/pages/experimental-edge.js b/test/e2e/app-dir/set-cookies/pages/api/pages/experimental-edge.js new file mode 100644 index 000000000000..80132349ceb4 --- /dev/null +++ b/test/e2e/app-dir/set-cookies/pages/api/pages/experimental-edge.js @@ -0,0 +1,12 @@ +export const config = { runtime: 'experimental-edge' } + +import cookies from '../../../cookies' + +export default async function handler() { + const headers = new Headers() + for (const cookie of cookies) { + headers.append('set-cookie', cookie) + } + + return new Response(null, { headers }) +} diff --git a/test/e2e/app-dir/set-cookies/pages/api/pages/node.js b/test/e2e/app-dir/set-cookies/pages/api/pages/node.js new file mode 100644 index 000000000000..605117aa1c45 --- /dev/null +++ b/test/e2e/app-dir/set-cookies/pages/api/pages/node.js @@ -0,0 +1,7 @@ +import cookies from '../../../cookies' + +export default async function handler(_req, res) { + res.appendHeader('set-cookie', cookies) + + res.json(null) +} diff --git a/test/e2e/app-dir/set-cookies/set-cookies.test.ts b/test/e2e/app-dir/set-cookies/set-cookies.test.ts new file mode 100644 index 000000000000..1474f5458e67 --- /dev/null +++ b/test/e2e/app-dir/set-cookies/set-cookies.test.ts @@ -0,0 +1,43 @@ +import { createNextDescribe } from 'e2e-utils' + +import cookies, { nextConfigHeaders } from './cookies.mjs' + +function getSetCookieHeaders(res: globalThis.Response): ReadonlyArray { + return ( + (res.headers as any).getSetCookie?.() ?? + (res.headers as any).raw()['set-cookie'] + ) +} + +createNextDescribe( + 'set-cookies', + { + files: __dirname, + }, + ({ next }) => { + describe.each(['edge', 'experimental-edge', 'node'])( + 'for %s runtime', + (runtime) => { + describe.each(['pages', 'app'])('for /%s', (dir) => { + it('should set two set-cookie headers', async () => { + let res = await next.fetch(`/api/${dir}/${runtime}`) + + let headers = getSetCookieHeaders(res) + + expect(headers).toHaveLength(2) + expect(headers).toEqual(cookies) + + res = await next.fetch( + `/api/${dir}/${runtime}?next-config-headers=true` + ) + + headers = getSetCookieHeaders(res) + + expect(headers).toHaveLength(4) + expect(headers).toEqual([...nextConfigHeaders, ...cookies]) + }) + }) + } + ) + } +)