[pull] main from withastro:main

.changeset/fix-i18n-fallback-redirect.md

@@ -0,0 +1,7 @@
+---
+'astro': patch
+---
+
+Fixes i18n fallback middleware intercepting non-404 responses
+
+The fallback middleware was triggering for all responses with status >= 300, including legitimate 3xx redirects, 403 forbidden, and 5xx server errors. This broke auth flows and form submissions on localized server routes. The fallback now correctly only triggers for 404 (page not found) responses.

.changeset/humble-humans-sink.md

@@ -0,0 +1,5 @@
+---
+'@astrojs/node': patch
+---
+
+Fixes an issue where static headers weren't correctly applied when the website uses `base`.

packages/astro/src/i18n/fallback.ts

@@ -54,8 +54,8 @@ export function computeFallbackRoute(options: ComputeFallbackRouteOptions): Fall
 		base,
 	} = options;
 
-	// Only apply fallback for 3xx+ status codes
-	if (responseStatus < 300) {
+	// Only apply fallback for 404 status codes (page not found in this locale)
+	if (responseStatus !== 404) {
 		return { type: 'none' };
 	}
 

packages/astro/src/i18n/index.ts

@@ -372,7 +372,7 @@ export function redirectToFallback({
 	fallbackType,
 }: MiddlewarePayload) {
 	return async function (context: APIContext, response: Response): Promise<Response> {
-		if (response.status >= 300 && fallback) {
+		if (response.status === 404 && fallback) {
 			const fallbackKeys = fallback ? Object.keys(fallback) : [];
 			// we split the URL using the `/`, and then check in the returned array we have the locale
 			const segments = context.url.pathname.split('/');

packages/astro/test/units/i18n/fallback.test.js

@@ -4,8 +4,8 @@ import { computeFallbackRoute } from '../../../dist/i18n/fallback.js';
 import { makeFallbackOptions } from './test-helpers.js';
 
 describe('computeFallbackRoute', () => {
-	describe('when response status < 300', () => {
-		it('returns none (no fallback needed)', () => {
+	describe('when response status is not 404', () => {
+		it('returns none for 200 (success)', () => {
 			const result = computeFallbackRoute(
 				makeFallbackOptions({
 					pathname: '/es/missing',
@@ -18,11 +18,50 @@ describe('computeFallbackRoute', () => {
 			assert.equal(result.type, 'none');
 		});
 
-		it('returns none for 299 status', () => {
+		it('returns none for 301 (redirect)', () => {
 			const result = computeFallbackRoute(
 				makeFallbackOptions({
-					pathname: '/es/missing',
-					responseStatus: 299,
+					pathname: '/es/redirect',
+					responseStatus: 301,
+					currentLocale: 'es',
+					fallback: { es: 'en' },
+				}),
+			);
+
+			assert.equal(result.type, 'none');
+		});
+
+		it('returns none for 302 (temporary redirect)', () => {
+			const result = computeFallbackRoute(
+				makeFallbackOptions({
+					pathname: '/es/redirect',
+					responseStatus: 302,
+					currentLocale: 'es',
+					fallback: { es: 'en' },
+				}),
+			);
+
+			assert.equal(result.type, 'none');
+		});
+
+		it('returns none for 403 (forbidden)', () => {
+			const result = computeFallbackRoute(
+				makeFallbackOptions({
+					pathname: '/es/forbidden',
+					responseStatus: 403,
+					currentLocale: 'es',
+					fallback: { es: 'en' },
+				}),
+			);
+
+			assert.equal(result.type, 'none');
+		});
+
+		it('returns none for 500 (server error)', () => {
+			const result = computeFallbackRoute(
+				makeFallbackOptions({
+					pathname: '/es/error',
+					responseStatus: 500,
 					currentLocale: 'es',
 					fallback: { es: 'en' },
 				}),
@@ -148,7 +187,7 @@ describe('computeFallbackRoute', () => {
 			assert.equal(result.pathname, '/es/missing');
 		});
 
-		it('handles 3xx redirect status', () => {
+		it('only triggers for 404 status, not 3xx', () => {
 			const result = computeFallbackRoute(
 				makeFallbackOptions({
 					pathname: '/es/redirect',
@@ -159,10 +198,10 @@ describe('computeFallbackRoute', () => {
 				}),
 			);
 
-			assert.equal(result.type, 'redirect');
+			assert.equal(result.type, 'none');
 		});
 
-		it('handles 4xx status', () => {
+		it('triggers for 404 status', () => {
 			const result = computeFallbackRoute(
 				makeFallbackOptions({
 					pathname: '/es/notfound',
@@ -176,7 +215,7 @@ describe('computeFallbackRoute', () => {
 			assert.equal(result.type, 'redirect');
 		});
 
-		it('handles 5xx status', () => {
+		it('only triggers for 404 status, not 5xx', () => {
 			const result = computeFallbackRoute(
 				makeFallbackOptions({
 					pathname: '/es/error',
@@ -187,7 +226,7 @@ describe('computeFallbackRoute', () => {
 				}),
 			);
 
-			assert.equal(result.type, 'redirect');
+			assert.equal(result.type, 'none');
 		});
 	});
 

packages/astro/test/units/i18n/manual-routing.test.js

@@ -961,7 +961,7 @@ describe('notFound', () => {
 
 describe('redirectToFallback', () => {
 	describe('basic fallback behavior', () => {
-		it('should return original response when status < 300', async () => {
+		it('should return original response when status is not 404', async () => {
 			const payload = createMiddlewarePayload({
 				fallback: { es: 'en' },
 			});
@@ -974,7 +974,7 @@ describe('redirectToFallback', () => {
 			assert.equal(response, originalResponse);
 		});
 
-		it('should redirect when status >= 300 and locale has fallback', async () => {
+		it('should redirect when status is 404 and locale has fallback', async () => {
 			const payload = createMiddlewarePayload({
 				locales: ['en', 'es', 'fr'],
 				defaultLocale: 'en',
@@ -1087,7 +1087,7 @@ describe('redirectToFallback', () => {
 			assert.equal(response.headers.get('Location'), '/search?q=test');
 		});
 
-		it('should handle 3xx status codes', async () => {
+		it('should not redirect for 3xx status codes', async () => {
 			const payload = createMiddlewarePayload({
 				locales: ['en', 'es'],
 				fallback: { es: 'en' },
@@ -1099,10 +1099,10 @@ describe('redirectToFallback', () => {
 
 			const response = await fallbackFn(context, originalResponse);
 
-			assert.equal(response.status, 302);
+			assert.equal(response, originalResponse);
 		});
 
-		it('should handle 4xx status codes', async () => {
+		it('should not redirect for non-404 4xx status codes', async () => {
 			const payload = createMiddlewarePayload({
 				locales: ['en', 'es'],
 				fallback: { es: 'en' },
@@ -1114,10 +1114,10 @@ describe('redirectToFallback', () => {
 
 			const response = await fallbackFn(context, originalResponse);
 
-			assert.equal(response.status, 302);
+			assert.equal(response, originalResponse);
 		});
 
-		it('should handle 5xx status codes', async () => {
+		it('should not redirect for 5xx status codes', async () => {
 			const payload = createMiddlewarePayload({
 				locales: ['en', 'es'],
 				fallback: { es: 'en' },
@@ -1129,7 +1129,7 @@ describe('redirectToFallback', () => {
 
 			const response = await fallbackFn(context, originalResponse);
 
-			assert.equal(response.status, 302);
+			assert.equal(response, originalResponse);
 		});
 	});
 

packages/integrations/mdx/package.json

@@ -52,6 +52,8 @@
     "astro": "^6.0.0-alpha.0"
   },
   "devDependencies": {
+    "@shikijs/rehype": "^3.22.0",
+    "@shikijs/twoslash": "^3.22.0",
     "@types/estree": "^1.0.8",
     "@types/hast": "^3.0.4",
     "@types/mdast": "^4.0.4",
@@ -65,7 +67,6 @@
     "rehype-pretty-code": "^0.14.1",
     "remark-math": "^6.0.0",
     "remark-rehype": "^11.1.2",
-    "remark-shiki-twoslash": "^3.1.3",
     "remark-toc": "^9.0.0",
     "shiki": "^3.22.0",
     "unified": "^11.0.5",

packages/integrations/mdx/test/fixtures/mdx-syntax-hightlighting/src/pages/index.mdx

@@ -7,3 +7,7 @@ const handlesAstroSyntax = true
 
 <h1>{handlesAstroSyntax}</h1>
 ```
+
+```ts twoslash
+console.log("Hover over the function to see its documentation")
+```

packages/integrations/mdx/test/mdx-syntax-highlighting.test.js

@@ -1,9 +1,10 @@
 import * as assert from 'node:assert/strict';
 import { describe, it } from 'node:test';
 import mdx from '@astrojs/mdx';
+import rehypeShiki from '@shikijs/rehype';
+import { transformerTwoslash } from '@shikijs/twoslash';
 import { parseHTML } from 'linkedom';
 import rehypePrettyCode from 'rehype-pretty-code';
-import shikiTwoslash from 'remark-shiki-twoslash';
 import { loadFixture } from '../../../astro/test/test-utils.js';
 
 const FIXTURE_ROOT = new URL('./fixtures/mdx-syntax-hightlighting/', import.meta.url);
@@ -95,15 +96,23 @@ describe('MDX syntax highlighting', () => {
 		}
 	});
 
-	it('supports custom highlighter - shiki-twoslash', async () => {
+	it('supports custom highlighter - @shikijs/rehype and @shikijs/twoslash', async () => {
 		const fixture = await loadFixture({
 			root: FIXTURE_ROOT,
 			markdown: {
 				syntaxHighlight: false,
 			},
 			integrations: [
 				mdx({
-					remarkPlugins: [shikiTwoslash.default ?? shikiTwoslash],
+					rehypePlugins: [
+						[
+							rehypeShiki,
+							{
+								theme: 'vitesse-light',
+								transformers: [transformerTwoslash({})],
+							},
+						],
+					],
 				}),
 			],
 		});
@@ -112,8 +121,11 @@ describe('MDX syntax highlighting', () => {
 		const html = await fixture.readFile('/index.html');
 		const { document } = parseHTML(html);
 
-		const twoslashCodeBlock = document.querySelector('pre.shiki');
-		assert.notEqual(twoslashCodeBlock, null);
+		const shikiCodeBlock = document.querySelector('pre.shiki');
+		assert.notEqual(shikiCodeBlock, null);
+
+		const twoslashPopup = document.querySelector('div.twoslash-popup-docs');
+		assert.notEqual(twoslashPopup, null);
 	});
 
 	it('supports custom highlighter - rehype-pretty-code', async () => {

packages/integrations/node/src/serve-static.ts

@@ -48,7 +48,13 @@ export function createStaticHandler(
 				});
 				const routeData = app.match(request, true);
 				if (routeData && routeData.prerender) {
-					const matchedRoute = headersMap.find((header) => header.pathname.includes(pathname));
+					// Headers are stored keyed by base-less route paths (e.g. "/one"), so we
+					// must strip config.base from the incoming URL before matching, just as
+					// we do for filesystem access above.
+					const baselessPathname = prependForwardSlash(app.removeBase(urlPath));
+					const matchedRoute = headersMap.find((header) =>
+						header.pathname.includes(baselessPathname),
+					);
 					if (matchedRoute) {
 						for (const header of matchedRoute.headers) {
 							res.setHeader(header.key, header.value);

packages/integrations/node/test/static-headers.test.js

@@ -36,11 +36,13 @@ describe('Static headers', () => {
 	before(async () => {
 		fixture = await loadFixture({
 			root: './fixtures/static-headers/',
+			outDir: './dist/root-base',
 			output: 'server',
 			adapter: nodejs({ mode: 'standalone', staticHeaders: true }),
 		});
 		await fixture.build();
 		const { startServer } = await fixture.loadAdapterEntryModule();
+		process.env.PORT = '4322';
 		const res = startServer();
 		server = res.server;
 		await waitServerListen(server.server);
@@ -69,3 +71,49 @@ describe('Static headers', () => {
 		);
 	});
 });
+
+describe('Static headers with non-root base', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+	let server;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/static-headers/',
+			outDir: './dist/non-root-base',
+			base: '/docs',
+			output: 'server',
+			adapter: nodejs({ mode: 'standalone', staticHeaders: true }),
+		});
+		await fixture.build();
+		const { startServer } = await fixture.loadAdapterEntryModule();
+		process.env.PORT = '4323';
+		const res = startServer();
+		server = res.server;
+		await waitServerListen(server.server);
+	});
+
+	after(async () => {
+		await server.stop();
+	});
+
+	it('CSP headers are added to the index route under the base path', async () => {
+		const res = await fetch(`http://${server.host}:${server.port}/docs/`);
+		const csp = res.headers.get('Content-Security-Policy');
+		assert.ok(csp, 'Content-Security-Policy header must be present for the index route');
+		assert.ok(
+			csp.includes('script-src'),
+			'should contain script-src directive due to server island',
+		);
+	});
+
+	it('CSP headers are added to a dynamic route under the base path', async () => {
+		const res = await fetch(`http://${server.host}:${server.port}/docs/one`);
+		const csp = res.headers.get('Content-Security-Policy');
+		assert.ok(csp, 'Content-Security-Policy header must be present for dynamic routes');
+		assert.ok(
+			csp.includes('script-src'),
+			'should contain script-src directive due to server island',
+		);
+	});
+});