Infrastructure/astro
0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2025-02-17 22:44:24 -05:00
Code Issues Activity

feat: redirect trailing slashes on on-demand rendered pages (#12994)

Browse source 
Co-authored-by: ematipico <602478+ematipico@users.noreply.github.com>
Co-authored-by: matthewp <361671+matthewp@users.noreply.github.com>
Co-authored-by: sarah11918 <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: bluwy <34116392+bluwy@users.noreply.github.com>
This commit is contained in:
Matt Kane 2025-01-29 11:54:15 +00:00 • committed by GitHub
parent e621712109
commit 536175528d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 433 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/blue-jokes-eat.md Normal file
View file

@ -0,0 +1,5 @@
---
'@astrojs/internal-helpers': minor
---
Adds `collapseDuplicateTrailingSlashes` function

11
.changeset/blue-spies-shave.md Normal file
View file

@ -0,0 +1,11 @@
---
'astro': minor
---
Redirects trailing slashes for on-demand pages
When the `trailingSlash` option is set to `always` or `never`, on-demand rendered pages will now redirect to the correct URL when the trailing slash doesn't match the configuration option. This was previously the case for static pages, but now works for on-demand pages as well.
Now, it doesn't matter whether your visitor navigates to `/about/`, `/about`, or even `/about///`. In production, they'll always end up on the correct page. For GET requests, the redirect will be a 301 (permanent) redirect, and for all other request methods, it will be a 308 (permanent, and preserve the request method) redirect.
In development, you'll see a helpful 404 page to alert you of a trailing slash mismatch so you can troubleshoot routes.

5
.changeset/many-fans-battle.md Normal file
View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Returns a more helpful 404 page in dev if there is a trailing slash mismatch between the route requested and the `trailingSlash` configuration

42
packages/astro/src/core/app/index.ts
View file

@ -1,3 +1,4 @@
import { collapseDuplicateTrailingSlashes, hasFileExtension } from '@astrojs/internal-helpers/path';
import { normalizeTheLocale } from '../../i18n/index.js';
import type { RoutesList } from '../../types/astro.js';
import type { RouteData, SSRManifest } from '../../types/public/internal.js';
@ -20,6 +21,7 @@ import {
} from '../path.js';
import { RenderContext } from '../render-context.js';
import { createAssetLink } from '../render/ssr-element.js';
import { redirectTemplate } from '../routing/3xx.js';
import { ensure404Route } from '../routing/astro-designed-error-pages.js';
import { createDefaultRoutes } from '../routing/default.js';
import { matchRoute } from '../routing/match.js';
@ -250,11 +252,51 @@ export class App {
return pathname;
}
#redirectTrailingSlash(pathname: string): string {
const { trailingSlash } = this.#manifest;
// Ignore root and internal paths
if (pathname === '/' || pathname.startsWith('/_')) {
return pathname;
}
// Redirect multiple trailing slashes to collapsed path
const path = collapseDuplicateTrailingSlashes(pathname, trailingSlash !== 'never');
if (path !== pathname) {
return path;
}
if (trailingSlash === 'ignore') {
return pathname;
}
if (trailingSlash === 'always' && !hasFileExtension(pathname)) {
return appendForwardSlash(pathname);
}
if (trailingSlash === 'never') {
return removeTrailingForwardSlash(pathname);
}
return pathname;
}
async render(request: Request, renderOptions?: RenderOptions): Promise<Response> {
let routeData: RouteData | undefined;
let locals: object | undefined;
let clientAddress: string | undefined;
let addCookieHeader: boolean | undefined;
const url = new URL(request.url);
const redirect = this.#redirectTrailingSlash(url.pathname);
if (redirect !== url.pathname) {
const status = request.method === 'GET' ? 301 : 308;
return new Response(redirectTemplate({ status, location: redirect, from: request.url }), {
status,
headers: {
location: redirect + url.search,
},
});
}
addCookieHeader = renderOptions?.addCookieHeader;
clientAddress = renderOptions?.clientAddress ?? Reflect.get(request, clientAddressSymbol);

1
packages/astro/src/core/constants.ts
View file

@ -100,3 +100,4 @@ export const SUPPORTED_MARKDOWN_FILE_EXTENSIONS = [
// The folder name where to find the middleware
export const MIDDLEWARE_PATH_SEGMENT_NAME = 'middleware';

16
packages/astro/src/template/4xx.ts
View file

@ -1,3 +1,4 @@
import { appendForwardSlash, removeTrailingForwardSlash } from '@astrojs/internal-helpers/path';
import { escape } from 'html-escaper';
interface ErrorTemplateOptions {
@ -129,6 +130,21 @@ export function subpathNotUsedTemplate(base: string, pathname: string) {
});
}
export function trailingSlashMismatchTemplate(pathname: string, trailingSlash: 'always' | 'never' | 'ignore') {
const corrected =
trailingSlash === 'always'
? appendForwardSlash(pathname)
: removeTrailingForwardSlash(pathname);
return template({
pathname,
statusCode: 404,
title: 'Not found',
tabTitle: '404: Not Found',
body: `<p>Your site is configured with <code>trailingSlash</code> set to <code>${trailingSlash}</code>. Do you want to go to <a href="${corrected}">${corrected}</a> instead?</p>
<p>See <a href=https://docs.astro.build/en/reference/configuration-reference/#trailingslash">the documentation for <code>trailingSlash</code></a> if you need help.</p>`,
});
}
export function notFoundTemplate(pathname: string, message = 'Not found') {
return template({
pathname,

15
packages/astro/src/types/public/config.ts
View file

@ -236,14 +236,15 @@ export interface ViteUserConfig extends OriginalViteUserConfig {
* @see build.format
* @description
*
* Set the route matching behavior of the dev server. Choose from the following options:
* - `'always'` - Only match URLs that include a trailing slash (ex: "/foo/")
* - `'never'` - Never match URLs that include a trailing slash (ex: "/foo")
* - `'ignore'` - Match URLs regardless of whether a trailing "/" exists
* Set the route matching behavior for trailing slashes in the dev server and on-demand rendered pages. Choose from the following options:
* - `'ignore'` - Match URLs regardless of whether a trailing "/" exists. Requests for "/about" and "/about/" will both match the same route.
* - `'always'` - Only match URLs that include a trailing slash (e.g: "/about/"). In production, requests for on-demand rendered URLs without a trailing slash will be redirected to the correct URL for your convenience. However, in development, they will display a warning page reminding you that you have `always` configured.
* - `'never'` - Only match URLs that do not include a trailing slash (e.g: "/about"). In production, requests for on-demand rendered URLs with a trailing slash will be redirected to the correct URL for your convenience. However, in development, they will display a warning page reminding you that you have `never` configured.
*
* Use this configuration option if your production host has strict handling of how trailing slashes work or do not work.
*
* You can also set this if you prefer to be more strict yourself, so that URLs with or without trailing slashes won't work during development.
* When redirects occur in production for GET requests, the redirect will be a 301 (permanent) redirect. For all other request methods, it will be a 308 (permanent, and preserve the request method) redirect.
*
* Trailing slashes on prerendered pages are handled by the hosting platform, and may not respect your chosen configuration.
* See your hosting platform's documentation for more information.
*
* ```js
* {

19
packages/astro/src/vite-plugin-astro-server/base.ts
View file

@ -3,13 +3,11 @@ import type { AstroSettings } from '../types/astro.js';
import * as fs from 'node:fs';
import path from 'node:path';
import { appendForwardSlash } from '@astrojs/internal-helpers/path';
import { bold } from 'kleur/colors';
import type { Logger } from '../core/logger/core.js';
import notFoundTemplate, { subpathNotUsedTemplate } from '../template/4xx.js';
import { writeHtmlResponse, writeRedirectResponse } from './response.js';
const manySlashes = /\/{2,}$/;
import { notFoundTemplate, subpathNotUsedTemplate } from '../template/4xx.js';
import { writeHtmlResponse } from './response.js';
import { appendForwardSlash } from '@astrojs/internal-helpers/path';
export function baseMiddleware(
settings: AstroSettings,
@ -23,10 +21,6 @@ export function baseMiddleware(
return function devBaseMiddleware(req, res, next) {
const url = req.url!;
if (manySlashes.test(url)) {
const destination = url.replace(manySlashes, '/');
return writeRedirectResponse(res, 301, destination);
}
let pathname: string;
try {
pathname = decodeURI(new URL(url, 'http://localhost').pathname);
@ -46,12 +40,7 @@ export function baseMiddleware(
}
if (req.headers.accept?.includes('text/html')) {
const html = notFoundTemplate({
statusCode: 404,
title: 'Not found',
tabTitle: '404: Not Found',
pathname,
});
const html = notFoundTemplate(pathname);
return writeHtmlResponse(res, 404, html);
}

5
packages/astro/src/vite-plugin-astro-server/plugin.ts
View file

@ -24,6 +24,7 @@ import { recordServerError } from './error.js';
import { DevPipeline } from './pipeline.js';
import { handleRequest } from './request.js';
import { setRouteError } from './server-state.js';
import { trailingSlashMiddleware } from './trailing-slash.js';
export interface AstroPluginOptions {
settings: AstroSettings;
@ -119,6 +120,10 @@ export default function createVitePluginAstroServer({
route: '',
handle: baseMiddleware(settings, logger),
});
viteServer.middlewares.stack.unshift({
route: '',
handle: trailingSlashMiddleware(settings),
});
// Note that this function has a name so other middleware can find it.
viteServer.middlewares.use(async function astroDevHandler(request, response) {
if (request.url === undefined || !request.method) {

34
packages/astro/src/vite-plugin-astro-server/trailing-slash.ts Normal file
View file

@ -0,0 +1,34 @@
import type * as vite from 'vite';
import type { AstroSettings } from '../types/astro.js';
import { collapseDuplicateTrailingSlashes, hasFileExtension } from '@astrojs/internal-helpers/path';
import { trailingSlashMismatchTemplate } from '../template/4xx.js';
import { writeHtmlResponse, writeRedirectResponse } from './response.js';
export function trailingSlashMiddleware(settings: AstroSettings): vite.Connect.NextHandleFunction {
const { trailingSlash } = settings.config;
return function devTrailingSlash(req, res, next) {
const url = req.url!;
const destination = collapseDuplicateTrailingSlashes(url, true);
if (url && destination !== url) {
return writeRedirectResponse(res, 301, destination);
}
let pathname: string;
try {
pathname = decodeURI(new URL(url, 'http://localhost').pathname);
} catch (e) {
/* malformed uri */
return next(e);
}
if (
(trailingSlash === 'never' && pathname.endsWith('/') && pathname !== '/') ||
(trailingSlash === 'always' && !pathname.endsWith('/') && !hasFileExtension(pathname))
) {
const html = trailingSlashMismatchTemplate(pathname, trailingSlash);
return writeHtmlResponse(res, 404, html);
}
return next();
};
}

13
packages/astro/test/fixtures/ssr-response/src/pages/another.astro vendored Normal file
View file

@ -0,0 +1,13 @@
---
---
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Document</title>
</head>
<body>
<h1>Hello {Astro.url}</h1>
</body>
</html>

13
packages/astro/test/fixtures/ssr-response/src/pages/index.astro vendored Normal file
View file

@ -0,0 +1,13 @@
---
---
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Document</title>
</head>
<body>
<h1>Hello /</h1>
</body>
</html>

13
packages/astro/test/fixtures/ssr-response/src/pages/sub/path.astro vendored Normal file
View file

@ -0,0 +1,13 @@
---
---
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Document</title>
</head>
<body>
<h1>Hello {Astro.url}</h1>
</body>
</html>

4
packages/astro/test/ssr-error-pages.test.js
View file

@ -163,7 +163,7 @@ describe('trailing slashes for error pages', () => {
});
it('renders 404 page when a route does not match the request', async () => {
const response = await fixture.fetch('/ashbfjkasn');
const response = await fixture.fetch('/ashbfjkasn/');
assert.equal(response.status, 404);
const html = await response.text();
const $ = cheerio.load(html);
@ -181,7 +181,7 @@ describe('trailing slashes for error pages', () => {
});
it('renders 404 page when a route does not match the request', async () => {
const response = await app.render(new Request('http://example.com/ajksalscla'));
const response = await app.render(new Request('http://example.com/ajksalscla/'));
assert.equal(response.status, 404);
const html = await response.text();
const $ = cheerio.load(html);

243
packages/astro/test/ssr-trailing-slash.js Normal file
View file

@ -0,0 +1,243 @@
import assert from 'node:assert/strict';
import { before, describe, it } from 'node:test';
import testAdapter from './test-adapter.js';
import { loadFixture } from './test-utils.js';
describe('Redirecting trailing slashes in SSR', () => {
/** @type {import('./test-utils.js').Fixture} */
let fixture;
describe('trailingSlash: always', () => {
before(async () => {
fixture = await loadFixture({
root: './fixtures/ssr-response/',
adapter: testAdapter(),
output: 'server',
trailingSlash: 'always',
});
await fixture.build();
});
it('Redirects to add a trailing slash', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/another/');
});
it('Redirects to collapse multiple trailing slashes', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another///');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/another/');
});
it('Does not redirect when trailing slash is present', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another/');
const response = await app.render(request);
assert.equal(response.status, 200);
});
it('Redirects with query params', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another?foo=bar');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/another/?foo=bar');
});
it('Does not redirect with query params when trailing slash is present', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another/?foo=bar');
const response = await app.render(request);
assert.equal(response.status, 200);
});
it('Redirects subdirectories to add a trailing slash', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/sub/path');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/sub/path/');
});
it('Does not redirect requests for files', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/favicon.ico');
const response = await app.render(request);
assert.equal(response.status, 404);
});
it('Does not redirect requests for files in subdirectories', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/sub/favicon.ico');
const response = await app.render(request);
assert.equal(response.status, 404);
});
it('Does redirect if the dot is in a directory name', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/dot.in.directory/path');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/dot.in.directory/path/');
});
it("Does not redirect internal paths", async () => {
const app = await fixture.loadTestAdapterApp();
for (const path of [
'/_astro/something',
'/_image?url=http://example.com/foo.jpg',
'/_server-islands/foo',
'/_actions/foo'
]) {
const request = new Request(`http://example.com${path}`);
const response = await app.render(request);
assert.notEqual(response.status, 301);
}
});
it("Redirects POST requests", async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another', { method: 'POST' });
const response = await app.render(request);
assert.equal(response.status, 308);
assert.equal(response.headers.get('Location'), '/another/');
});
});
describe('trailingSlash: never', () => {
before(async () => {
fixture = await loadFixture({
root: './fixtures/ssr-response/',
adapter: testAdapter(),
output: 'server',
trailingSlash: 'never',
});
await fixture.build();
});
it('Redirects to remove a trailing slash', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another/');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/another');
});
it('Redirects to collapse multiple trailing slashes', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another///');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/another');
});
it('Does not redirect when trailing slash is absent', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another');
const response = await app.render(request);
assert.equal(response.status, 200);
});
it('Redirects with query params', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another/?foo=bar');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/another?foo=bar');
});
it('Does not redirect with query params when trailing slash is absent', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another?foo=bar');
const response = await app.render(request);
assert.equal(response.status, 200);
});
it("Does not redirect when there's a slash at the end of query params", async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another?foo=bar/');
const response = await app.render(request);
assert.equal(response.status, 200);
});
it('Redirects subdirectories to remove a trailing slash', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/sub/path/');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/sub/path');
});
it("Redirects even if there's a dot in the directory name", async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/favicon.ico/');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/favicon.ico');
});
it('Does not redirect internal paths', async () => {
const app = await fixture.loadTestAdapterApp();
for (const path of [
'/_astro/something/',
'/_image/?url=http://example.com/foo.jpg',
'/_server-islands/foo/',
'/_actions/foo/'
]) {
const request = new Request(`http://example.com${path}/`);
const response = await app.render(request);
assert.notEqual(response.status, 301);
}
});
it('Redirects POST requests', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another/', { method: 'POST' });
const response = await app.render(request);
assert.equal(response.status, 308);
assert.equal(response.headers.get('Location'), '/another');
});
});
describe('trailingSlash: ignore', () => {
before(async () => {
fixture = await loadFixture({
root: './fixtures/ssr-response/',
adapter: testAdapter(),
output: 'server',
trailingSlash: 'ignore',
});
await fixture.build();
});
it("Redirects to collapse multiple trailing slashes", async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another///');
const response = await app.render(request);
assert.equal(response.status, 301);
assert.equal(response.headers.get('Location'), '/another/');
});
it('Does not redirect when trailing slash is absent', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another');
const response = await app.render(request);
assert.equal(response.status, 200);
});
it('Does not redirect when trailing slash is present', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/another/');
const response = await app.render(request);
assert.equal(response.status, 200);
});
});
});

