diff --git a/.changeset/blue-jokes-eat.md b/.changeset/blue-jokes-eat.md new file mode 100644 index 0000000000..a5a584ce1c --- /dev/null +++ b/.changeset/blue-jokes-eat.md @@ -0,0 +1,5 @@ +--- +'@astrojs/internal-helpers': minor +--- + +Adds `collapseDuplicateTrailingSlashes` function diff --git a/.changeset/blue-spies-shave.md b/.changeset/blue-spies-shave.md new file mode 100644 index 0000000000..562de6a461 --- /dev/null +++ b/.changeset/blue-spies-shave.md @@ -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. diff --git a/.changeset/many-fans-battle.md b/.changeset/many-fans-battle.md new file mode 100644 index 0000000000..8240531d02 --- /dev/null +++ b/.changeset/many-fans-battle.md @@ -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 diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index a90e3aa824..543d82d0fd 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -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 { 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); diff --git a/packages/astro/src/core/constants.ts b/packages/astro/src/core/constants.ts index dc09a1f694..92e99f8d1b 100644 --- a/packages/astro/src/core/constants.ts +++ b/packages/astro/src/core/constants.ts @@ -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'; + diff --git a/packages/astro/src/template/4xx.ts b/packages/astro/src/template/4xx.ts index 45f4ac10ae..3b6eff2106 100644 --- a/packages/astro/src/template/4xx.ts +++ b/packages/astro/src/template/4xx.ts @@ -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: `

Your site is configured with trailingSlash set to ${trailingSlash}. Do you want to go to ${corrected} instead?

+

See the documentation for trailingSlash if you need help.

`, + }); +} + export function notFoundTemplate(pathname: string, message = 'Not found') { return template({ pathname, diff --git a/packages/astro/src/types/public/config.ts b/packages/astro/src/types/public/config.ts index 30ed2a42cd..0b73f8f9e8 100644 --- a/packages/astro/src/types/public/config.ts +++ b/packages/astro/src/types/public/config.ts @@ -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 * { diff --git a/packages/astro/src/vite-plugin-astro-server/base.ts b/packages/astro/src/vite-plugin-astro-server/base.ts index 4aa7e2a2d9..d6e5ddf3a4 100644 --- a/packages/astro/src/vite-plugin-astro-server/base.ts +++ b/packages/astro/src/vite-plugin-astro-server/base.ts @@ -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); } diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 6f27c475c4..113a858043 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -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) { diff --git a/packages/astro/src/vite-plugin-astro-server/trailing-slash.ts b/packages/astro/src/vite-plugin-astro-server/trailing-slash.ts new file mode 100644 index 0000000000..fff8d31178 --- /dev/null +++ b/packages/astro/src/vite-plugin-astro-server/trailing-slash.ts @@ -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(); + }; +} diff --git a/packages/astro/test/fixtures/ssr-response/src/pages/another.astro b/packages/astro/test/fixtures/ssr-response/src/pages/another.astro new file mode 100644 index 0000000000..5d9e2e97dd --- /dev/null +++ b/packages/astro/test/fixtures/ssr-response/src/pages/another.astro @@ -0,0 +1,13 @@ +--- +--- + + + + + + Document + + +

Hello {Astro.url}

+ + diff --git a/packages/astro/test/fixtures/ssr-response/src/pages/index.astro b/packages/astro/test/fixtures/ssr-response/src/pages/index.astro new file mode 100644 index 0000000000..a9f08bffe9 --- /dev/null +++ b/packages/astro/test/fixtures/ssr-response/src/pages/index.astro @@ -0,0 +1,13 @@ +--- +--- + + + + + + Document + + +

Hello /

+ + diff --git a/packages/astro/test/fixtures/ssr-response/src/pages/sub/path.astro b/packages/astro/test/fixtures/ssr-response/src/pages/sub/path.astro new file mode 100644 index 0000000000..5d9e2e97dd --- /dev/null +++ b/packages/astro/test/fixtures/ssr-response/src/pages/sub/path.astro @@ -0,0 +1,13 @@ +--- +--- + + + + + + Document + + +

Hello {Astro.url}

+ + diff --git a/packages/astro/test/ssr-error-pages.test.js b/packages/astro/test/ssr-error-pages.test.js index 9ebc58770d..61335574d2 100644 --- a/packages/astro/test/ssr-error-pages.test.js +++ b/packages/astro/test/ssr-error-pages.test.js @@ -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); diff --git a/packages/astro/test/ssr-trailing-slash.js b/packages/astro/test/ssr-trailing-slash.js new file mode 100644 index 0000000000..0ea97842fe --- /dev/null +++ b/packages/astro/test/ssr-trailing-slash.js @@ -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); + }); + }); +}); diff --git a/packages/astro/test/units/test-utils.js b/packages/astro/test/units/test-utils.js index f415ca2577..601042413d 100644 --- a/packages/astro/test/units/test-utils.js +++ b/packages/astro/test/units/test-utils.js @@ -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', () => { diff --git a/packages/internal-helpers/src/path.ts b/packages/internal-helpers/src/path.ts index 6b1c981257..6078a0d9a4 100644 --- a/packages/internal-helpers/src/path.ts +++ b/packages/internal-helpers/src/path.ts @@ -19,6 +19,15 @@ export function collapseDuplicateSlashes(path: string) { return path.replace(/(?