diff --git a/.changeset/spotty-turtles-complain.md b/.changeset/spotty-turtles-complain.md new file mode 100644 index 000000000000..021eac91fd9f --- /dev/null +++ b/.changeset/spotty-turtles-complain.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix preview issues triggered by pageUrlFormat & trailingSlash options diff --git a/packages/astro/src/core/dev/template/4xx.ts b/packages/astro/src/core/dev/template/4xx.ts index af63feea7cfe..7f90b6e16a5d 100644 --- a/packages/astro/src/core/dev/template/4xx.ts +++ b/packages/astro/src/core/dev/template/4xx.ts @@ -17,45 +17,45 @@ interface ErrorTemplateOptions { /** Display all errors */ export default function template({ title, pathname, statusCode = 404, tabTitle, body }: ErrorTemplateOptions): string { return ` - - - - ${tabTitle} - - - -
- -

${statusCode ? `${statusCode}: ` : ''}${title}

- ${ - body || - ` - 
Path: ${encode(pathname)}
- ` - } -
- - `; + .astro { + height: 120px; + width: 120px; + } + + + +
+ +

${statusCode ? `${statusCode}: ` : ''}${title}

+ ${ + body || + ` + 
Path: ${encode(pathname)}
+ ` + } +
+ +`; } export function subpathNotUsedTemplate(base: string, pathname: string) { @@ -64,9 +64,16 @@ export function subpathNotUsedTemplate(base: string, pathname: string) { statusCode: 404, title: 'Not found', tabTitle: '404: Not Found', - body: ` -

In your buildOptions.site you have your base path set to ${base}. Do you want to go there instead?

-

Come to our Discord if you need help.

- `, + body: `

In your buildOptions.site you have your base path set to ${base}. Do you want to go there instead?

+

Come to our Discord if you need help.

`, + }); +} + +export function notFoundTemplate(pathname: string, message = 'Not found') { + return template({ + pathname, + statusCode: 404, + title: message, + tabTitle: `404: ${message}`, }); } diff --git a/packages/astro/src/core/path.ts b/packages/astro/src/core/path.ts index 2d6f62dffb82..9f66baf28617 100644 --- a/packages/astro/src/core/path.ts +++ b/packages/astro/src/core/path.ts @@ -34,3 +34,7 @@ export function prependDotSlash(path: string) { return './' + path; } + +export function trimSlashes(path: string) { + return path.replace(/^\/|\/$/g, ''); +} diff --git a/packages/astro/src/core/preview/index.ts b/packages/astro/src/core/preview/index.ts index aa742b716386..60296bc7308a 100644 --- a/packages/astro/src/core/preview/index.ts +++ b/packages/astro/src/core/preview/index.ts @@ -1,16 +1,16 @@ import type { AstroConfig } from '../../@types/astro'; import type { LogOptions } from '../logger'; +import type { Stats } from 'fs'; import http from 'http'; import { performance } from 'perf_hooks'; import send from 'send'; import { fileURLToPath } from 'url'; +import fs from 'fs'; import * as msg from '../dev/messages.js'; import { error, info } from '../logger.js'; -import { subpathNotUsedTemplate, default as template } from '../dev/template/4xx.js'; -import { prependForwardSlash } from '../path.js'; -import * as npath from 'path'; -import * as fs from 'fs'; +import { subpathNotUsedTemplate, notFoundTemplate, default as template } from '../dev/template/4xx.js'; +import { appendForwardSlash, trimSlashes } from '../path.js'; interface PreviewOptions { logging: LogOptions; @@ -23,100 +23,94 @@ export interface PreviewServer { stop(): Promise; } -type SendStreamWithPath = send.SendStream & { path: string }; - -function removeBase(base: string, pathname: string) { - if (base === pathname) { - return '/'; - } - let requrl = pathname.substr(base.length); - return prependForwardSlash(requrl); -} - /** The primary dev action */ export default async function preview(config: AstroConfig, { logging }: PreviewOptions): Promise { const startServerTime = performance.now(); - const base = config.buildOptions.site ? new URL(config.buildOptions.site).pathname : '/'; + const pageUrlFormat = config.buildOptions.pageUrlFormat; + const trailingSlash = config.devOptions.trailingSlash; + const forceTrailingSlash = trailingSlash === 'always'; + const blockTrailingSlash = trailingSlash === 'never'; + + /** Default file served from a directory. */ + const defaultFile = 'index.html'; + + const defaultOrigin = 'http://localhost'; + + const sendOptions = { + extensions: pageUrlFormat === 'file' ? ['html'] : false, + index: false, + root: fileURLToPath(config.dist), + }; + + /** Base request URL. */ + let baseURL = new URL(appendForwardSlash(config.buildOptions.site || ''), defaultOrigin); // Create the preview server, send static files out of the `dist/` directory. const server = http.createServer((req, res) => { - if (!req.url!.startsWith(base)) { + const requestURL = new URL(req.url as string, defaultOrigin); + + // respond 404 to requests outside the base request directory + if (!requestURL.pathname.startsWith(baseURL.pathname)) { res.statusCode = 404; - res.end(subpathNotUsedTemplate(base, req.url!)); + res.end(subpathNotUsedTemplate(baseURL.pathname, requestURL.pathname)); return; } - switch (config.devOptions.trailingSlash) { - case 'always': { - if (!req.url?.endsWith('/')) { - res.statusCode = 404; - res.end( - template({ - title: 'Not found', - tabTitle: 'Not found', - pathname: req.url!, - }) - ); - return; - } - break; - } - case 'never': { - if (req.url?.endsWith('/')) { - res.statusCode = 404; - res.end( - template({ - title: 'Not found', - tabTitle: 'Not found', - pathname: req.url!, - }) - ); - return; - } - break; - } - case 'ignore': { - break; - } - } + /** Relative request path. */ + const pathname = requestURL.pathname.slice(baseURL.pathname.length - 1); + + const isRoot = pathname === '/'; + const hasTrailingSlash = isRoot || pathname.endsWith('/'); + + let tryTrailingSlash = true; + let tryHtmlExtension = true; + + let url: URL; - let sendpath = removeBase(base, req.url!); - const sendOptions: send.SendOptions = { - root: fileURLToPath(config.dist), + const onErr = (message: string) => { + res.statusCode = 404; + res.end(notFoundTemplate(pathname, message)); }; - if (config.buildOptions.pageUrlFormat === 'file' && !sendpath.endsWith('.html')) { - sendOptions.index = false; - const parts = sendpath.split('/'); - let lastPart = parts.pop(); - switch (config.devOptions.trailingSlash) { - case 'always': { - lastPart = parts.pop(); - break; - } - case 'never': { - // lastPart is the actually last part like `page` - break; - } - case 'ignore': { - // this could end in slash, so resolve either way - if (lastPart === '') { - lastPart = parts.pop(); - } - break; - } + + const onStat = (err: NodeJS.ErrnoException | null, stat: Stats) => { + switch (true) { + // retry nonexistent paths without an html extension + case err && tryHtmlExtension && hasTrailingSlash && !blockTrailingSlash: + case err && tryHtmlExtension && !hasTrailingSlash && !forceTrailingSlash && !pathname.endsWith('.html'): + tryHtmlExtension = false; + return fs.stat((url = new URL(url.pathname + '.html', url)), onStat); + + // 404 on nonexistent paths (that are yet handled) + case err !== null: + return onErr('Path not found'); + + // 404 on directories when a trailing slash is present but blocked + case stat.isDirectory() && hasTrailingSlash && blockTrailingSlash && !isRoot: + return onErr('Prohibited trailing slash'); + + // 404 on directories when a trailing slash is missing but forced + case stat.isDirectory() && !hasTrailingSlash && forceTrailingSlash && !isRoot: + return onErr('Required trailing slash'); + + // retry on directories when a default file is missing but allowed (that are yet handled) + case stat.isDirectory() && tryTrailingSlash: + tryTrailingSlash = false; + return fs.stat((url = new URL(url.pathname + (url.pathname.endsWith('/') ? defaultFile : '/' + defaultFile), url)), onStat); + + // 404 on existent directories (that are yet handled) + case stat.isDirectory(): + return onErr('Path not found'); + + // handle existent paths + default: + send(req, fileURLToPath(url), { + extensions: false, + index: false, + }).pipe(res); } - const part = lastPart || 'index'; - sendpath = npath.sep + npath.join(...parts, `${part}.html`); - } - send(req, sendpath, sendOptions) - .once('directory', function (this: SendStreamWithPath, _res, path) { - if (config.buildOptions.pageUrlFormat === 'directory' && !path.endsWith('index.html')) { - return this.sendIndex(path); - } else { - this.error(404); - } - }) - .pipe(res); + }; + + fs.stat((url = new URL(trimSlashes(pathname), config.dist)), onStat); }); let { hostname, port } = config.devOptions; @@ -132,7 +126,7 @@ export default async function preview(config: AstroConfig, { logging }: PreviewO httpServer = server.listen(port, hostname, () => { if (!showedListenMsg) { info(logging, 'astro', msg.devStart({ startupTime: performance.now() - timerStart })); - info(logging, 'astro', msg.devHost({ host: `http://${hostname}:${port}${base}` })); + info(logging, 'astro', msg.devHost({ host: `http://${hostname}:${port}${baseURL.pathname}` })); } showedListenMsg = true; resolve(); diff --git a/packages/astro/test/preview-routing.test.js b/packages/astro/test/preview-routing.test.js index 8c94a6879f0a..4f382bd8b62f 100644 --- a/packages/astro/test/preview-routing.test.js +++ b/packages/astro/test/preview-routing.test.js @@ -30,15 +30,15 @@ describe('Preview Routing', () => { expect(response.status).to.equal(404); }); - it('404 when loading subpath root with trailing slash', async () => { + it('200 when loading subpath root with trailing slash', async () => { const response = await fixture.fetch('/blog/'); - expect(response.status).to.equal(404); + expect(response.status).to.equal(200); + expect(response.redirected).to.equal(false); }); - it('200 when loading subpath root without trailing slash', async () => { + it('404 when loading subpath root without trailing slash', async () => { const response = await fixture.fetch('/blog'); - expect(response.status).to.equal(200); - expect(response.redirected).to.equal(false); + expect(response.status).to.equal(404); }); it('404 when loading another page with subpath used', async () => { @@ -92,7 +92,6 @@ describe('Preview Routing', () => { it('404 when loading subpath root without trailing slash', async () => { const response = await fixture.fetch('/blog'); expect(response.status).to.equal(404); - expect(response.redirected).to.equal(false); }); it('200 when loading another page with subpath used', async () => { @@ -148,10 +147,9 @@ describe('Preview Routing', () => { expect(response.status).to.equal(200); }); - it('200 when loading subpath root without trailing slash', async () => { + it('404 when loading subpath root without trailing slash', async () => { const response = await fixture.fetch('/blog'); - expect(response.status).to.equal(200); - expect(response.redirected).to.equal(false); + expect(response.status).to.equal(404); }); it('200 when loading another page with subpath used', async () => { @@ -207,15 +205,15 @@ describe('Preview Routing', () => { expect(response.status).to.equal(404); }); - it('404 when loading subpath root with trailing slash', async () => { + it('200 when loading subpath root with trailing slash', async () => { const response = await fixture.fetch('/blog/'); - expect(response.status).to.equal(404); + expect(response.status).to.equal(200); + expect(response.redirected).to.equal(false); }); - it('200 when loading subpath root without trailing slash', async () => { + it('404 when loading subpath root without trailing slash', async () => { const response = await fixture.fetch('/blog'); - expect(response.status).to.equal(200); - expect(response.redirected).to.equal(false); + expect(response.status).to.equal(404); }); it('404 when loading another page with subpath used', async () => { @@ -272,7 +270,6 @@ describe('Preview Routing', () => { it('404 when loading subpath root without trailing slash', async () => { const response = await fixture.fetch('/blog'); expect(response.status).to.equal(404); - expect(response.redirected).to.equal(false); }); it('200 when loading another page with subpath used', async () => { @@ -331,10 +328,9 @@ describe('Preview Routing', () => { expect(response.status).to.equal(200); }); - it('200 when loading subpath root without trailing slash', async () => { + it('404 when loading subpath root without trailing slash', async () => { const response = await fixture.fetch('/blog'); - expect(response.status).to.equal(200); - expect(response.redirected).to.equal(false); + expect(response.status).to.equal(404); }); it('200 when loading another page with subpath used', async () => {