fix: don't lose cookies in middleware sequences

ascorbic · ascorbic · commit 45a760e9969c · 2025-08-28T12:15:47.000+01:00
diff --git a/.changeset/open-cities-make.md b/.changeset/open-cities-make.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a bug that caused cookies to not be correctly set when using middleware sequences
diff --git a/packages/astro/src/core/middleware/sequence.ts b/packages/astro/src/core/middleware/sequence.ts
@@ -1,6 +1,5 @@
 import type { MiddlewareHandler, RewritePayload } from '../../types/public/common.js';
 import type { APIContext } from '../../types/public/context.js';
-import { AstroCookies } from '../cookies/cookies.js';
 import { ForbiddenRewrite } from '../errors/errors-data.js';
 import { AstroError } from '../errors/index.js';
 import { getParams, type Pipeline } from '../render/index.js';
@@ -78,7 +77,6 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler {
 						carriedPayload = payload;
 						handleContext.request = newRequest;
 						handleContext.url = new URL(newRequest.url);
-						handleContext.cookies = new AstroCookies(newRequest);
 						handleContext.params = getParams(routeData, pathname);
 						handleContext.routePattern = routeData.route;
 						setOriginPathname(
diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
@@ -131,7 +131,7 @@ export class RenderContext {
 		componentInstance: ComponentInstance | undefined,
 		slots: Record<string, any> = {},
 	): Promise<Response> {
-		const { cookies, middleware, pipeline } = this;
+		const { middleware, pipeline } = this;
 		const { logger, serverLike, streaming, manifest } = pipeline;
 
 		const props =
@@ -271,7 +271,7 @@ export class RenderContext {
 			// because they may need to be passed along from a rewrite.
 			const responseCookies = getCookiesFromResponse(response);
 			if (responseCookies) {
-				cookies.merge(responseCookies);
+				this.cookies.merge(responseCookies);
 			}
 			return response;
 		};
@@ -289,7 +289,7 @@ export class RenderContext {
 		// LEGACY: we put cookies on the response object,
 		// where the adapter might be expecting to read it.
 		// New code should be using `app.render({ addCookieHeader: true })` instead.
-		attachCookiesToResponse(response, cookies);
+		attachCookiesToResponse(response, this.cookies);
 		return response;
 	}
 
@@ -346,7 +346,11 @@ export class RenderContext {
 			);
 		}
 		this.url = new URL(this.request.url);
-		this.cookies = new AstroCookies(this.request);
+		const newCookies = new AstroCookies(this.request);
+		if(this.cookies) {
+			newCookies.merge(this.cookies);
+		}
+		this.cookies = newCookies;
 		this.params = getParams(routeData, pathname);
 		this.pathname = pathname;
 		this.isRewriting = true;
@@ -363,15 +367,18 @@ export class RenderContext {
 
 	createActionAPIContext(): ActionAPIContext {
 		const renderContext = this;
-		const { cookies, params, pipeline, url } = this;
+		const { params, pipeline, url } = this;
 		const generator = `Astro v${ASTRO_VERSION}`;
 
 		const rewrite = async (reroutePayload: RewritePayload) => {
 			return await this.#executeRewrite(reroutePayload);
 		};
 
 		return {
-			cookies,
+			// Don't allow reassignment of cookies because it doesn't work
+			get cookies() {
+				return renderContext.cookies;
+			},
 			routePattern: this.routeData.route,
 			isPrerendered: this.routeData.prerender,
 			get clientAddress() {
diff --git a/packages/astro/test/fixtures/middleware-sequence-request-clone/src/middleware.js b/packages/astro/test/fixtures/middleware-sequence-request-clone/src/middleware.js
@@ -2,14 +2,16 @@ import { sequence, defineMiddleware } from 'astro/middleware';
 
 const middleware1 = defineMiddleware((_, next) => next('/'));
 
-const middleware2 = defineMiddleware(async ({ request }, next) => {
+const middleware2 = defineMiddleware(async ({ request, cookies }, next) => {
+	cookies.set('cookie1', 'Cookie from middleware 1');
 	console.log(await request.clone().text());
 	return next();
 });
 
-const middleware3 = defineMiddleware(async ({ request }, next) => {
+const middleware3 = defineMiddleware(async ({ request, cookies }, next) => {
+	cookies.set('cookie2', 'Cookie from middleware 2');
 	await request.clone();
 	return next();
 });
 
-export const onRequest = sequence(middleware1, middleware2, middleware3);
+export const onRequest = sequence(middleware1, middleware2, middleware3);
diff --git a/packages/astro/test/fixtures/middleware-sequence-rewrite/astro.config.mjs b/packages/astro/test/fixtures/middleware-sequence-rewrite/astro.config.mjs
@@ -0,0 +1,8 @@
+import { defineConfig } from 'astro/config';
+
+// https://astro.build/config
+export default defineConfig({
+	vite: {
+		plugins: [],
+	}
+});
diff --git a/packages/astro/test/fixtures/middleware-sequence-rewrite/package.json b/packages/astro/test/fixtures/middleware-sequence-rewrite/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "@test/middleware-sequence-rewrite",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}
diff --git a/packages/astro/test/fixtures/middleware-sequence-rewrite/src/middleware.ts b/packages/astro/test/fixtures/middleware-sequence-rewrite/src/middleware.ts
@@ -0,0 +1,19 @@
+import type { MiddlewareHandler } from 'astro';
+import { sequence } from 'astro:middleware';
+
+export const mid1: MiddlewareHandler = async ({ cookies, url, rewrite, request }, next) => {
+	cookies.set('cookie1', 'Cookie from middleware 1');
+	if (url.pathname === '/') {
+		console.log('Rewriting');
+		return rewrite('/another');
+	}
+	return next();
+};
+
+export const mid2: MiddlewareHandler = async ({ cookies, url }, next) => {
+	cookies.set('cookie2', 'Cookie from middleware 2');
+	const res = next();
+	return res;
+};
+
+export const onRequest: MiddlewareHandler = sequence(mid1, mid2);
diff --git a/packages/astro/test/fixtures/middleware-sequence-rewrite/src/pages/another.astro b/packages/astro/test/fixtures/middleware-sequence-rewrite/src/pages/another.astro
@@ -0,0 +1 @@
+<h1>Hello Another</h1>
diff --git a/packages/astro/test/fixtures/middleware-sequence-rewrite/src/pages/index.astro b/packages/astro/test/fixtures/middleware-sequence-rewrite/src/pages/index.astro
@@ -0,0 +1 @@
+<h1>Hello Sequence</h1>
diff --git a/packages/astro/test/middleware.test.js b/packages/astro/test/middleware.test.js
@@ -413,4 +413,35 @@ describe('Middleware should support clone request', () => {
 		const html = await res.text();
 		assert.equal(html.includes('Hello Sequence and Request Clone'), true);
 	});
+
+	it('should preserve cookies set in sequence', async () => {
+		const res = await fixture.fetch('/');
+		assert.ok(res.headers.get('set-cookie').includes('cookie1=Cookie%20from%20middleware%201'));
+		assert.ok(res.headers.get('set-cookie').includes('cookie2=Cookie%20from%20middleware%202'));
+	});
+});
+
+describe('Middleware sequence rewrites', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+	let devServer;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/middleware-sequence-rewrite/',
+		});
+		devServer = await fixture.startDevServer();
+	});
+
+	after(async () => {
+		await devServer.stop();
+	});
+
+	it('should preserve cookies set in sequence', async () => {
+		const res = await fixture.fetch('/');
+		const html = await res.text();
+		assert.ok(html.includes('Hello Another'));
+		assert.ok(res.headers.get('set-cookie').includes('cookie1=Cookie%20from%20middleware%201'));
+		assert.ok(res.headers.get('set-cookie').includes('cookie2=Cookie%20from%20middleware%202'));
+	});
 });
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'astro': patch
 +---
++
 +Fixes a bug that caused cookies to not be correctly set when using middleware sequences