refactor: FileResolver should not handle URL parsing

lib/handler.js

@@ -8,28 +8,22 @@ import { getContentType, typeForFilePath } from './content-type.js';
 import { getLocalPath, isSubpath } from './fs-utils.js';
 import { dirListPage, errorPage } from './pages.js';
 import { PathMatcher } from './path-matcher.js';
-import { headerCase } from './utils.js';
+import { headerCase, trimSlash } from './utils.js';
 
 /**
+@typedef {import('node:http').IncomingMessage & {originalUrl?: string}} Request
+@typedef {import('node:http').ServerResponse<Request>} Response
 @typedef {import('./types.d.ts').FSLocation} FSLocation
 @typedef {import('./types.d.ts').ResMetaData} ResMetaData
 @typedef {import('./types.d.ts').ServerOptions} ServerOptions
-*/
-
-/**
-@typedef {{
-	req: import('node:http').IncomingMessage;
-	res: import('node:http').ServerResponse<import('node:http').IncomingMessage>;
-	resolver: import('./resolver.js').FileResolver;
-	options: ServerOptions & {_noStream?: boolean}
-}} ReqHandlerConfig
 @typedef {{
 	body?: string | Buffer | import('node:fs').ReadStream;
 	contentType?: string;
 	isText?: boolean;
 	statSize?: number;
 }} SendPayload
 */
