Skip to content

Commit

Permalink
Revert #12173 (#12206)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy authored Oct 12, 2024
1 parent c73d65d commit 12b0022
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 64 deletions.
5 changes: 5 additions & 0 deletions .changeset/gold-masks-trade.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Reverts https://github.com/withastro/astro/pull/12173 which caused `Can't modify immutable headers` warnings and 500 errors on Cloudflare Pages
3 changes: 2 additions & 1 deletion packages/astro/e2e/actions-blog.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ test.describe('Astro Actions - Blog', () => {
await expect(page).toHaveURL(astro.resolveUrl('/blog/'));
});

test('Should redirect to the origin pathname when there is a rewrite', async ({
// TODO: fix regression #12201 and #12202
test.skip('Should redirect to the origin pathname when there is a rewrite', async ({
page,
astro,
}) => {
Expand Down
7 changes: 0 additions & 7 deletions packages/astro/src/actions/runtime/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { yellow } from 'kleur/colors';
import type { APIContext, MiddlewareNext } from '../../@types/astro.js';
import { ASTRO_ORIGIN_HEADER } from '../../core/constants.js';
import { defineMiddleware } from '../../core/middleware/index.js';
import { getOriginHeader } from '../../core/routing/rewrite.js';
import { ACTION_QUERY_PARAMS } from '../consts.js';
import { formContentTypes, hasContentType } from './utils.js';
import { getAction } from './virtual/get-action.js';
Expand Down Expand Up @@ -137,11 +135,6 @@ async function redirectWithResult({
return context.redirect(referer);
}

const referer = getOriginHeader(context.request);
if (referer) {
return context.redirect(referer);
}

return context.redirect(context.url.pathname);
}

Expand Down
5 changes: 0 additions & 5 deletions packages/astro/src/core/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ export const REROUTE_DIRECTIVE_HEADER = 'X-Astro-Reroute';
* This metadata is used to determine the origin of a Response. If a rewrite has occurred, it should be prioritised over other logic.
*/
export const REWRITE_DIRECTIVE_HEADER_KEY = 'X-Astro-Rewrite';

/**
* Header used to track the original URL requested by the user. This information is useful rewrites are involved.
*/
export const ASTRO_ORIGIN_HEADER = 'X-Astro-Origin';
export const REWRITE_DIRECTIVE_HEADER_VALUE = 'yes';

/**
Expand Down
5 changes: 2 additions & 3 deletions packages/astro/src/core/middleware/sequence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type { APIContext, MiddlewareHandler, RewritePayload } from '../../@types
import { AstroCookies } from '../cookies/cookies.js';
import { apiContextRoutesSymbol } from '../render-context.js';
import { type Pipeline, getParams } from '../render/index.js';
import { copyRequest } from '../routing/rewrite.js';
import { defineMiddleware } from './index.js';

// From SvelteKit: https://github.com/sveltejs/kit/blob/master/packages/kit/src/exports/hooks/sequence.js
Expand Down Expand Up @@ -37,9 +36,9 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler {
if (payload instanceof Request) {
newRequest = payload;
} else if (payload instanceof URL) {
newRequest = copyRequest(payload, handleContext.request);
newRequest = new Request(payload, handleContext.request);
} else {
newRequest = copyRequest(
newRequest = new Request(
new URL(payload, handleContext.url.origin),
handleContext.request,
);
Expand Down
36 changes: 31 additions & 5 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
import { renderEndpoint } from '../runtime/server/endpoint.js';
import { renderPage } from '../runtime/server/index.js';
import {
ASTRO_ORIGIN_HEADER,
ASTRO_VERSION,
REROUTE_DIRECTIVE_HEADER,
REWRITE_DIRECTIVE_HEADER_KEY,
Expand All @@ -37,7 +36,6 @@ import { callMiddleware } from './middleware/callMiddleware.js';
import { sequence } from './middleware/index.js';
import { renderRedirect } from './redirects/render.js';
import { type Pipeline, Slots, getParams, getProps } from './render/index.js';
import { copyRequest, setOriginHeader } from './routing/rewrite.js';

export const apiContextRoutesSymbol = Symbol.for('context.routes');

Expand Down Expand Up @@ -83,7 +81,6 @@ export class RenderContext {
Pick<RenderContext, 'locals' | 'middleware' | 'status' | 'props'>
>): Promise<RenderContext> {
const pipelineMiddleware = await pipeline.getMiddleware();
setOriginHeader(request, pathname);
return new RenderContext(
pipeline,
locals,
Expand Down Expand Up @@ -156,7 +153,7 @@ export class RenderContext {
if (payload instanceof Request) {
this.request = payload;
} else {
this.request = copyRequest(newUrl, this.request);
this.request = this.#copyRequest(newUrl, this.request);
}
this.isRewriting = true;
this.url = new URL(this.request.url);
Expand Down Expand Up @@ -256,7 +253,7 @@ export class RenderContext {
if (reroutePayload instanceof Request) {
this.request = reroutePayload;
} else {
this.request = copyRequest(newUrl, this.request);
this.request = this.#copyRequest(newUrl, this.request);
}
this.url = new URL(this.request.url);
this.cookies = new AstroCookies(this.request);
Expand Down Expand Up @@ -564,4 +561,33 @@ export class RenderContext {
if (!i18n) return;
return (this.#preferredLocaleList ??= computePreferredLocaleList(request, i18n.locales));
}

/**
* Utility function that creates a new `Request` with a new URL from an old `Request`.
*
* @param newUrl The new `URL`
* @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,
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',
});
}
}
43 changes: 0 additions & 43 deletions packages/astro/src/core/routing/rewrite.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import type { AstroConfig, RewritePayload, RouteData } from '../../@types/astro.js';
import { shouldAppendForwardSlash } from '../build/util.js';
import { ASTRO_ORIGIN_HEADER } from '../constants.js';
import { AstroError, AstroErrorData } from '../errors/index.js';
import { appendForwardSlash, removeTrailingForwardSlash } from '../path.js';
import { DEFAULT_404_ROUTE } from './astro-designed-error-pages.js';

Expand Down Expand Up @@ -72,44 +70,3 @@ export function findRouteToRewrite({
}
}
}

/**
* Utility function that creates a new `Request` with a new URL from an old `Request`.
*
* @param newUrl The new `URL`
* @param oldRequest The old `Request`
*/
export function copyRequest(newUrl: URL, oldRequest: Request): Request {
if (oldRequest.bodyUsed) {
throw new AstroError(AstroErrorData.RewriteWithBodyUsed);
}
return new Request(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',
});
}

export function setOriginHeader(request: Request, pathname: string): void {
request.headers.set(ASTRO_ORIGIN_HEADER, encodeURIComponent(pathname));
}

export function getOriginHeader(request: Request): string | undefined {
const origin = request.headers.get(ASTRO_ORIGIN_HEADER);
if (origin) {
return decodeURIComponent(origin);
}
return undefined;
}

0 comments on commit 12b0022

Please sign in to comment.