From c8379d6ca5c0ffbe1b26b22a7577fc5f8b98424d Mon Sep 17 00:00:00 2001 From: Mateusz Bocian Date: Tue, 15 Apr 2025 19:00:55 -0400 Subject: [PATCH 1/7] fix: allow cookies in middleware to be accessible during the same request --- edge-runtime/lib/middleware.ts | 17 ++++++++++++ edge-runtime/lib/response.ts | 47 +++++++++++++++++++++------------- 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/edge-runtime/lib/middleware.ts b/edge-runtime/lib/middleware.ts index 2ae25c1d34..d280bcdec2 100644 --- a/edge-runtime/lib/middleware.ts +++ b/edge-runtime/lib/middleware.ts @@ -1,6 +1,7 @@ import type { Context } from '@netlify/edge-functions' import type { ElementHandlers } from '../vendor/deno.land/x/htmlrewriter@v1.0.0/src/index.ts' +import { getCookies } from '../vendor/deno.land/std@0.175.0/http/cookie.ts' type NextDataTransform = (data: T) => T @@ -58,3 +59,19 @@ export const addMiddlewareHeaders = async ( }) return response } + +export function mergeMiddlewareCookies(middlewareResponse: Response, request: Request) { + let mergedCookies = getCookies(request.headers) + const middlewareCookies = middlewareResponse.headers.get('x-middleware-set-cookie') || '' + const regex = new RegExp(/,(?!\s)/) // commas that are not followed by whitespace + + middlewareCookies.split(regex).forEach((entry) => { + const [cookie] = entry.split(';') + const [name, value] = cookie.split('=') + mergedCookies[name] = value + }) + + return Object.entries(mergedCookies) + .map((kv) => kv.join('=')) + .join('; ') +} diff --git a/edge-runtime/lib/response.ts b/edge-runtime/lib/response.ts index ec0730e8ce..5ba44a037e 100644 --- a/edge-runtime/lib/response.ts +++ b/edge-runtime/lib/response.ts @@ -6,7 +6,12 @@ import { import { updateModifiedHeaders } from './headers.ts' import type { StructuredLogger } from './logging.ts' -import { addMiddlewareHeaders, isMiddlewareRequest, isMiddlewareResponse } from './middleware.ts' +import { + addMiddlewareHeaders, + isMiddlewareRequest, + isMiddlewareResponse, + mergeMiddlewareCookies, +} from './middleware.ts' import { RequestData } from './next-request.ts' import { addBasePath, @@ -116,12 +121,13 @@ export const buildResponse = async ({ } return rewriter.transform(response.originResponse) } - const res = new Response(result.response.body, result.response) + + const edgeResponse = new Response(result.response.body, result.response) request.headers.set('x-nf-next-middleware', 'skip') - let rewrite = res.headers.get('x-middleware-rewrite') - let redirect = res.headers.get('location') - let nextRedirect = res.headers.get('x-nextjs-redirect') + let rewrite = edgeResponse.headers.get('x-middleware-rewrite') + let redirect = edgeResponse.headers.get('location') + let nextRedirect = edgeResponse.headers.get('x-nextjs-redirect') // Data requests (i.e. requests for /_next/data ) need special handling const isDataReq = request.headers.has('x-nextjs-data') @@ -152,7 +158,7 @@ export const buildResponse = async ({ // Data requests might be rewritten to an external URL // This header tells the client router the redirect target, and if it's external then it will do a full navigation - res.headers.set('x-nextjs-rewrite', relativeUrl) + edgeResponse.headers.set('x-nextjs-rewrite', relativeUrl) } if (rewriteUrl.origin !== baseUrl.origin) { @@ -178,7 +184,7 @@ export const buildResponse = async ({ }) } - return addMiddlewareHeaders(fetch(proxyRequest, { redirect: 'manual' }), res) + return addMiddlewareHeaders(fetch(proxyRequest, { redirect: 'manual' }), edgeResponse) } if (isDataReq) { @@ -197,9 +203,9 @@ export const buildResponse = async ({ logger.withFields({ rewrite_url: rewrite }).debug('Rewrite url is same as original url') return } - res.headers.set('x-middleware-rewrite', relativeUrl) + edgeResponse.headers.set('x-middleware-rewrite', relativeUrl) request.headers.set('x-middleware-rewrite', target) - return addMiddlewareHeaders(context.rewrite(target), res) + return addMiddlewareHeaders(context.rewrite(target), edgeResponse) } if (redirect) { @@ -208,27 +214,32 @@ export const buildResponse = async ({ logger.withFields({ redirect_url: redirect }).debug('Redirect url is same as original url') return } - res.headers.set('location', redirect) + edgeResponse.headers.set('location', redirect) } // Data requests shouldn't automatically redirect in the browser (they might be HTML pages): they're handled by the router if (redirect && isDataReq) { - res.headers.delete('location') - res.headers.set('x-nextjs-redirect', relativizeURL(redirect, request.url)) + edgeResponse.headers.delete('location') + edgeResponse.headers.set('x-nextjs-redirect', relativizeURL(redirect, request.url)) } - nextRedirect = res.headers.get('x-nextjs-redirect') + nextRedirect = edgeResponse.headers.get('x-nextjs-redirect') if (nextRedirect && isDataReq) { - res.headers.set('x-nextjs-redirect', normalizeDataUrl(nextRedirect)) + edgeResponse.headers.set('x-nextjs-redirect', normalizeDataUrl(nextRedirect)) } - if (res.headers.get('x-middleware-next') === '1') { - res.headers.delete('x-middleware-next') - return addMiddlewareHeaders(context.next(), res) + if (edgeResponse.headers.get('x-middleware-next') === '1') { + edgeResponse.headers.delete('x-middleware-next') + + // coookies set in middleware need to be available during the lambda request + const newRequest = new Request(request) + newRequest.headers.set('Cookie', mergeMiddlewareCookies(edgeResponse, newRequest)) + + return addMiddlewareHeaders(context.next(newRequest), edgeResponse) } - return res + return edgeResponse } /** From f85b1e158b8ca44a3f762fa000aece38b745dbc4 Mon Sep 17 00:00:00 2001 From: Mateusz Bocian Date: Wed, 16 Apr 2025 11:32:24 -0400 Subject: [PATCH 2/7] test: validate functionality of mergeMiddlewareCookies --- edge-runtime/lib/middleware.test.ts | 92 +++++++++++++++++++++++++++++ edge-runtime/lib/middleware.ts | 20 ++++--- edge-runtime/lib/response.ts | 5 +- 3 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 edge-runtime/lib/middleware.test.ts diff --git a/edge-runtime/lib/middleware.test.ts b/edge-runtime/lib/middleware.test.ts new file mode 100644 index 0000000000..e3f33245eb --- /dev/null +++ b/edge-runtime/lib/middleware.test.ts @@ -0,0 +1,92 @@ +import { assertEquals } from 'https://deno.land/std@0.175.0/testing/asserts.ts' +import { mergeMiddlewareCookies } from './middleware.ts' + +const MIDDLEWARE_HEADER = 'x-middleware-set-cookie' + +Deno.test('mergeMiddlewareCookies', async (t) => { + await t.step('should handle empty cookies', async () => { + const request = new Request('https://www.test-url.com') + const response = new Response() + + const result = mergeMiddlewareCookies(response, request) + assertEquals(result, '') + }) + + await t.step('should return request cookies when there are no middleware headers', async () => { + const request = new Request('https://www.test-url.com') + const response = new Response() + + request.headers.set('Cookie', 'oatmeal=raisin') + + const result = mergeMiddlewareCookies(response, request) + assertEquals(result, 'oatmeal=raisin') + }) + + await t.step('should not require cookies in request to be set', async () => { + const request = new Request('https://www.test-url.com') + const response = new Response() + + response.headers.set(MIDDLEWARE_HEADER, 'peanut=butter; Path=/') + + const result = mergeMiddlewareCookies(response, request) + assertEquals(result, 'peanut=butter') + }) + + await t.step('should merge request and middleware cookies', async () => { + const request = new Request('https://www.test-url.com') + const response = new Response() + + request.headers.set('Cookie', 'oatmeal=raisin') + response.headers.set(MIDDLEWARE_HEADER, 'peanut=butter; Path=/') + + const result = mergeMiddlewareCookies(response, request) + assertEquals(result, 'oatmeal=raisin; peanut=butter') + }) + + await t.step('should overwrite request cookies with latest values', async () => { + const request = new Request('https://www.test-url.com') + const response = new Response() + + request.headers.set('Cookie', 'oatmeal=chocolate') + response.headers.set(MIDDLEWARE_HEADER, 'oatmeal=raisin; Path=/') + + const result = mergeMiddlewareCookies(response, request) + assertEquals(result, 'oatmeal=raisin') + }) + + await t.step('should not decode middleware cookie values', async () => { + const request = new Request('https://www.test-url.com') + const response = new Response() + + response.headers.set(MIDDLEWARE_HEADER, 'greeting=Hello%20from%20the%20cookie; Path=/') + + const result = mergeMiddlewareCookies(response, request) + assertEquals(result, 'greeting=Hello%20from%20the%20cookie') + }) + + await t.step('should support multiple cookies being set in middleware', async () => { + const request = new Request('https://www.test-url.com') + const response = new Response() + + response.headers.set( + MIDDLEWARE_HEADER, + 'oatmeal=raisin; Path=/,peanut=butter; Path=/,chocolate=chip; Path=/', + ) + + const result = mergeMiddlewareCookies(response, request) + assertEquals(result, 'oatmeal=raisin; peanut=butter; chocolate=chip') + }) + + await t.step('should ignore comma in middleware cookie expiry', async () => { + const request = new Request('https://www.test-url.com') + const response = new Response() + + response.headers.set( + MIDDLEWARE_HEADER, + 'oatmeal=raisin; Path=/; Expires=Wed, 23 Apr 2025 13:37:43 GMT; Max-Age=604800', + ) + + const result = mergeMiddlewareCookies(response, request) + assertEquals(result, 'oatmeal=raisin') + }) +}) diff --git a/edge-runtime/lib/middleware.ts b/edge-runtime/lib/middleware.ts index d280bcdec2..8f507a5722 100644 --- a/edge-runtime/lib/middleware.ts +++ b/edge-runtime/lib/middleware.ts @@ -60,16 +60,20 @@ export const addMiddlewareHeaders = async ( return response } -export function mergeMiddlewareCookies(middlewareResponse: Response, request: Request) { - let mergedCookies = getCookies(request.headers) - const middlewareCookies = middlewareResponse.headers.get('x-middleware-set-cookie') || '' +// This serves the same purpose as the mergeMiddlewareCookies in Next.js but has been customized to our domain +// See: https://github.com/vercel/next.js/blob/6e4495f8430eab33b12cd11dffdd8e27eee6e0cf/packages/next/src/server/async-storage/request-store.ts#L78-L105 +export function mergeMiddlewareCookies(middlewareResponse: Response, lambdaRequest: Request) { + let mergedCookies = getCookies(lambdaRequest.headers) + const middlewareCookies = middlewareResponse.headers.get('x-middleware-set-cookie') const regex = new RegExp(/,(?!\s)/) // commas that are not followed by whitespace - middlewareCookies.split(regex).forEach((entry) => { - const [cookie] = entry.split(';') - const [name, value] = cookie.split('=') - mergedCookies[name] = value - }) + if (middlewareCookies) { + middlewareCookies.split(regex).forEach((entry) => { + const [cookie] = entry.split(';') + const [name, value] = cookie.split('=') + mergedCookies[name] = value + }) + } return Object.entries(mergedCookies) .map((kv) => kv.join('=')) diff --git a/edge-runtime/lib/response.ts b/edge-runtime/lib/response.ts index 5ba44a037e..8c5a3f18dd 100644 --- a/edge-runtime/lib/response.ts +++ b/edge-runtime/lib/response.ts @@ -234,7 +234,10 @@ export const buildResponse = async ({ // coookies set in middleware need to be available during the lambda request const newRequest = new Request(request) - newRequest.headers.set('Cookie', mergeMiddlewareCookies(edgeResponse, newRequest)) + const newRequestCookies = mergeMiddlewareCookies(edgeResponse, newRequest) + if (newRequestCookies) { + newRequest.headers.set('Cookie', newRequestCookies) + } return addMiddlewareHeaders(context.next(newRequest), edgeResponse) } From ce05e85d5379d71be125faa3dac9b54102db76b9 Mon Sep 17 00:00:00 2001 From: Mateusz Bocian Date: Wed, 16 Apr 2025 11:46:55 -0400 Subject: [PATCH 3/7] ci: pin deno version to 1.x until builds are ready for 2.x --- .github/workflows/deno-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deno-test.yml b/.github/workflows/deno-test.yml index ccd44cdf6a..699935e824 100644 --- a/.github/workflows/deno-test.yml +++ b/.github/workflows/deno-test.yml @@ -11,6 +11,6 @@ jobs: - name: Setup Deno uses: denoland/setup-deno@v1 with: - deno-version: vx.x.x + deno-version: v1.x.x - name: Test run: deno test -A edge-runtime/ From bc2521898deba1e7eb52fbcd9466e91b8e76bdd3 Mon Sep 17 00:00:00 2001 From: Mateusz Bocian Date: Wed, 16 Apr 2025 11:52:29 -0400 Subject: [PATCH 4/7] ci: add vendor step before running deno tests --- .github/workflows/deno-test.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/deno-test.yml b/.github/workflows/deno-test.yml index 699935e824..803abf8e3a 100644 --- a/.github/workflows/deno-test.yml +++ b/.github/workflows/deno-test.yml @@ -12,5 +12,7 @@ jobs: uses: denoland/setup-deno@v1 with: deno-version: v1.x.x + - name: Vendor Deno modules + run: deno vendor edge-runtime/vendor.ts --output=edge-runtime/vendor --force - name: Test run: deno test -A edge-runtime/ From a140dc8679c113e2a402d0ebac6487caefb14c56 Mon Sep 17 00:00:00 2001 From: Mateusz Bocian Date: Thu, 17 Apr 2025 10:09:58 -0400 Subject: [PATCH 5/7] chore: include reference link for cookie regex --- edge-runtime/lib/middleware.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/edge-runtime/lib/middleware.ts b/edge-runtime/lib/middleware.ts index 8f507a5722..511017c4b2 100644 --- a/edge-runtime/lib/middleware.ts +++ b/edge-runtime/lib/middleware.ts @@ -65,9 +65,12 @@ export const addMiddlewareHeaders = async ( export function mergeMiddlewareCookies(middlewareResponse: Response, lambdaRequest: Request) { let mergedCookies = getCookies(lambdaRequest.headers) const middlewareCookies = middlewareResponse.headers.get('x-middleware-set-cookie') - const regex = new RegExp(/,(?!\s)/) // commas that are not followed by whitespace if (middlewareCookies) { + // Targets commas that are not followed by whitespace + // See: https://github.com/vercel/next.js/blob/e6145d3a37bb4c7b481fd58e05cdff9046ace8ad/packages/next/src/server/web/spec-extension/response.ts#L58-L66 + const regex = new RegExp(/,(?!\s)/) + middlewareCookies.split(regex).forEach((entry) => { const [cookie] = entry.split(';') const [name, value] = cookie.split('=') From 5d7b2d9e8944cd2158d95731e9660d2157f595a0 Mon Sep 17 00:00:00 2001 From: Mateusz Bocian Date: Thu, 17 Apr 2025 17:04:39 -0400 Subject: [PATCH 6/7] fix: directives in cookies are joined by a semi-colon with a space --- edge-runtime/lib/middleware.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/edge-runtime/lib/middleware.ts b/edge-runtime/lib/middleware.ts index 511017c4b2..28f923a794 100644 --- a/edge-runtime/lib/middleware.ts +++ b/edge-runtime/lib/middleware.ts @@ -72,7 +72,9 @@ export function mergeMiddlewareCookies(middlewareResponse: Response, lambdaReque const regex = new RegExp(/,(?!\s)/) middlewareCookies.split(regex).forEach((entry) => { - const [cookie] = entry.split(';') + // Extra directives within a cookie are joined on separated by "; " + // See: https://github.com/vercel/next.js/blob/0edb1123066a010eff2aac274f948ca2c6e85c0f/packages/next/src/compiled/%40edge-runtime/cookies/index.js#L32-L47 + const [cookie] = entry.split('; ') const [name, value] = cookie.split('=') mergedCookies[name] = value }) From 4a900a78feb5e5e2f26ba3f3182729deff397209 Mon Sep 17 00:00:00 2001 From: Mateusz Bocian Date: Thu, 17 Apr 2025 18:35:37 -0400 Subject: [PATCH 7/7] fix: make cookies available during first request on rewrite as well --- edge-runtime/lib/response.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/edge-runtime/lib/response.ts b/edge-runtime/lib/response.ts index 8c5a3f18dd..60cf9fd4b9 100644 --- a/edge-runtime/lib/response.ts +++ b/edge-runtime/lib/response.ts @@ -205,7 +205,15 @@ export const buildResponse = async ({ } edgeResponse.headers.set('x-middleware-rewrite', relativeUrl) request.headers.set('x-middleware-rewrite', target) - return addMiddlewareHeaders(context.rewrite(target), edgeResponse) + + // coookies set in middleware need to be available during the lambda request + const newRequest = new Request(target, request) + const newRequestCookies = mergeMiddlewareCookies(edgeResponse, newRequest) + if (newRequestCookies) { + newRequest.headers.set('Cookie', newRequestCookies) + } + + return addMiddlewareHeaders(context.next(newRequest), edgeResponse) } if (redirect) {