Handle GET and HEAD request bodies during rewrites (#17813)

Author: matthewp

.changeset/tiny-mirrors-relate.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes `rewrite()` and `next(payload)` for GET and HEAD requests with host-provided bodies

packages/astro/src/core/middleware/sequence.ts

@@ -9,7 +9,7 @@ import { AstroError } from '../errors/index.js';
 // module, which sits on the other side of the middleware import cycle.
 import type { FetchState } from '../fetch/fetch-state.js';
 import { getParams } from '../render/params-and-props.js';
-import { setOriginPathname } from '../routing/rewrite.js';
+import { copyRequest, setOriginPathname } from '../routing/rewrite.js';
 import { defineMiddleware } from './defineMiddleware.js';
 
 // From SvelteKit: https://github.com/sveltejs/kit/blob/master/packages/kit/src/exports/hooks/sequence.js
@@ -40,19 +40,6 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler {
 			const result = handle(handleContext, async (payload?: RewritePayload) => {
 				if (i < length - 1) {
 					if (payload) {
-						let newRequest;
-						if (payload instanceof Request) {
-							newRequest = payload;
-						} else if (payload instanceof URL) {
-							// Cloning the original request ensures that the new Request gets its own copy of the body stream
-							// Without this it will throw an error if they both try to consume the stream, which will happen in a rewrite
-							newRequest = new Request(payload, handleContext.request.clone());
-						} else {
-							newRequest = new Request(
-								new URL(payload, handleContext.url.origin),
-								handleContext.request.clone(),
-							);
-						}
 						const oldPathname = handleContext.url.pathname;
 						const state = Reflect.get(handleContext, fetchStateSymbol) as FetchState | undefined;
 						if (!state) {
@@ -67,6 +54,18 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler {
 							payload,
 							handleContext.request,
 						);
+						let newRequest: Request;
+						if (payload instanceof Request) {
+							newRequest = payload;
+						} else {
+							const request =
+								handleContext.request.method === 'GET' || handleContext.request.method === 'HEAD'
+									? handleContext.request
+									: handleContext.request.clone();
+							const newUrl =
+								payload instanceof URL ? payload : new URL(payload, handleContext.url.origin);
+							newRequest = copyRequest(newUrl, request, false, state.logger, routeData.route);
+						}
 
 						// This is a case where the user tries to rewrite from a SSR route to a prerendered route (SSG).
 						// This case isn't valid because when building for SSR, the prerendered route disappears from the server output because it becomes an HTML file,

packages/astro/src/core/routing/rewrite.ts

@@ -149,13 +149,14 @@ export function copyRequest(
 	logger: AstroLogger,
 	routePattern: string,
 ): Request {
-	if (oldRequest.bodyUsed) {
+	const canHaveBody = oldRequest.method !== 'GET' && oldRequest.method !== 'HEAD';
+	if (canHaveBody && oldRequest.bodyUsed) {
 		throw new AstroError(AstroErrorData.RewriteWithBodyUsed);
 	}
 	return createRequest({
 		url: newUrl,
 		method: oldRequest.method,
-		body: oldRequest.body,
+		body: canHaveBody ? oldRequest.body : undefined,
 		isPrerendered,
 		logger,
 		headers: isPrerendered ? {} : oldRequest.headers,

packages/astro/test/units/routing/rewrite-app.test.ts

@@ -31,6 +31,16 @@ const postBPage = createComponent(async (result: any, props: any, slots: any) =>
 	return render`<h1>Post B</h1><h2>${email}</h2>`;
 });
 
+function createRequestWithHostProvidedBody(method: 'GET' | 'HEAD', url: string) {
+	const request = new Request(url, {
+		method: 'POST',
+		body: 'x',
+		headers: { 'content-length': '1', 'content-type': 'text/plain' },
+	});
+	Object.defineProperty(request, 'method', { value: method });
+	return request;
+}
+
 describe('Rewrites via App - basic', () => {
 	const app = createTestApp([
 		createPage(rewriteTo('/'), { route: '/reroute' }),
@@ -125,6 +135,86 @@ describe('Rewrites via App - POST body forwarding', () => {
 	});
 });
 
+describe('Rewrites via App - GET and HEAD body handling', () => {
+	const rewrittenRequests: Request[] = [];
+	const targetPage = createComponent((result: any, props: any, slots: any) => {
+		const Astro = result.createAstro(props, slots);
+		rewrittenRequests.push(Astro.request);
+		return render`<h1>Target</h1>`;
+	});
+	const app = createTestApp([
+		createPage(rewriteTo('/target'), { route: '/source' }),
+		createPage(targetPage, { route: '/target' }),
+	]);
+
+	for (const method of ['GET', 'HEAD'] as const) {
+		it(`omits a host-provided ${method} body from the rewritten request`, async () => {
+			rewrittenRequests.length = 0;
+			const res = await app.render(
+				createRequestWithHostProvidedBody(method, 'http://example.com/source'),
+			);
+
+			assert.equal(res.status, 200);
+			const rewrittenRequest = rewrittenRequests.at(-1);
+			assert.ok(rewrittenRequest);
+			assert.equal(rewrittenRequest.method, method);
+			assert.equal(rewrittenRequest.body, null);
+		});
+	}
+});
+
+describe('Rewrites via App - GET and HEAD body handling in middleware sequences', () => {
+	const sequencedRequests: Request[] = [];
+	const first = async (_context: APIContext, next: MiddlewareNext) => next('/post/post-b');
+	const second = async (context: APIContext, next: MiddlewareNext) => {
+		sequencedRequests.push(context.request);
+		return next();
+	};
+	const app = createTestApp(
+		[
+			createPage(
+				createComponent(() => render``),
+				{ route: '/source' },
+			),
+			createPage(postBPage, { route: '/post/post-b' }),
+		],
+		{ middleware: () => ({ onRequest: sequence(first, second) }) },
+	);
+
+	for (const method of ['GET', 'HEAD'] as const) {
+		it(`omits a host-provided ${method} body before running the next middleware`, async () => {
+			sequencedRequests.length = 0;
+			const res = await app.render(
+				createRequestWithHostProvidedBody(method, 'http://example.com/source'),
+			);
+
+			assert.equal(res.status, 200);
+			const sequencedRequest = sequencedRequests.at(-1);
+			assert.ok(sequencedRequest);
+			assert.equal(sequencedRequest.method, method);
+			assert.equal(sequencedRequest.body, null);
+		});
+	}
+
+	it('preserves a POST body through the sequenced and final rewritten requests', async () => {
+		sequencedRequests.length = 0;
+		const res = await app.render(
+			new Request('http://example.com/source', {
+				method: 'POST',
+				body: JSON.stringify({ email: 'example@example.com' }),
+				headers: { 'content-type': 'application/json' },
+			}),
+		);
+
+		assert.equal(res.status, 200);
+		const $ = cheerio.load(await res.text());
+		assert.match($('h2').text(), /example@example.com/);
+		const sequencedRequest = sequencedRequests.at(-1);
+		assert.ok(sequencedRequest);
+		assert.deepEqual(await sequencedRequest.json(), { email: 'example@example.com' });
+	});
+});
+
 describe('Rewrites via App - URL and Request payloads', () => {
 	const app = createTestApp([
 		createPage(