From 12495bef6f55d4ae5701f084e375298518a1e05a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20L=C3=AA?= <115204145+DucMinhNe@users.noreply.github.com> Date: Fri, 5 Jun 2026 16:48:27 +0700 Subject: [PATCH 1/3] =?UTF-8?q?fix(errors):=20correct=20param=20key=20in?= =?UTF-8?q?=20getStaticPaths=20error-doc=20examples=20(slug=20=E2=86=92=20?= =?UTF-8?q?id)=20(#16976)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/astro/src/core/errors/errors-data.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index 01d15fca2878..bf42ccc778cf 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -223,8 +223,8 @@ export const NoClientOnlyHint = { * --- * export async function getStaticPaths() { * return [ - * { params: { slug: "blog" } }, - * { params: { slug: "about" } } + * { params: { id: "blog" } }, + * { params: { id: "about" } } * ]; *} *--- @@ -247,8 +247,8 @@ export const InvalidGetStaticPathParam = { * ```ts title="pages/blog/[id].astro" * export async function getStaticPaths() { * return [ // <-- Array - * { params: { slug: "blog" } }, // <-- Object - * { params: { slug: "about" } } + * { params: { id: "blog" } }, // <-- Object + * { params: { id: "about" } } * ]; *} * ``` @@ -271,8 +271,8 @@ export const InvalidGetStaticPathsEntry = { * ```ts title="pages/blog/[id].astro" * export async function getStaticPaths() { * return [ // <-- Array - * { params: { slug: "blog" } }, - * { params: { slug: "about" } } + * { params: { id: "blog" } }, + * { params: { id: "about" } } * ]; *} * ``` From 7a5c00169ecc3a1ceccf27f457818c04bcc5d12b Mon Sep 17 00:00:00 2001 From: "Houston (Bot)" <108291165+astrobot-houston@users.noreply.github.com> Date: Fri, 5 Jun 2026 05:29:05 -0700 Subject: [PATCH 2/3] fix(cloudflare): defer createApp/setGetEnv in fetch.ts to avoid circular import crash (#16956) (#16968) --- .../fix-cloudflare-fetch-circular-import.md | 5 ++ packages/integrations/cloudflare/src/fetch.ts | 18 +++-- .../cloudflare/test/fetch-lazy-init.test.ts | 68 +++++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 .changeset/fix-cloudflare-fetch-circular-import.md create mode 100644 packages/integrations/cloudflare/test/fetch-lazy-init.test.ts diff --git a/.changeset/fix-cloudflare-fetch-circular-import.md b/.changeset/fix-cloudflare-fetch-circular-import.md new file mode 100644 index 000000000000..0c250300ad7a --- /dev/null +++ b/.changeset/fix-cloudflare-fetch-circular-import.md @@ -0,0 +1,5 @@ +--- +'@astrojs/cloudflare': patch +--- + +Fixes a build crash when using `experimental.advancedRouting` with a custom `fetchFile` that statically imports `cf` from `@astrojs/cloudflare/fetch`. The circular dependency between `@astrojs/cloudflare/fetch` and `astro/app/entrypoint` caused `createApp` or `createGetEnv` to be `undefined` at module evaluation time. Initialization is now deferred to the first `cf()` call, breaking the cycle. diff --git a/packages/integrations/cloudflare/src/fetch.ts b/packages/integrations/cloudflare/src/fetch.ts index a46bec71a0f7..242eb9cd24d4 100644 --- a/packages/integrations/cloudflare/src/fetch.ts +++ b/packages/integrations/cloudflare/src/fetch.ts @@ -31,9 +31,18 @@ import { getClientAddress, } from './utils/cf.js'; -setGetEnv(createGetEnv(globalEnv)); +// Lazy initialization — `createApp` and `setGetEnv` are deferred to the +// first `cf()` call so that this module can be statically imported from a +// custom `fetchFile` without triggering a circular-dependency crash. +// (The cycle: fetch.ts → astro/app/entrypoint → virtual:astro:fetchable → user worker → fetch.ts) +let app: ReturnType | undefined; -const app = createApp(); +function ensureInitialized() { + if (!app) { + setGetEnv(createGetEnv(globalEnv)); + app = createApp(); + } +} /** * Applies Cloudflare-specific setup to a `FetchState`: @@ -50,9 +59,10 @@ export async function cf( env: Env, ctx: ExecutionContext, ): Promise { - injectSessionBinding(app.manifest, env); + ensureInitialized(); + injectSessionBinding(app!.manifest, env); - const staticAsset = matchStaticAsset(app.manifest, state.request.url, env); + const staticAsset = matchStaticAsset(app!.manifest, state.request.url, env); if (staticAsset) return staticAsset; if (!state.routeData) { diff --git a/packages/integrations/cloudflare/test/fetch-lazy-init.test.ts b/packages/integrations/cloudflare/test/fetch-lazy-init.test.ts new file mode 100644 index 000000000000..9d967d6c0b5b --- /dev/null +++ b/packages/integrations/cloudflare/test/fetch-lazy-init.test.ts @@ -0,0 +1,68 @@ +import * as assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { describe, it } from 'node:test'; + +/** + * `@astrojs/cloudflare/fetch` must not call `createApp()` or `setGetEnv()` + * at module top-level. Eager calls cause a circular-dependency crash when + * a user's custom `fetchFile` (`src/worker.ts`) statically imports `cf` + * from `@astrojs/cloudflare/fetch`. + * + * The cycle: fetch.ts → astro/app/entrypoint → virtual:astro:fetchable + * → user worker → fetch.ts + * + * See: https://github.com/withastro/astro/issues/16956 + */ +describe('@astrojs/cloudflare/fetch lazy initialization', () => { + const source = readFileSync(new URL('../dist/fetch.js', import.meta.url), 'utf-8'); + + it('does not call createApp() at module top-level', () => { + // After the fix, createApp() should only appear inside the + // ensureInitialized() function body, not as a bare top-level statement. + // We check that there is no top-level `createApp()` by verifying the + // pattern does NOT appear outside a function body. + // A simple heuristic: the call `= createApp()` should not exist as a + // top-level const/let/var assignment (the old pattern was `const app = createApp()`). + const topLevelCreateApp = /^(?:const|let|var)\s+\w+\s*=\s*createApp\(\)/m; + assert.equal( + topLevelCreateApp.test(source), + false, + 'createApp() must not be called at module top-level (causes circular import crash)', + ); + }); + + it('does not call setGetEnv() at module top-level', () => { + // The old pattern was a bare `setGetEnv(createGetEnv(globalEnv))` statement + // at the top level. After the fix this should only appear inside ensureInitialized(). + // A line starting with `setGetEnv(` (not inside a function) is the problem pattern. + const lines = source.split('\n'); + const topLevelSetGetEnv = lines.some((line) => { + const trimmed = line.trim(); + // Skip lines inside function bodies (indented or after opening brace) + // A simple heuristic: if the line starts with setGetEnv and is not indented, + // it's top-level. + return trimmed.startsWith('setGetEnv(') && !line.startsWith(' ') && !line.startsWith('\t'); + }); + assert.equal( + topLevelSetGetEnv, + false, + 'setGetEnv() must not be called at module top-level (causes circular import crash)', + ); + }); + + it('exports ensureInitialized or calls it inside cf()', () => { + // Verify that lazy init is wired into the cf() function + assert.ok( + source.includes('ensureInitialized()'), + 'cf() should call ensureInitialized() for lazy setup', + ); + }); + + it('exports a cf function', () => { + // Sanity check: the module still exports cf + assert.ok( + source.includes('export {') && source.includes('cf'), + 'Module should export the cf function', + ); + }); +}); From e0703a6e815be829759ab7912f7024ee8424c3ac Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 5 Jun 2026 15:19:04 +0100 Subject: [PATCH 3/3] fix(fetch): sync request.url with forwarded headers in FetchState (#16947) * fix(fetch): sync request.url with forwarded headers in FetchState (#16945) * chore: improve tests and solution --------- Co-authored-by: astrobot-houston --- .changeset/kind-snails-wash.md | 5 + packages/astro/src/core/fetch/fetch-state.ts | 26 +++- packages/astro/test/units/fetch/index.test.ts | 116 +++++++++++++++++- 3 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 .changeset/kind-snails-wash.md diff --git a/.changeset/kind-snails-wash.md b/.changeset/kind-snails-wash.md new file mode 100644 index 000000000000..617b736c118f --- /dev/null +++ b/.changeset/kind-snails-wash.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes `Astro.request.url` not reflecting validated `X-Forwarded-Proto`/`X-Forwarded-Host` headers when `security.allowedDomains` is configured. Previously, only `Astro.url` was updated with the forwarded origin while `Astro.request.url` retained the socket-derived URL, causing the two to diverge behind TLS-terminating proxies. diff --git a/packages/astro/src/core/fetch/fetch-state.ts b/packages/astro/src/core/fetch/fetch-state.ts index 729a169cf82b..935596a29e42 100644 --- a/packages/astro/src/core/fetch/fetch-state.ts +++ b/packages/astro/src/core/fetch/fetch-state.ts @@ -14,6 +14,7 @@ import type { RouteData, SSRResult } from '../../types/public/internal.js'; import { AstroCookies } from '../cookies/index.js'; import { type Pipeline, Slots } from '../render/index.js'; import { + appSymbol, ASTRO_GENERATOR, fetchStateSymbol, originPathnameSymbol, @@ -308,10 +309,12 @@ export class FetchState implements AstroFetchState { this.#applyForwardedHeaders(); } - // Set origin pathname for rewrite tracking. - if (!Reflect.get(request, originPathnameSymbol)) { + // Set origin pathname for rewrite tracking. Use this.request + // (not the local parameter) because #applyForwardedHeaders() + // may have reconstructed it with a forwarded URL. + if (!Reflect.get(this.request, originPathnameSymbol)) { setOriginPathname( - request, + this.request, this.pathname, pipeline.manifest.trailingSlash, pipeline.manifest.buildFormat, @@ -956,6 +959,23 @@ export class FetchState implements AstroFetchState { this.clientAddress = forwardedFor; } } + + // Reconstruct the Request with the resolved URL so that + // request.url stays in sync with this.url. Request.url is a + // readonly string, so we must create a new Request object. The + // constructor carries over method, headers, body (incl. stream + + // duplex) and signal from the old request. + const oldRequest = this.request; + this.request = new Request(this.url, oldRequest); + // Re-attach `appSymbol`: the rest of the pipeline resolves the app + // via `getApp(state.request)` (see core/fetch/index.ts), so the new + // Request must carry it. We copy only this known Astro symbol. + // Other request-bound state is either already captured on + // `this` (clientAddress) or set after this point (originPathname). + const app = Reflect.get(oldRequest, appSymbol); + if (app !== undefined) { + Reflect.set(this.request, appSymbol, app); + } } /** diff --git a/packages/astro/test/units/fetch/index.test.ts b/packages/astro/test/units/fetch/index.test.ts index 3b37f6410ee6..70afad3dca69 100644 --- a/packages/astro/test/units/fetch/index.test.ts +++ b/packages/astro/test/units/fetch/index.test.ts @@ -831,8 +831,6 @@ describe('FetchState X-Forwarded-* header resolution', () => { }); it('handles headers set by user fetch handler before FetchState creation', () => { - // This is the core use case from the issue: user sets forwarded - // headers in their src/app.ts fetch() before creating FetchState. const app = createTestApp([createPage(simplePage, { route: '/' })], { allowedDomains: [{ hostname: 'example.com' }], }); @@ -849,8 +847,6 @@ describe('FetchState X-Forwarded-* header resolution', () => { }); it('renders through the full pipeline with forwarded headers applied', async () => { - // End-to-end: forwarded headers should be visible in Astro.url - // when rendering through the astro() combined handler. const app = createTestApp([createPage(simplePage, { route: '/' })], { allowedDomains: [{ hostname: 'example.com' }], }); @@ -865,13 +861,123 @@ describe('FetchState X-Forwarded-* header resolution', () => { ); const state = new FetchState(request); - // Verify URL was updated before the handler runs assert.equal(state.url.protocol, 'https:'); assert.equal(state.url.hostname, 'example.com'); const response = await astro(state); assert.equal(response.status, 200); }); + + it('updates request.url to match the forwarded URL', () => { + const app = createTestApp([createPage(simplePage, { route: '/' })], { + allowedDomains: [{ hostname: 'example.com' }], + }); + const request = stampApp( + new Request('http://localhost:4321/page', { + headers: { + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'example.com', + }, + }), + app, + ); + const state = new FetchState(request); + + assert.equal(state.url.protocol, 'https:'); + assert.equal(state.url.hostname, 'example.com'); + + const requestUrl = new URL(state.request.url); + assert.equal(requestUrl.protocol, 'https:'); + assert.equal(requestUrl.hostname, 'example.com'); + assert.equal(requestUrl.pathname, '/page'); + }); + + it('does not reconstruct request when no forwarded headers are validated', () => { + const app = createTestApp([createPage(simplePage, { route: '/' })], { + allowedDomains: [{ hostname: 'trusted.com' }], + }); + const original = new Request('http://localhost:4321/', { + headers: { + 'x-forwarded-host': 'evil.com', + }, + }); + const request = stampApp(original, app); + const state = new FetchState(request); + + // Rejected forwarded host — request should stay unchanged + assert.equal(state.url.hostname, 'localhost'); + assert.equal(new URL(state.request.url).hostname, 'localhost'); + }); + + it('carries appSymbol onto the reconstructed request so the app still resolves', async () => { + const app = createTestApp([createPage(simplePage, { route: '/' })], { + allowedDomains: [{ hostname: 'example.com' }], + }); + const original = new Request('http://localhost:4321/', { + headers: { + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'example.com', + }, + }); + const request = stampApp(original, app); + const state = new FetchState(request); + + assert.notEqual(state.request, original); + assert.equal(Reflect.get(state.request, appSymbol), app); + const response = await astro(state); + assert.equal(response.status, 200); + }); + + it('preserves method, headers and body when reconstructing for forwarded headers', async () => { + const app = createTestApp([createPage(simplePage, { route: '/api' })], { + allowedDomains: [{ hostname: 'example.com' }], + }); + const request = stampApp( + new Request('http://localhost:4321/api', { + method: 'POST', + headers: { + 'content-type': 'application/x-www-form-urlencoded', + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'example.com', + }, + body: 'token=abc123', + }), + app, + ); + const state = new FetchState(request); + + assert.equal(new URL(state.request.url).protocol, 'https:'); + assert.equal(new URL(state.request.url).hostname, 'example.com'); + assert.equal(state.request.method, 'POST'); + assert.equal(state.request.headers.get('content-type'), 'application/x-www-form-urlencoded'); + assert.equal(await state.request.text(), 'token=abc123'); + }); + + it('preserves a streaming request body (duplex) when reconstructing', async () => { + const app = createTestApp([createPage(simplePage, { route: '/api' })], { + allowedDomains: [{ hostname: 'example.com' }], + }); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('chunked-payload')); + controller.close(); + }, + }); + const init: any = { + method: 'POST', + headers: { + 'x-forwarded-proto': 'https', + 'x-forwarded-host': 'example.com', + }, + body, + duplex: 'half', + }; + const request = stampApp(new Request('http://localhost:4321/api', init), app); + const state = new FetchState(request); + + assert.equal(new URL(state.request.url).hostname, 'example.com'); + assert.equal(await state.request.text(), 'chunked-payload'); + }); }); // #endregion