fix: cookies set in middleware accessible during the same request (#2847)

* fix: allow cookies in middleware to be accessible during the same request

* test: validate functionality of mergeMiddlewareCookies

* ci: pin deno version to 1.x until builds are ready for 2.x

* ci: add vendor step before running deno tests

* chore: include reference link for cookie regex

* fix: directives in cookies are joined by a semi-colon with a space

* fix: make cookies available during first request on rewrite as well

---------

Co-authored-by: Michal Piechowiak <[email protected]>

Author: mrstork

.github/workflows/deno-test.yml

@@ -11,6 +11,8 @@ jobs:
       - name: Setup Deno
         uses: denoland/setup-deno@v1
         with:
-          deno-version: vx.x.x
+          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/

edge-runtime/lib/middleware.test.ts

@@ -0,0 +1,92 @@
+import { assertEquals } from 'https://deno.land/[email protected]/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')
+  })
+})

edge-runtime/lib/middleware.ts

@@ -1,6 +1,7 @@
 import type { Context } from '@netlify/edge-functions'
 
 import type { ElementHandlers } from '../vendor/deno.land/x/[email protected]/src/index.ts'
+import { getCookies } from '../vendor/deno.land/[email protected]/http/cookie.ts'
 
 type NextDataTransform = <T>(data: T) => T
 
@@ -58,3 +59,28 @@ export const addMiddlewareHeaders = async (
   })
   return response
 }
+
+// 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')
+
+  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) => {
+      // 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
+    })
+  }
+
+  return Object.entries(mergedCookies)
+    .map((kv) => kv.join('='))
+    .join('; ')
+}

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,17 @@ 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)
+
+    // 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) {
@@ -208,27 +222,35 @@ 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)
+    const newRequestCookies = mergeMiddlewareCookies(edgeResponse, newRequest)
+    if (newRequestCookies) {
+      newRequest.headers.set('Cookie', newRequestCookies)
+    }
+
+    return addMiddlewareHeaders(context.next(newRequest), edgeResponse)
   }
 
-  return res
+  return edgeResponse
 }
 
 /**