summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--.changeset/curvy-trees-compare.md5
-rw-r--r--packages/astro/src/core/app/index.ts1
-rw-r--r--packages/astro/src/core/render-context.ts6
-rw-r--r--packages/astro/src/i18n/middleware.ts32
-rw-r--r--packages/astro/src/vite-plugin-astro-server/route.ts8
-rw-r--r--packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/404.astro4
-rw-r--r--packages/astro/test/i18n-routing.test.js16
-rw-r--r--packages/astro/test/units/routing/endpoints.test.js9
8 files changed, 61 insertions, 20 deletions
diff --git a/.changeset/curvy-trees-compare.md b/.changeset/curvy-trees-compare.md
new file mode 100644
index 000000000..0580ab6d7
--- /dev/null
+++ b/.changeset/curvy-trees-compare.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fixes an issue where `404.astro` was ignored with `i18n` routing enabled.
diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
index 2c334223f..7c8d0067a 100644
--- a/packages/astro/src/core/app/index.ts
+++ b/packages/astro/src/core/app/index.ts
@@ -320,6 +320,7 @@ export class App {
});
}
+ // We remove internally-used header before we send the response to the user agent.
if (response.headers.has(REROUTE_DIRECTIVE_HEADER)) {
response.headers.delete(REROUTE_DIRECTIVE_HEADER);
}
diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
index 50aa7b710..1bf5d652a 100644
--- a/packages/astro/src/core/render-context.ts
+++ b/packages/astro/src/core/render-context.ts
@@ -13,6 +13,7 @@ import { renderEndpoint } from '../runtime/server/endpoint.js';
import { renderPage } from '../runtime/server/index.js';
import {
ASTRO_VERSION,
+ REROUTE_DIRECTIVE_HEADER,
ROUTE_TYPE_HEADER,
clientAddressSymbol,
clientLocalsSymbol,
@@ -102,7 +103,12 @@ export class RenderContext {
streaming,
routeData
);
+ // Signal to the i18n middleware to maybe act on this response
response.headers.set(ROUTE_TYPE_HEADER, 'page');
+ // Signal to the error-page-rerouting infra to let this response pass through to avoid loops
+ if (routeData.route === "/404" || routeData.route === "/500") {
+ response.headers.set(REROUTE_DIRECTIVE_HEADER, "no")
+ }
return response;
}
: type === 'fallback'
diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts
index 2098ffc7a..4affbf931 100644
--- a/packages/astro/src/i18n/middleware.ts
+++ b/packages/astro/src/i18n/middleware.ts
@@ -2,7 +2,7 @@ import { appendForwardSlash, joinPaths } from '@astrojs/internal-helpers/path';
import type { APIContext, Locales, MiddlewareHandler, SSRManifest } from '../@types/astro.js';
import type { SSRManifestI18n } from '../core/app/types.js';
import { shouldAppendForwardSlash } from '../core/build/util.js';
-import { ROUTE_TYPE_HEADER } from '../core/constants.js';
+import { REROUTE_DIRECTIVE_HEADER, ROUTE_TYPE_HEADER } from '../core/constants.js';
import { getPathByLocale, normalizeTheLocale } from './index.js';
// Checks if the pathname has any locale, exception for the defaultLocale, which is ignored on purpose.
@@ -44,12 +44,9 @@ export function createI18nMiddleware(
}
}
- // Astro can't know where the default locale is supposed to be, so it returns a 404 with no content.
+ // Astro can't know where the default locale is supposed to be, so it returns a 404.
else if (!pathnameHasLocale(url.pathname, i18n.locales)) {
- return new Response(null, {
- status: 404,
- headers: response.headers,
- });
+ return notFound(response);
}
return undefined;
@@ -66,10 +63,7 @@ export function createI18nMiddleware(
if (pathnameContainsDefaultLocale) {
const newLocation = url.pathname.replace(`/${i18n.defaultLocale}`, '');
response.headers.set('Location', newLocation);
- return new Response(null, {
- status: 404,
- headers: response.headers,
- });
+ return notFound(response);
}
return undefined;
@@ -88,10 +82,7 @@ export function createI18nMiddleware(
// - the URL doesn't contain a locale
const isRoot = url.pathname === base + '/' || url.pathname === base;
if (!(isRoot || pathnameHasLocale(url.pathname, i18n.locales))) {
- return new Response(null, {
- status: 404,
- headers: response.headers,
- });
+ return notFound(response);
}
return undefined;
@@ -202,6 +193,19 @@ export function createI18nMiddleware(
}
/**
+ * The i18n returns empty 404 responses in certain cases.
+ * Error-page-rerouting infra will attempt to render the 404.astro page, causing the middleware to run a second time.
+ * To avoid loops and overwriting the contents of `404.astro`, we allow error pages to pass through.
+ */
+function notFound(response: Response) {
+ if (response.headers.get(REROUTE_DIRECTIVE_HEADER) === "no") return response;
+ return new Response(null, {
+ status: 404,
+ headers: response.headers,
+ });
+}
+
+/**
* Checks if the current locale doesn't belong to a configured domain
* @param i18n
* @param currentLocale
diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts
index 19af22ff6..1c533dbc8 100644
--- a/packages/astro/src/vite-plugin-astro-server/route.ts
+++ b/packages/astro/src/vite-plugin-astro-server/route.ts
@@ -274,7 +274,7 @@ export async function handleRoute({
response.headers.get(REROUTE_DIRECTIVE_HEADER) !== 'no'
) {
const fourOhFourRoute = await matchRoute('/404', manifestData, pipeline);
- if (options && fourOhFourRoute?.route !== options.route)
+ if (options)
return handleRoute({
...options,
matchedRoute: fourOhFourRoute,
@@ -288,6 +288,12 @@ export async function handleRoute({
incomingResponse,
});
}
+
+ // We remove the internally-used header before we send the response to the user agent.
+ if (response.headers.has(REROUTE_DIRECTIVE_HEADER)) {
+ response.headers.delete(REROUTE_DIRECTIVE_HEADER);
+ }
+
if (route.type === 'endpoint') {
await writeWebResponse(incomingResponse, response);
return;
diff --git a/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/404.astro b/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/404.astro
new file mode 100644
index 000000000..045415d44
--- /dev/null
+++ b/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/404.astro
@@ -0,0 +1,4 @@
+<html>
+ <head><title>Not Found</title></head>
+ <body>Can't find the page youre looking for.</body>
+</html>
diff --git a/packages/astro/test/i18n-routing.test.js b/packages/astro/test/i18n-routing.test.js
index 8be9847d3..23255ef9b 100644
--- a/packages/astro/test/i18n-routing.test.js
+++ b/packages/astro/test/i18n-routing.test.js
@@ -353,6 +353,13 @@ describe('[DEV] i18n routing', () => {
assert.equal(response.status, 200);
assert.equal((await response.text()).includes('I am index'), true);
});
+
+ it('can render the 404.astro route on unmatched requests', async () => {
+ const response = await fixture.fetch('/xyz');
+ assert.equal(response.status, 404);
+ const text = await response.text();
+ assert.equal(text.includes("Can't find the page youre looking for."), true);
+ });
});
describe('i18n routing with routing strategy [pathname-prefix-always]', () => {
@@ -1338,6 +1345,15 @@ describe('[SSR] i18n routing', () => {
assert.equal(response.status, 200);
assert.equal((await response.text()).includes('I am index'), true);
});
+
+
+ it('can render the 404.astro route on unmatched requests', async () => {
+ const request = new Request('http://example.com/xyz');
+ const response = await app.render(request);
+ assert.equal(response.status, 404);
+ const text = await response.text();
+ assert.equal(text.includes("Can't find the page youre looking for."), true);
+ });
});
describe('i18n routing with routing strategy [pathname-prefix-always]', () => {
diff --git a/packages/astro/test/units/routing/endpoints.test.js b/packages/astro/test/units/routing/endpoints.test.js
index cff44f1b4..43d76af5a 100644
--- a/packages/astro/test/units/routing/endpoints.test.js
+++ b/packages/astro/test/units/routing/endpoints.test.js
@@ -62,11 +62,10 @@ describe('endpoints', () => {
await done;
const headers = res.getHeaders();
assert.equal(headers['location'], 'https://example.com/destination');
- assert.equal(headers['x-astro-reroute'], undefined);
assert.equal(res.statusCode, 307);
});
- it('should append reroute header for HTTP status 404', async () => {
+ it('should remove internally-used for HTTP status 404', async () => {
const { req, res, done } = createRequestAndResponse({
method: 'GET',
url: '/not-found',
@@ -74,11 +73,11 @@ describe('endpoints', () => {
container.handle(req, res);
await done;
const headers = res.getHeaders();
- assert.equal(headers['x-astro-reroute'], 'no');
+ assert.equal(headers['x-astro-reroute'], undefined);
assert.equal(res.statusCode, 404);
});
- it('should append reroute header for HTTP status 500', async () => {
+ it('should remove internally-used header for HTTP status 500', async () => {
const { req, res, done } = createRequestAndResponse({
method: 'GET',
url: '/internal-error',
@@ -86,7 +85,7 @@ describe('endpoints', () => {
container.handle(req, res);
await done;
const headers = res.getHeaders();
- assert.equal(headers['x-astro-reroute'], 'no');
+ assert.equal(headers['x-astro-reroute'], undefined);
assert.equal(res.statusCode, 500);
});
});