Skip to content

Commit

Permalink
fix(routing): don't trigger get of headers (#12937)
Browse files Browse the repository at this point in the history
* fix(routing): don't trigger get of headers

* remove bad copy-paste
  • Loading branch information
ematipico authored Jan 8, 2025
1 parent dbb04f3 commit 30edb6d
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/sweet-zoos-move.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes a bug where users could use `Astro.request.headers` during a rewrite inside prerendered routes. This an invalid behaviour, and now Astro will show a warning if this happens.
5 changes: 5 additions & 0 deletions .changeset/tame-pianos-approve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes an issue where the use of `Astro.rewrite` would trigger the invalid use of `Astro.request.headers`
18 changes: 16 additions & 2 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,14 @@ export class RenderContext {
if (payload instanceof Request) {
this.request = payload;
} else {
this.request = copyRequest(newUrl, this.request);
this.request = copyRequest(
newUrl,
this.request,
// need to send the flag of the previous routeData
routeData.prerender,
this.pipeline.logger,
this.routeData.route,
);
}
this.isRewriting = true;
this.url = new URL(this.request.url);
Expand Down Expand Up @@ -290,7 +297,14 @@ export class RenderContext {
if (reroutePayload instanceof Request) {
this.request = reroutePayload;
} else {
this.request = copyRequest(newUrl, this.request);
this.request = copyRequest(
newUrl,
this.request,
// need to send the flag of the previous routeData
routeData.prerender,
this.pipeline.logger,
this.routeData.route,
);
}
this.url = new URL(this.request.url);
this.cookies = new AstroCookies(this.request);
Expand Down
14 changes: 12 additions & 2 deletions packages/astro/src/core/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@ import type { IncomingHttpHeaders } from 'node:http';
import type { Logger } from './logger/core.js';

type HeaderType = Headers | Record<string, any> | IncomingHttpHeaders;
type RequestBody = ArrayBuffer | Blob | ReadableStream | URLSearchParams | FormData;
type RequestBody =
| ArrayBuffer
| Blob
| ReadableStream
| URLSearchParams
| FormData
| ReadableStream<Uint8Array>;

export interface CreateRequestOptions {
url: URL | string;
clientAddress?: string | undefined;
headers: HeaderType;
method?: string;
body?: RequestBody | undefined;
body?: RequestBody | undefined | null;
logger: Logger;
locals?: object | undefined;
/**
Expand All @@ -22,6 +28,8 @@ export interface CreateRequestOptions {
isPrerendered?: boolean;

routePattern: string;

init?: RequestInit;
}

/**
Expand All @@ -39,6 +47,7 @@ export function createRequest({
logger,
isPrerendered = false,
routePattern,
init,
}: CreateRequestOptions): Request {
// headers are made available on the created request only if the request is for a page that will be on-demand rendered
const headersObj = isPrerendered
Expand All @@ -65,6 +74,7 @@ export function createRequest({
headers: headersObj,
// body is made available only if the request is for a page that will be on-demand rendered
body: isPrerendered ? null : body,
...init,
});

if (isPrerendered) {
Expand Down
47 changes: 32 additions & 15 deletions packages/astro/src/core/routing/rewrite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import type { RouteData } from '../../types/public/internal.js';
import { shouldAppendForwardSlash } from '../build/util.js';
import { originPathnameSymbol } from '../constants.js';
import { AstroError, AstroErrorData } from '../errors/index.js';
import type { Logger } from '../logger/core.js';
import { appendForwardSlash, removeTrailingForwardSlash } from '../path.js';
import { createRequest } from '../request.js';
import { DEFAULT_404_ROUTE } from './astro-designed-error-pages.js';

export type FindRouteToRewrite = {
Expand Down Expand Up @@ -80,27 +82,42 @@ export function findRouteToRewrite({
*
* @param newUrl The new `URL`
* @param oldRequest The old `Request`
* @param isPrerendered It needs to be the flag of the previous routeData, before the rewrite
* @param logger
* @param routePattern
*/
export function copyRequest(newUrl: URL, oldRequest: Request): Request {
export function copyRequest(
newUrl: URL,
oldRequest: Request,
isPrerendered: boolean,
logger: Logger,
routePattern: string,
): Request {
if (oldRequest.bodyUsed) {
throw new AstroError(AstroErrorData.RewriteWithBodyUsed);
}
return new Request(newUrl, {
return createRequest({
url: newUrl,
method: oldRequest.method,
headers: oldRequest.headers,
body: oldRequest.body,
referrer: oldRequest.referrer,
referrerPolicy: oldRequest.referrerPolicy,
mode: oldRequest.mode,
credentials: oldRequest.credentials,
cache: oldRequest.cache,
redirect: oldRequest.redirect,
integrity: oldRequest.integrity,
signal: oldRequest.signal,
keepalive: oldRequest.keepalive,
// https://fetch.spec.whatwg.org/#dom-request-duplex
// @ts-expect-error It isn't part of the types, but undici accepts it and it allows to carry over the body to a new request
duplex: 'half',
isPrerendered,
logger,
headers: isPrerendered ? {} : oldRequest.headers,
routePattern,
init: {
referrer: oldRequest.referrer,
referrerPolicy: oldRequest.referrerPolicy,
mode: oldRequest.mode,
credentials: oldRequest.credentials,
cache: oldRequest.cache,
redirect: oldRequest.redirect,
integrity: oldRequest.integrity,
signal: oldRequest.signal,
keepalive: oldRequest.keepalive,
// https://fetch.spec.whatwg.org/#dom-request-duplex
// @ts-expect-error It isn't part of the types, but undici accepts it and it allows to carry over the body to a new request
duplex: 'half',
},
});
}

Expand Down

0 comments on commit 30edb6d

Please sign in to comment.