3
packages/astro/test/units/test-utils.js
View file

@ -78,6 +78,9 @@ export function toPromise(res) {
if (ArrayBuffer.isView(data) && !Buffer.isBuffer(data)) {
data = Buffer.from(data.buffer);
}
if(typeof data === 'string') {
data = Buffer.from(data);
}
return write.call(this, data, encoding);
};
res.on('end', () => {

15
packages/internal-helpers/src/path.ts
View file

@ -19,6 +19,15 @@ export function collapseDuplicateSlashes(path: string) {
return path.replace(/(?<!:)\/{2,}/g, '/');
}
export const MANY_TRAILING_SLASHES = /\/{2,}/g;
export function collapseDuplicateTrailingSlashes(path: string, trailingSlash: boolean) {
if(!path) {
return path
}
return path.replace(MANY_TRAILING_SLASHES, trailingSlash ? '/' : '') || '/';
}
export function removeTrailingForwardSlash(path: string) {
return path.endsWith('/') ? path.slice(0, path.length - 1) : path;
}
@ -104,3 +113,9 @@ export function removeBase(path: string, base: string) {
}
return path;
}
const WITH_FILE_EXT = /\/[^/]+\.\w+$/;
export function hasFileExtension(path: string) {
return WITH_FILE_EXT.test(path);
}