diff --git a/.changeset/wild-geckos-draw.md b/.changeset/wild-geckos-draw.md new file mode 100644 index 0000000000..2965af1599 --- /dev/null +++ b/.changeset/wild-geckos-draw.md @@ -0,0 +1,9 @@ +--- +'astro': patch +--- + +Fix use of cloned requests in middleware with clientAddress + +When using `context.clientAddress` or `Astro.clientAddress` Astro looks up the address in a hidden property. Cloning a request can cause this hidden property to be lost. + +The fix is to pass the address as an internal property instead, decoupling it from the request. diff --git a/packages/astro/src/container/index.ts b/packages/astro/src/container/index.ts index 2564e28750..c762773d3b 100644 --- a/packages/astro/src/container/index.ts +++ b/packages/astro/src/container/index.ts @@ -508,6 +508,7 @@ export class experimental_AstroContainer { pathname: url.pathname, locals: options?.locals ?? {}, partial: options?.partial ?? true, + clientAddress: '' }); if (options.params) { renderContext.params = options.params; diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 09b3edea12..da5f917c1f 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -72,6 +72,7 @@ export interface RenderErrorOptions { * Allows passing an error to 500.astro. It will be available through `Astro.props.error`. */ error?: unknown; + clientAddress: string | undefined; } export class App { @@ -240,7 +241,7 @@ export class App { let addCookieHeader: boolean | undefined; addCookieHeader = renderOptions?.addCookieHeader; - clientAddress = renderOptions?.clientAddress; + clientAddress = renderOptions?.clientAddress ?? Reflect.get(request,clientAddressSymbol); routeData = renderOptions?.routeData; locals = renderOptions?.locals; @@ -256,13 +257,10 @@ export class App { if (typeof locals !== 'object') { const error = new AstroError(AstroErrorData.LocalsNotAnObject); this.#logger.error(null, error.stack!); - return this.#renderError(request, { status: 500, error }); + return this.#renderError(request, { status: 500, error, clientAddress }); } Reflect.set(request, clientLocalsSymbol, locals); } - if (clientAddress) { - Reflect.set(request, clientAddressSymbol, clientAddress); - } if (!routeData) { routeData = this.match(request); this.#logger.debug('router', 'Astro matched the following route for ' + request.url); @@ -271,7 +269,7 @@ export class App { if (!routeData) { this.#logger.debug('router', "Astro hasn't found routes that match " + request.url); this.#logger.debug('router', "Here's the available routes:\n", this.#manifestData); - return this.#renderError(request, { locals, status: 404 }); + return this.#renderError(request, { locals, status: 404, clientAddress }); } const pathname = this.#getPathnameFromRequest(request); const defaultStatus = this.#getDefaultStatusCode(routeData, pathname); @@ -288,11 +286,12 @@ export class App { request, routeData, status: defaultStatus, + clientAddress }); response = await renderContext.render(await mod.page()); } catch (err: any) { this.#logger.error(null, err.stack || err.message || String(err)); - return this.#renderError(request, { locals, status: 500, error: err }); + return this.#renderError(request, { locals, status: 500, error: err, clientAddress }); } if ( @@ -306,6 +305,7 @@ export class App { // We don't have an error to report here. Passing null means we pass nothing intentionally // while undefined means there's no error error: response.status === 500 ? null : undefined, + clientAddress }); } @@ -353,6 +353,7 @@ export class App { response: originalResponse, skipMiddleware = false, error, + clientAddress, }: RenderErrorOptions, ): Promise { const errorRoutePath = `/${status}${this.#manifest.trailingSlash === 'always' ? '/' : ''}`; @@ -386,6 +387,7 @@ export class App { routeData: errorRouteData, status, props: { error }, + clientAddress }); const response = await renderContext.render(await mod.page()); return this.#mergeResponses(response, originalResponse); @@ -397,6 +399,7 @@ export class App { status, response: originalResponse, skipMiddleware: true, + clientAddress }); } } diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 5bdd6dc2f4..6fe6351cb6 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -444,6 +444,7 @@ async function generatePath( pathname: pathname, request, routeData: route, + clientAddress: undefined }); let body: string | Uint8Array; diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 611b9cbcbe..614777ac5f 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -48,6 +48,7 @@ export class RenderContext { public request: Request, public routeData: RouteData, public status: number, + public clientAddress: string | undefined, protected cookies = new AstroCookies(request), public params = getParams(routeData, pathname), protected url = new URL(request.url), @@ -71,10 +72,11 @@ export class RenderContext { pipeline, request, routeData, + clientAddress, status = 200, props, partial = undefined, - }: Pick & + }: Pick & Partial< Pick >): Promise { @@ -88,6 +90,7 @@ export class RenderContext { request, routeData, status, + clientAddress, undefined, undefined, undefined, @@ -309,7 +312,7 @@ export class RenderContext { routePattern: this.routeData.route, isPrerendered: this.routeData.prerender, get clientAddress() { - return renderContext.clientAddress(); + return renderContext.getClientAddress(); }, get currentLocale() { return renderContext.computeCurrentLocale(); @@ -490,7 +493,7 @@ export class RenderContext { isPrerendered: this.routeData.prerender, cookies, get clientAddress() { - return renderContext.clientAddress(); + return renderContext.getClientAddress(); }, get currentLocale() { return renderContext.computeCurrentLocale(); @@ -519,14 +522,22 @@ export class RenderContext { }; } - clientAddress() { - const { pipeline, request } = this; - if (clientAddressSymbol in request) { - return Reflect.get(request, clientAddressSymbol) as string; + getClientAddress() { + const { pipeline, request, routeData, clientAddress } = this; + + if(routeData.prerender) { + throw new AstroError(AstroErrorData.PrerenderClientAddressNotAvailable); } - if (request.body === null) { - throw new AstroError(AstroErrorData.PrerenderClientAddressNotAvailable); + if(clientAddress) { + return clientAddress; + } + + // TODO: Legacy, should not need to get here. + // Some adapters set this symbol so we can't remove support yet. + // Adapters should be updated to provide it via RenderOptions instead. + if (clientAddressSymbol in request) { + return Reflect.get(request, clientAddressSymbol) as string; } if (pipeline.adapterName) { diff --git a/packages/astro/src/core/request.ts b/packages/astro/src/core/request.ts index 71e7da83c9..b8cc89fb84 100644 --- a/packages/astro/src/core/request.ts +++ b/packages/astro/src/core/request.ts @@ -24,7 +24,6 @@ export interface CreateRequestOptions { routePattern: string; } -const clientAddressSymbol = Symbol.for('astro.clientAddress'); const clientLocalsSymbol = Symbol.for('astro.locals'); /** @@ -37,7 +36,6 @@ const clientLocalsSymbol = Symbol.for('astro.locals'); export function createRequest({ url, headers, - clientAddress, method = 'GET', body = undefined, logger, @@ -93,9 +91,6 @@ export function createRequest({ _headers = newHeaders; }, }); - } else if (clientAddress) { - // clientAddress is stored to be read by RenderContext, only if the request is for a page that will be on-demand rendered - Reflect.set(request, clientAddressSymbol, clientAddress); } Reflect.set(request, clientLocalsSymbol, locals ?? {}); diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index eb06846499..3498571e87 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -172,7 +172,6 @@ export async function handleRoute({ method: incomingRequest.method, body, logger, - clientAddress: incomingRequest.socket.remoteAddress, isPrerendered: route.prerender, routePattern: route.component, }); @@ -191,6 +190,7 @@ export async function handleRoute({ middleware: isDefaultPrerendered404(matchedRoute.route) ? undefined : middleware, request, routeData: route, + clientAddress: incomingRequest.socket.remoteAddress }); let response; @@ -248,6 +248,7 @@ export async function handleRoute({ middleware: isDefaultPrerendered404(fourOhFourRoute.route) ? undefined : middleware, request, routeData: fourOhFourRoute.route, + clientAddress: incomingRequest.socket.remoteAddress }); response = await renderContext.render(fourOhFourRoute.preloadedComponent); } diff --git a/packages/astro/test/fixtures/client-address/src/middleware.ts b/packages/astro/test/fixtures/client-address/src/middleware.ts new file mode 100644 index 0000000000..9709a5595f --- /dev/null +++ b/packages/astro/test/fixtures/client-address/src/middleware.ts @@ -0,0 +1,12 @@ +import { defineMiddleware } from 'astro:middleware'; + +export const onRequest = defineMiddleware(async (ctx, next) => { + // Clone a request, losing all symbols + const clonedRequest = ctx.request.clone(); + const safeInternalRequest = new Request(clonedRequest, { + method: clonedRequest.method, + headers: clonedRequest.headers, + }); + + return next(safeInternalRequest); +});