+
 export class RequestHandler {
 	#req;
 	#res;
@@ -38,25 +32,33 @@ export class RequestHandler {
 
 	/** @type {ResMetaData['timing']} */
 	timing = { start: Date.now() };
-	/** @type {string} */
-	urlPath = '';
+	/** @type {string | null} */
+	urlPath = null;
 	/** @type {FSLocation | null} */
 	file = null;
-	/**
-	Error that may be logged to the terminal
-	@type {Error | string | undefined}
-	*/
+	/** @type {Error | string | undefined} */
 	error;
 
-	/** @param {ReqHandlerConfig} config */
+	/**
+	@param {{
+		req: Request;
+		res: Response;
+		resolver: import('./resolver.js').FileResolver;
+		options: ServerOptions & {_noStream?: boolean};
+	}} config
+	*/
 	constructor({ req, res, resolver, options }) {
 		this.#req = req;
 		this.#res = res;
 		this.#resolver = resolver;
 		this.#options = options;
-		if (typeof req.url === 'string') {
-			this.urlPath = req.url.split(/[\?\#]/)[0];
+
+		try {
+			this.urlPath = extractUrlPath(req.url ?? '');
+		} catch(/** @type {any} */ err) {
+			this.error = err;
 		}
+
 		res.on('close', () => {
 			this.timing.close = Date.now();
 		});
@@ -97,9 +99,13 @@ export class RequestHandler {
 			return this.#send();
 		}
 
-		const { status, urlPath, file = null } = await this.#resolver.find(this.urlPath);
+		if (this.urlPath == null) {
+			this.status = 400;
+			return this.#sendErrorPage();
+		}
+
+		const { status, file } = await this.#resolver.find(trimSlash(this.urlPath));
 		this.status = status;
-		this.urlPath = urlPath;
 		this.file = file;
 
 		// found a file to serve
@@ -169,14 +175,18 @@ export class RequestHandler {
 			cors: false,
 			headers: [],
 		});
-
 		if (this.method === 'OPTIONS') {
 			this.status = 204;
 			return this.#send();
 		}
-
 		const items = await this.#resolver.index(filePath);
-		const body = await dirListPage({ urlPath: this.urlPath, filePath, items }, this.#options);
+		const body = await dirListPage({
+			root: this.#options.root,
+			ext: this.#options.ext,
+			urlPath: this.urlPath ?? '',
+			filePath,
+			items,
+		});
 		return this.#send({ body, isText: true });
 	}
 
@@ -186,15 +196,15 @@ export class RequestHandler {
 			cors: this.#options.cors,
 			headers: [],
 		});
-
 		if (this.method === 'OPTIONS') {
 			return this.#send();
 		}
-
-		return this.#send({
-			body: await errorPage({ status: this.status, urlPath: this.urlPath }),
-			isText: true,
+		const body = await errorPage({
+			status: this.status,
+			url: this.#req.url ?? '',
+			urlPath: this.urlPath,
 		});
+		return this.#send({ body, isText: true });
 	}
 
 	/** @type {(payload?: SendPayload) => void} */
@@ -315,6 +325,7 @@ export class RequestHandler {
 		return {
 			status: this.status,
 			method: this.method,
+			url: this.#req.url ?? '',
 			urlPath: this.urlPath,
 			localPath: this.localPath,
 			timing: structuredClone(this.timing),
@@ -373,3 +384,26 @@ function parseHeaderNames(input = '') {
 		.map((h) => h.trim())
 		.filter(isHeader);
 }
+
+/** @type {(url: string) => string} */
+export function extractUrlPath(url) {
+	if (url === '*') return url;
+	const path = new URL(url, 'http://localhost/').pathname || '/';
+	if (!isValidUrlPath(path)) {
+		throw new Error(`Invalid URL path: '${path}'`)
+	}
+	return path;
+}
+
+/** @type {(urlPath: string) => boolean} */
+export function isValidUrlPath(urlPath) {
+	if (urlPath === '/') return true;
+	if (!urlPath.startsWith('/') || urlPath.includes('//')) return false;
+	for (const s of trimSlash(urlPath).split('/')) {
+		const d = decodeURIComponent(s);
+		if (d === '.' || d === '..') return false;
+		if (s.includes('?') || s.includes('#')) return false;
+		if (d.includes('/') || d.includes('\\')) return false;
+	}
+	return true;
+}

lib/logger.js

@@ -116,16 +116,16 @@ class Logger {
 }
 
 /** @type {(data: import('./types.d.ts').ResMetaData) => string} */
-export function requestLogLine({ status, method, urlPath, localPath, timing, error }) {
+export function requestLogLine({ status, method, url, urlPath, localPath, timing, error }) {
 	const { start, close } = timing;
 	const { style: _, brackets } = color;
 
 	const isSuccess = status >= 200 && status < 300;
 	const timestamp = start ? new Date(start).toTimeString().split(' ')[0]?.padStart(8) : undefined;
 	const duration = start && close ? Math.ceil(close - start) : undefined;
 
-	let displayPath = _(urlPath, 'cyan');
-	if (isSuccess && localPath != null) {
+	let displayPath = _(urlPath ?? url, 'cyan');
+	if (isSuccess && urlPath != null && localPath != null) {
 		const basePath = urlPath.length > 1 ? trimSlash(urlPath, { end: true }) : urlPath;
 		const suffix = pathSuffix(basePath, `/${fwdSlash(localPath)}`);
 		if (suffix) {

lib/pages.js

@@ -5,7 +5,6 @@ import { clamp, escapeHtml, trimSlash } from './utils.js';
 
 /**
 @typedef {import('./types.d.ts').FSLocation} FSLocation
-@typedef {import('./types.d.ts').ServerOptions} ServerOptions
 */
 
 /**
@@ -32,11 +31,10 @@ ${body}
 }
 
 /**
-@param {{ status: number, urlPath: string }} data
-@returns {Promise<string>}
+@param {{ status: number; url: string; urlPath: string | null }} data
 */
-export function errorPage({ status, urlPath }) {
-	const displayPath = decodeURIPathSegments(urlPath);
+export function errorPage({ status, url, urlPath }) {
+	const displayPath = decodeURIPathSegments(urlPath ?? url);
 	const pathHtml = `<code class="filepath">${html(nl2sp(displayPath))}</code>`;
 
 	const page = (title = '', desc = '') => {
@@ -45,6 +43,8 @@ export function errorPage({ status, urlPath }) {
 	};
 
 	switch (status) {
+		case 400:
+			return page('400: Bad request', `Invalid request for ${pathHtml}`);
 		case 403:
 			return page('403: Forbidden', `Could not access ${pathHtml}`);
 		case 404:
@@ -59,12 +59,10 @@ export function errorPage({ status, urlPath }) {
 }
 
 /**
-@param {{ urlPath: string; filePath: string; items: FSLocation[] }} data
-@param {Pick<ServerOptions, 'root' | 'ext'>} options
-@returns {Promise<string>}
+@param {{ root: string; urlPath: string; filePath: string; items: FSLocation[]; ext: string[] }} data
 */
-export function dirListPage({ urlPath, filePath, items }, options) {
-	const rootName = basename(options.root);
+export function dirListPage({ root, urlPath, filePath, items, ext }) {
+	const rootName = basename(root);
 	const trimmedUrl = trimSlash(urlPath);
 	const baseUrl = trimmedUrl ? `/${trimmedUrl}/` : '/';
 
@@ -89,14 +87,15 @@ export function dirListPage({ urlPath, filePath, items }, options) {
 	Index of <span class="bc">${renderBreadcrumbs(displayPath)}</span>
 </h1>
 <ul class="files" style="--max-col-count:${maxCols}">
-${sorted.map((item) => renderListItem(item, { ext: options.ext, parentPath })).join('\n')}
+${sorted.map((item) => renderListItem(item, { ext, parentPath })).join('\n')}
 </ul>
 `.trim(),
 	});
 }
 
 /**
-@type {(item: FSLocation, options: { ext: ServerOptions['ext']; parentPath: string }) => string}
+@param {FSLocation} item
+@param {{ ext: string[]; parentPath: string }} options
 */
 function renderListItem(item, { ext, parentPath }) {
 	const isDir = isDirLike(item);

lib/resolver.js

@@ -2,7 +2,7 @@ import { isAbsolute, join } from 'node:path';
 
 import { getIndex, getKind, getLocalPath, getRealpath, isReadable, isSubpath } from './fs-utils.js';
 import { PathMatcher } from './path-matcher.js';
-import { fwdSlash, trimSlash } from './utils.js';
+import { trimSlash } from './utils.js';
 
 /**
 @typedef {import('./types.d.ts').FSLocation} FSLocation
@@ -39,13 +39,15 @@ export class FileResolver {
 		this.#excludeMatcher = new PathMatcher(options.exclude ?? [], { caseSensitive: true });
 	}
 
-	/** @param {string} url */
-	async find(url) {
-		const { urlPath, filePath: targetPath } = resolveUrlPath(this.#root, url);
-
-		/** @type {{status: number; urlPath: string; file?: FSLocation}} */
-		const result = { status: 404, urlPath };
+	/** @param {string} relativePath */
+	async find(relativePath) {
+		const result = {
+			status: 404,
+			/** @type {FSLocation | null} */
+			file: null,
+		};
 
+		const targetPath = this.resolvePath(relativePath);
 		if (targetPath == null) {
 			return result;
 		}
@@ -144,42 +146,14 @@ export class FileResolver {
 		return this.#excludeMatcher.test(localPath) === false;
 	}
 
-	/** @type {(urlPath: string | null) => string | null} */
-	urlToTargetPath(urlPath) {
-		if (urlPath && urlPath.startsWith('/')) {
-			const filePath = join(this.#root, decodeURIComponent(urlPath));
-			return trimSlash(filePath, { end: true });
-		}
-		return null;
+	/** @type {(relativePath: string) => string | null} */
+	resolvePath(relativePath) {
+		const filePath = join(this.#root, relativePath);
+		return this.withinRoot(filePath) ? trimSlash(filePath, { end: true }) : null;
 	}
 
 	/** @type {(filePath: string) => boolean} */
 	withinRoot(filePath) {
 		return isSubpath(this.#root, filePath);
 	}
 }
-
-/** @type {(urlPath: string) => boolean} */
-export function isValidUrlPath(urlPath) {
-	if (urlPath === '/') return true;
-	if (!urlPath.startsWith('/') || urlPath.includes('//')) return false;
-	for (const s of trimSlash(urlPath).split('/')) {
-		const d = decodeURIComponent(s);
-		if (d === '.' || d === '..') return false;
-		if (s.includes('?') || s.includes('#')) return false;
-		if (d.includes('/') || d.includes('\\')) return false;
-	}
-	return true;
-}
-
-/** @type {(root: url, url: string) => {urlPath: string; filePath: string | null}} */
-export function resolveUrlPath(root, url) {
-	try {
-		const urlPath = fwdSlash(new URL(url, 'http://localhost/').pathname) ?? '/';
-		const filePath = isValidUrlPath(urlPath)
-			? trimSlash(join(root, decodeURIComponent(urlPath)), { end: true })
-			: null;
-		return { urlPath, filePath };
-	} catch {}
-	return { urlPath: url, filePath: null };
-}

lib/types.d.ts

@@ -21,7 +21,8 @@ export interface OptionSpec {
 export interface ResMetaData {
 	method: string;
 	status: number;
-	urlPath: string;
+	url: string;
+	urlPath: string | null;
 	localPath: string | null;
 	timing: { start: number; send?: number; close?: number };
 	error?: Error | string;