From d996db6f0bf361ebd84b23d022db7bb10fb316e6 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Fri, 14 Jun 2024 13:57:17 +0100 Subject: [PATCH] fix: throw error if rewrite is attempted after body is used (#11258) * fix: throw error if rewrite is attempted after body is used * Apply suggestions from code review Co-authored-by: Sarah Rainsberger --------- Co-authored-by: Sarah Rainsberger --- .changeset/healthy-oranges-report.md | 5 ++++ packages/astro/src/core/errors/errors-data.ts | 23 ++++++++++++++++ packages/astro/src/core/render-context.ts | 3 +++ .../rewrite-server/src/pages/post/index.astro | 7 +++++ .../src/pages/post/post-b.astro | 15 +++++++++++ .../src/pages/post/post-body-used.astro | 17 ++++++++++++ packages/astro/test/rewrite.test.js | 27 +++++++++++++++++++ 7 files changed, 97 insertions(+) create mode 100644 .changeset/healthy-oranges-report.md create mode 100644 packages/astro/test/fixtures/rewrite-server/src/pages/post/index.astro create mode 100644 packages/astro/test/fixtures/rewrite-server/src/pages/post/post-b.astro create mode 100644 packages/astro/test/fixtures/rewrite-server/src/pages/post/post-body-used.astro diff --git a/.changeset/healthy-oranges-report.md b/.changeset/healthy-oranges-report.md new file mode 100644 index 0000000000..f7c62749ca --- /dev/null +++ b/.changeset/healthy-oranges-report.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Adds a new error `RewriteWithBodyUsed` that throws when `Astro.rewrite` is used after the request body has already been read. diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index 5a691c7501..6d0692e7e9 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -1266,6 +1266,29 @@ export const ServerOnlyModule = { message: (name: string) => `The "${name}" module is only available server-side.`, } satisfies ErrorData; + +/** + * @docs + * @description + * `Astro.rewrite()` cannot be used if the request body has already been read. If you need to read the body, first clone the request. For example: + * + * ```js + * const data = await Astro.request.clone().formData(); + * + * Astro.rewrite("/target") + * ``` + * + * @see + * - [Request.clone()](https://developer.mozilla.org/en-US/docs/Web/API/Request/clone) + * - [Astro.rewrite](https://docs.astro.build/en/reference/configuration-reference/#experimentalrewriting) + */ + +export const RewriteWithBodyUsed = { + name: 'RewriteWithBodyUsed', + title: 'Cannot use Astro.rewrite after the request body has been read', + message: 'Astro.rewrite() cannot be used if the request body has already been read. If you need to read the body, first clone the request.', +} satisfies ErrorData; + /** * @docs * @kind heading diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index ff25cef28f..6bafba1e7c 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -542,6 +542,9 @@ export class RenderContext { * @param oldRequest The old `Request` */ #copyRequest(newUrl: URL, oldRequest: Request): Request { + if(oldRequest.bodyUsed) { + throw new AstroError(AstroErrorData.RewriteWithBodyUsed); + } return new Request(newUrl, { method: oldRequest.method, headers: oldRequest.headers, diff --git a/packages/astro/test/fixtures/rewrite-server/src/pages/post/index.astro b/packages/astro/test/fixtures/rewrite-server/src/pages/post/index.astro new file mode 100644 index 0000000000..a41d39c2fc --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-server/src/pages/post/index.astro @@ -0,0 +1,7 @@ +--- +--- + +
+ + +
diff --git a/packages/astro/test/fixtures/rewrite-server/src/pages/post/post-b.astro b/packages/astro/test/fixtures/rewrite-server/src/pages/post/post-b.astro new file mode 100644 index 0000000000..c0e04bc7d7 --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-server/src/pages/post/post-b.astro @@ -0,0 +1,15 @@ +--- +let email = '' +if (Astro.request.method === 'POST') { + try { + const data = await Astro.request.formData(); + email = data.get('email'); + } catch (e) { + console.log(e) + } +} +--- + +

Post B

+ +

{email}

diff --git a/packages/astro/test/fixtures/rewrite-server/src/pages/post/post-body-used.astro b/packages/astro/test/fixtures/rewrite-server/src/pages/post/post-body-used.astro new file mode 100644 index 0000000000..7fb0fbb1dd --- /dev/null +++ b/packages/astro/test/fixtures/rewrite-server/src/pages/post/post-body-used.astro @@ -0,0 +1,17 @@ +--- +let data +if (Astro.request.method === 'POST') { + try { + data = await Astro.request.text(); + } catch (e) { + console.log(e) + } +} + +return Astro.rewrite('/post/post-b') + +--- + +

Post body used

+ +

{data}

diff --git a/packages/astro/test/rewrite.test.js b/packages/astro/test/rewrite.test.js index d0d2376398..89e7576545 100644 --- a/packages/astro/test/rewrite.test.js +++ b/packages/astro/test/rewrite.test.js @@ -85,6 +85,21 @@ describe('Dev rewrite, hybrid/server', () => { assert.match($('h1').text(), /Title/); assert.match($('p').text(), /some-slug/); }); + + it('should display an error if a rewrite is attempted after the body has been consumed', async () => { + const formData = new FormData(); + formData.append('email', 'example@example.com'); + + const request = new Request('http://example.com/post/post-body-used', { + method: 'POST', + body: formData, + }); + const response = await fixture.fetch('/post/post-body-used', request); + const html = await response.text(); + const $ = cheerioLoad(html); + + assert.equal($('title').text(), 'RewriteWithBodyUsed'); + }); }); describe('Build reroute', () => { @@ -272,6 +287,18 @@ describe('SSR rewrite, hybrid/server', () => { assert.match($('h1').text(), /Title/); assert.match($('p').text(), /some-slug/); }); + + it('should return a 500 if a rewrite is attempted after the body has been read', async () => { + const formData = new FormData(); + formData.append('email', 'example@example.com'); + + const request = new Request('http://example.com/post/post-body-used', { + method: 'POST', + body: formData, + }); + const response = await app.render(request); + assert.equal(response.status, 500); + }); }); describe('Middleware', () => {