summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Emanuele Stoppa <my.burning@gmail.com> 2024-10-11 14:47:14 +0100
committerGravatar GitHub <noreply@github.com> 2024-10-11 14:47:14 +0100
commit2d10de5f212323e6e19c7ea379826dcc18fe739c (patch)
tree7ecc61fda0e38217be0f254b65db06134e83e672
parenta4ffbfaa5cb460c12bd486fd75e36147f51d3e5e (diff)
downloadastro-2d10de5f212323e6e19c7ea379826dcc18fe739c.tar.gz
astro-2d10de5f212323e6e19c7ea379826dcc18fe739c.tar.zst
astro-2d10de5f212323e6e19c7ea379826dcc18fe739c.zip
fix(routing): actions should redirect the original pathname (#12173)
* fix(routing): actions should redirect the original pathname * decode header to avoid index out of bounds * fix e2e test * Update packages/astro/e2e/actions-blog.test.js Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com> --------- Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Diffstat
-rw-r--r--.changeset/tough-snakes-reflect.md5
-rw-r--r--packages/astro/e2e/actions-blog.test.js12
-rw-r--r--packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts10
-rw-r--r--packages/astro/e2e/fixtures/actions-blog/src/middleware.ts9
-rw-r--r--packages/astro/e2e/fixtures/actions-blog/src/pages/rewritten.astro18
-rw-r--r--packages/astro/src/actions/runtime/middleware.ts9
-rw-r--r--packages/astro/src/core/constants.ts5
-rw-r--r--packages/astro/src/core/middleware/sequence.ts5
-rw-r--r--packages/astro/src/core/render-context.ts36
-rw-r--r--packages/astro/src/core/routing/rewrite.ts43
10 files changed, 118 insertions, 34 deletions
diff --git a/.changeset/tough-snakes-reflect.md b/.changeset/tough-snakes-reflect.md
new file mode 100644
index 000000000..7fdaca603
--- /dev/null
+++ b/.changeset/tough-snakes-reflect.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a bug where Astro Actions couldn't redirect to the correct pathname when there was a rewrite involved.
diff --git a/packages/astro/e2e/actions-blog.test.js b/packages/astro/e2e/actions-blog.test.js
index d9c1bc1df..4b76928cf 100644
--- a/packages/astro/e2e/actions-blog.test.js
+++ b/packages/astro/e2e/actions-blog.test.js
@@ -135,4 +135,16 @@ test.describe('Astro Actions - Blog', () => {
await logoutButton.click();
await expect(page).toHaveURL(astro.resolveUrl('/blog/'));
});
+
+ test('Should redirect to the origin pathname when there is a rewrite', async ({
+ page,
+ astro,
+ }) => {
+ await page.goto(astro.resolveUrl('/sum'));
+ const submitButton = page.getByTestId('submit');
+ await submitButton.click();
+ await expect(page).toHaveURL(astro.resolveUrl('/sum'));
+ const p = page.locator('p').nth(0);
+ await expect(p).toContainText('Form result: {"data":3}');
+ });
});
diff --git a/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts b/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts
index 43ffb43d4..a5437ad49 100644
--- a/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts
+++ b/packages/astro/e2e/fixtures/actions-blog/src/actions/index.ts
@@ -56,4 +56,14 @@ export const server = {
},
}),
},
+ sum: defineAction({
+ accept: "form",
+ input: z.object({
+ a: z.number(),
+ b: z.number(),
+ }),
+ async handler({ a, b }) {
+ return a + b
+ },
+ })
};
diff --git a/packages/astro/e2e/fixtures/actions-blog/src/middleware.ts b/packages/astro/e2e/fixtures/actions-blog/src/middleware.ts
new file mode 100644
index 000000000..53bb8235a
--- /dev/null
+++ b/packages/astro/e2e/fixtures/actions-blog/src/middleware.ts
@@ -0,0 +1,9 @@
+import { defineMiddleware } from "astro:middleware";
+
+export const onRequest = defineMiddleware((ctx, next) => {
+ if (ctx.request.method === "GET" && ctx.url.pathname === "/sum") {
+ return next("/rewritten")
+ }
+
+ return next()
+})
diff --git a/packages/astro/e2e/fixtures/actions-blog/src/pages/rewritten.astro b/packages/astro/e2e/fixtures/actions-blog/src/pages/rewritten.astro
new file mode 100644
index 000000000..72eebf1bb
--- /dev/null
+++ b/packages/astro/e2e/fixtures/actions-blog/src/pages/rewritten.astro
@@ -0,0 +1,18 @@
+---
+import { actions } from "astro:actions";
+
+const result = Astro.getActionResult(actions.sum)
+
+---
+
+<html>
+ <body>
+ <form method="post" action={actions.sum}>
+ <input type="number" name="a" value="1" />
+ <input type="number" name="b" value="2" />
+ <button data-testid="submit" type="submit">Sum</button>
+ </form>
+ <p>Form result: {JSON.stringify(result)}</p>
+
+ <body>
+</html>
diff --git a/packages/astro/src/actions/runtime/middleware.ts b/packages/astro/src/actions/runtime/middleware.ts
index a61e1652c..fcd42b799 100644
--- a/packages/astro/src/actions/runtime/middleware.ts
+++ b/packages/astro/src/actions/runtime/middleware.ts
@@ -1,5 +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 { ACTION_QUERY_PARAMS } from '../consts.js';
import { formContentTypes, hasContentType } from './utils.js';
@@ -9,6 +10,7 @@ import {
type SerializedActionResult,
serializeActionResult,
} from './virtual/shared.js';
+import {getOriginHeader} from "../../core/routing/rewrite.js";
export type ActionPayload = {
actionResult: SerializedActionResult;
@@ -32,7 +34,7 @@ export const onRequest = defineMiddleware(async (context, next) => {
const locals = context.locals as Locals;
// Actions middleware may have run already after a path rewrite.
- // See https://github.com/withastro/roadmap/blob/feat/reroute/proposals/0047-rerouting.md#ctxrewrite
+ // See https://github.com/withastro/roadmap/blob/main/proposals/0048-rerouting.md#ctxrewrite
// `_actionPayload` is the same for every page,
// so short circuit if already defined.
if (locals._actionPayload) return next();
@@ -135,6 +137,11 @@ async function redirectWithResult({
return context.redirect(referer);
}
+ const referer = getOriginHeader(context.request);
+ if (referer) {
+ return context.redirect(referer);
+ }
+
return context.redirect(context.url.pathname);
}
diff --git a/packages/astro/src/core/constants.ts b/packages/astro/src/core/constants.ts
index 274f86797..3fb937ac4 100644
--- a/packages/astro/src/core/constants.ts
+++ b/packages/astro/src/core/constants.ts
@@ -28,6 +28,11 @@ 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';
/**
diff --git a/packages/astro/src/core/middleware/sequence.ts b/packages/astro/src/core/middleware/sequence.ts
index 03ae7d3a3..1e4c92f12 100644
--- a/packages/astro/src/core/middleware/sequence.ts
+++ b/packages/astro/src/core/middleware/sequence.ts
@@ -2,6 +2,7 @@ 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
@@ -36,9 +37,9 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler {
if (payload instanceof Request) {
newRequest = payload;
} else if (payload instanceof URL) {
- newRequest = new Request(payload, handleContext.request);
+ newRequest = copyRequest(payload, handleContext.request);
} else {
- newRequest = new Request(
+ newRequest = copyRequest(
new URL(payload, handleContext.url.origin),
handleContext.request,
);
diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
index 8850da132..a79c01922 100644
--- a/packages/astro/src/core/render-context.ts
+++ b/packages/astro/src/core/render-context.ts
@@ -20,6 +20,7 @@ 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,
@@ -36,6 +37,7 @@ 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');
@@ -81,6 +83,7 @@ export class RenderContext {
Pick<RenderContext, 'locals' | 'middleware' | 'status' | 'props'>
>): Promise<RenderContext> {
const pipelineMiddleware = await pipeline.getMiddleware();
+ setOriginHeader(request, pathname)
return new RenderContext(
pipeline,
locals,
@@ -153,7 +156,7 @@ export class RenderContext {
if (payload instanceof Request) {
this.request = payload;
} else {
- this.request = this.#copyRequest(newUrl, this.request);
+ this.request = copyRequest(newUrl, this.request);
}
this.isRewriting = true;
this.url = new URL(this.request.url);
@@ -253,7 +256,7 @@ export class RenderContext {
if (reroutePayload instanceof Request) {
this.request = reroutePayload;
} else {
- this.request = this.#copyRequest(newUrl, this.request);
+ this.request = copyRequest(newUrl, this.request);
}
this.url = new URL(this.request.url);
this.cookies = new AstroCookies(this.request);
@@ -561,33 +564,4 @@ 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',
- });
- }
}
diff --git a/packages/astro/src/core/routing/rewrite.ts b/packages/astro/src/core/routing/rewrite.ts
index d50434f22..29d84ce74 100644
--- a/packages/astro/src/core/routing/rewrite.ts
+++ b/packages/astro/src/core/routing/rewrite.ts
@@ -1,7 +1,9 @@
import type { AstroConfig, RewritePayload, RouteData } from '../../@types/astro.js';
import { shouldAppendForwardSlash } from '../build/util.js';
+import { AstroError, AstroErrorData } from '../errors/index.js';
import { appendForwardSlash, removeTrailingForwardSlash } from '../path.js';
import { DEFAULT_404_ROUTE } from './astro-designed-error-pages.js';
+import {ASTRO_ORIGIN_HEADER} from "../constants.js";
export type FindRouteToRewrite = {
payload: RewritePayload;
@@ -70,3 +72,44 @@ 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
+}
generated by cgit v1.2.3 (git 2.46.0) at 2025-08-10 05:39:57 +0000