Skip to content

Commit

Permalink
fix(dev): remove params for prerendered pages (#10199)
Browse files Browse the repository at this point in the history
* fix(dev): remove params for prerendered pages

* add test

* add changset

* deduplicate param removal

* format

* adjust tests
  • Loading branch information
lilnasy authored Feb 23, 2024
1 parent 3de7b2c commit 6aa660a
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/nice-hats-live.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fixes an issue where prerendered pages had access to query params in dev mode.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { APIContext } from 'astro';

export const prerender = false;

export const POST = async ({ request, redirect }: APIContext) => {
const formData = await request.formData();
const name = formData.get('name');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Layout from '../components/Layout.astro';
const method = Astro.url.searchParams.get('method') ?? 'POST';
const enctype = Astro.url.searchParams.get('enctype');
const postShowThrow = Astro.url.searchParams.has('throw') ?? false;
export const prerender = false;
---

<Layout>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
---
import Layout from '../components/Layout.astro';
const name = Astro.url.searchParams.get('name');
export const prerender = false;
---
<Layout>
<div>Submitted contact: <span id="contact-name">{name}</span></div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ if(Astro.request.method === 'POST') {
const name = formData.get('name');
return Astro.redirect(`/form-response?name=${name}`);
}
export const prerender = false;
---
<Layout>
<h2>Contact Form</h2>
Expand Down
11 changes: 10 additions & 1 deletion packages/astro/src/core/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface CreateRequestOptions {
logger: Logger;
ssr: boolean;
locals?: object | undefined;
removeParams?: boolean;
}

const clientAddressSymbol = Symbol.for('astro.clientAddress');
Expand All @@ -27,13 +28,21 @@ export function createRequest({
logger,
ssr,
locals,
removeParams = false,
}: CreateRequestOptions): Request {
let headersObj =
headers instanceof Headers
? headers
: new Headers(Object.entries(headers as Record<string, any>));

const request = new Request(url.toString(), {
if (typeof url === 'string') url = new URL(url);

// HACK! astro:assets uses query params for the injected route in `dev`
if (removeParams && url.pathname !== '/_image') {
url.search = '';
}

const request = new Request(url, {
method: method,
headers: headersObj,
body,
Expand Down
11 changes: 0 additions & 11 deletions packages/astro/src/vite-plugin-astro-server/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,6 @@ export async function handleRequest({
// Add config.base back to url before passing it to SSR
url.pathname = removeTrailingForwardSlash(config.base) + url.pathname;

// HACK! astro:assets uses query params for the injected route in `dev`
if (!buildingToSSR && pathname !== '/_image') {
// Prevent user from depending on search params when not doing SSR.
// NOTE: Create an array copy here because deleting-while-iterating
// creates bugs where not all search params are removed.
const allSearchParams = Array.from(url.searchParams);
for (const [key] of allSearchParams) {
url.searchParams.delete(key);
}
}

let body: ArrayBuffer | undefined = undefined;
if (!(incomingRequest.method === 'GET' || incomingRequest.method === 'HEAD')) {
let bytes: Uint8Array[] = [];
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ export async function handleRoute({
logger,
ssr: buildingToSSR,
clientAddress: buildingToSSR ? incomingRequest.socket.remoteAddress : undefined,
removeParams: buildingToSSR === false || route.prerender
});

// Set user specified headers to response object.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import * as assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { after, before, describe, it } from 'node:test';
import { fileURLToPath } from 'node:url';
import { createContainer } from '../../../dist/core/dev/container.js';
import { createLoader } from '../../../dist/core/module-loader/index.js';
import { createRouteManifest } from '../../../dist/core/routing/index.js';
import { createComponent, render } from '../../../dist/runtime/server/index.js';
import { createController, handleRequest } from '../../../dist/vite-plugin-astro-server/index.js';
import { DevPipeline } from '../../../dist/vite-plugin-astro-server/pipeline.js';
import { createDevelopmentManifest } from '../../../dist/vite-plugin-astro-server/plugin.js';
import testAdapter from '../../test-adapter.js';
import {
createAstroModule,
createBasicSettings,
Expand Down Expand Up @@ -73,4 +76,55 @@ describe('vite-plugin-astro-server', () => {
assert.equal(html.includes('<div id="test">'), true);
});
});

describe('url', () => {
let container;
let settings;

before(async () => {
const root = new URL('../../fixtures/api-routes/', import.meta.url);
const fileSystem = {
'/src/pages/url.astro': `{Astro.request.url}`,
'/src/pages/prerendered.astro': `---
export const prerender = true;
---
{Astro.request.url}`,
};
const fs = createFs(fileSystem, root);
settings = await createBasicSettings({
root: fileURLToPath(root),
output: 'server',
adapter: testAdapter(),
});
container = await createContainer({
fs,
settings,
logger: defaultLogger,
});
});

after(async () => {
await container.close();
});

it('params are included', async () => {
const { req, res, text } = createRequestAndResponse({
method: 'GET',
url: '/url?xyz=123',
});
container.handle(req, res);
const html = await text();
assert.deepEqual(html, '<!DOCTYPE html>http://undefined/url?xyz=123');
});

it('params are excluded on prerendered routes', async () => {
const { req, res, text } = createRequestAndResponse({
method: 'GET',
url: '/prerendered?xyz=123',
});
container.handle(req, res);
const html = await text();
assert.deepEqual(html, '<!DOCTYPE html>http://undefined/prerendered');
});
});
});

0 comments on commit 6aa660a

Please sign in to comment.