From 572f5653fb2b3462c1334d5daf6839101636a3c9 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Sat, 24 Jan 2026 20:54:38 -0500 Subject: [PATCH 1/5] fix: update getFilenameFromUrl to return errorCode and filename --- src/index.js | 2 +- src/middleware.js | 14 ++++++-------- src/utils/getFilenameFromUrl.js | 18 +++++++++--------- 3 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/index.js b/src/index.js index ccdc8fedc..09aabf68e 100644 --- a/src/index.js +++ b/src/index.js @@ -279,7 +279,7 @@ function wdm(compiler, options = {}) { // API instance.getFilenameFromUrl = (url, extra) => - getFilenameFromUrl(filledContext, url, extra); + getFilenameFromUrl(filledContext, url, extra)?.filename; instance.waitUntilValid = (callback = noop) => { ready(filledContext, callback); diff --git a/src/middleware.js b/src/middleware.js index 3441f3285..afe955ab8 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -498,22 +498,20 @@ function wrapper(context) { */ async function processRequest() { // Pipe and SendFile - /** @type {import("./utils/getFilenameFromUrl").Extra} */ - const extra = {}; - const filename = getFilenameFromUrl( + const { filename, extra, errorCode } = getFilenameFromUrl( context, /** @type {string} */ (getRequestURL(req)), - extra, + {}, ); - if (extra.errorCode) { - if (extra.errorCode === 403) { + if (errorCode) { + if (errorCode === 403) { context.logger.error(`Malicious path "${filename}".`); } await sendError( - extra.errorCode === 400 ? "Bad Request" : "Forbidden", - extra.errorCode, + errorCode === 400 ? "Bad Request" : "Forbidden", + errorCode, { modifyResponseData: context.options.modifyResponseData, }, diff --git a/src/utils/getFilenameFromUrl.js b/src/utils/getFilenameFromUrl.js index 44c7b7bd6..6c6e3986d 100644 --- a/src/utils/getFilenameFromUrl.js +++ b/src/utils/getFilenameFromUrl.js @@ -30,7 +30,6 @@ const UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/; /** * @typedef {object} Extra * @property {import("fs").Stats=} stats stats - * @property {number=} errorCode error code * @property {boolean=} immutable true when immutable, otherwise false */ @@ -42,7 +41,6 @@ const UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/; * @returns {string} */ -// TODO refactor me in the next major release, this function should return `{ filename, stats, error }` // TODO fix redirect logic when `/` at the end, like https://github.com/pillarjs/send/blob/master/index.js#L586 /** * @template {IncomingMessage} Request @@ -50,7 +48,7 @@ const UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/; * @param {import("../index.js").FilledContext} context context * @param {string} url url * @param {Extra=} extra extra - * @returns {string | undefined} filename + * @returns {{ filename?: string, extra: Extra, errorCode?: number }} filename */ function getFilenameFromUrl(context, url, extra = {}) { const { options } = context; @@ -58,6 +56,8 @@ function getFilenameFromUrl(context, url, extra = {}) { /** @type {string | undefined} */ let foundFilename; + /** @type {number | undefined} */ + let errorCode; /** @type {import("node:url").Url} */ let urlObject; @@ -65,7 +65,7 @@ function getFilenameFromUrl(context, url, extra = {}) { // The `url` property of the `request` is contains only `pathname`, `search` and `hash` urlObject = memoizedParse(url, false, true); } catch { - return; + return { errorCode, filename: foundFilename, extra }; } for (const { publicPath, outputPath, assetsInfo } of paths) { @@ -94,16 +94,16 @@ function getFilenameFromUrl(context, url, extra = {}) { ) { // Null byte(s) if (pathname.includes("\0")) { - extra.errorCode = 400; + errorCode = 400; - return; + return { errorCode, filename: foundFilename, extra }; } // ".." is malicious if (UP_PATH_REGEXP.test(path.normalize(`./${pathname}`))) { - extra.errorCode = 403; + errorCode = 403; - return; + return { errorCode, filename: foundFilename, extra }; } // Strip the `pathname` property from the `publicPath` option from the start of requested url @@ -161,7 +161,7 @@ function getFilenameFromUrl(context, url, extra = {}) { } } - return foundFilename; + return { filename: foundFilename, extra, errorCode }; } module.exports = getFilenameFromUrl; From 58ca1cf7ca5b70b46efd00797e620ab65c1282e9 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Tue, 27 Jan 2026 02:10:50 +0000 Subject: [PATCH 2/5] refactor: remove extra of arguments --- README.md | 2 +- src/index.js | 6 +-- src/middleware.js | 1 - src/utils/getFilenameFromUrl.js | 7 +-- test/middleware.test.js | 80 +++++++++++++++++------------ types/index.d.ts | 12 ++--- types/utils/getFilenameFromUrl.d.ts | 15 +++--- 7 files changed, 66 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index 4a7cfbbf2..67e50ff9e 100644 --- a/README.md +++ b/README.md @@ -459,7 +459,7 @@ const app = new express(); app.use(instance); instance.waitUntilValid(() => { - const filename = instance.getFilenameFromUrl("/bundle.js"); + const { filename } = instance.getFilenameFromUrl("/bundle.js"); console.log(`Filename is ${filename}`); }); diff --git a/src/index.js b/src/index.js index 09aabf68e..292fe24ac 100644 --- a/src/index.js +++ b/src/index.js @@ -142,8 +142,7 @@ const noop = () => {}; /** * @callback GetFilenameFromUrl * @param {string} url - * @param {Extra=} extra - * @returns {string | undefined} + * @returns {{ filename?: string, extra: Extra, errorCode?: number }} */ /** @@ -278,8 +277,7 @@ function wdm(compiler, options = {}) { (middleware(filledContext)); // API - instance.getFilenameFromUrl = (url, extra) => - getFilenameFromUrl(filledContext, url, extra)?.filename; + instance.getFilenameFromUrl = (url) => getFilenameFromUrl(filledContext, url); instance.waitUntilValid = (callback = noop) => { ready(filledContext, callback); diff --git a/src/middleware.js b/src/middleware.js index afe955ab8..1d87e1d85 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -501,7 +501,6 @@ function wrapper(context) { const { filename, extra, errorCode } = getFilenameFromUrl( context, /** @type {string} */ (getRequestURL(req)), - {}, ); if (errorCode) { diff --git a/src/utils/getFilenameFromUrl.js b/src/utils/getFilenameFromUrl.js index 6c6e3986d..0c5e2702f 100644 --- a/src/utils/getFilenameFromUrl.js +++ b/src/utils/getFilenameFromUrl.js @@ -47,10 +47,11 @@ const UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/; * @template {ServerResponse} Response * @param {import("../index.js").FilledContext} context context * @param {string} url url - * @param {Extra=} extra extra - * @returns {{ filename?: string, extra: Extra, errorCode?: number }} filename + * @returns {{ filename?: string, extra: Extra, errorCode?: number }} result of get filename from url */ -function getFilenameFromUrl(context, url, extra = {}) { +function getFilenameFromUrl(context, url) { + /** @type {Extra} */ + const extra = {}; const { options } = context; const paths = getPaths(context); diff --git a/test/middleware.test.js b/test/middleware.test.js index 4e076031b..ee2018258 100644 --- a/test/middleware.test.js +++ b/test/middleware.test.js @@ -573,23 +573,24 @@ describe.each([ it("should work", (done) => { instance.waitUntilValid(() => { - expect(instance.getFilenameFromUrl("/bundle.js")).toBe( + expect(instance.getFilenameFromUrl("/bundle.js").filename).toBe( path.join(webpackConfig.output.path, "/bundle.js"), ); - expect(instance.getFilenameFromUrl("/")).toBe( + expect(instance.getFilenameFromUrl("/").filename).toBe( path.join(webpackConfig.output.path, "/index.html"), ); - expect(instance.getFilenameFromUrl("/index.html")).toBe( + expect(instance.getFilenameFromUrl("/index.html").filename).toBe( path.join(webpackConfig.output.path, "/index.html"), ); - expect(instance.getFilenameFromUrl("/svg.svg")).toBe( + expect(instance.getFilenameFromUrl("/svg.svg").filename).toBe( path.join(webpackConfig.output.path, "/svg.svg"), ); expect( - instance.getFilenameFromUrl("/unknown.unknown"), + instance.getFilenameFromUrl("/unknown.unknown").filename, ).toBeUndefined(); expect( - instance.getFilenameFromUrl("/unknown/unknown.unknown"), + instance.getFilenameFromUrl("/unknown/unknown.unknown") + .filename, ).toBeUndefined(); done(); @@ -617,22 +618,23 @@ describe.each([ it("should work", (done) => { instance.waitUntilValid(() => { - expect(instance.getFilenameFromUrl("/bundle.js")).toBe( + expect(instance.getFilenameFromUrl("/bundle.js").filename).toBe( path.join(webpackConfig.output.path, "/bundle.js"), ); - expect(instance.getFilenameFromUrl("/")).toBeUndefined(); - expect(instance.getFilenameFromUrl("/index.html")).toBe( + expect(instance.getFilenameFromUrl("/").filename).toBeUndefined(); + expect(instance.getFilenameFromUrl("/index.html").filename).toBe( path.join(webpackConfig.output.path, "/index.html"), ); - expect(instance.getFilenameFromUrl("/svg.svg")).toBe( + expect(instance.getFilenameFromUrl("/svg.svg").filename).toBe( path.join(webpackConfig.output.path, "/svg.svg"), ); expect( - instance.getFilenameFromUrl("/unknown.unknown"), + instance.getFilenameFromUrl("/unknown.unknown").filename, ).toBeUndefined(); expect( - instance.getFilenameFromUrl("/unknown/unknown.unknown"), + instance.getFilenameFromUrl("/unknown/unknown.unknown") + .filename, ).toBeUndefined(); done(); @@ -658,28 +660,33 @@ describe.each([ it("should work", (done) => { instance.waitUntilValid(() => { expect( - instance.getFilenameFromUrl("/public/path/bundle.js"), + instance.getFilenameFromUrl("/public/path/bundle.js").filename, ).toBe( path.join(webpackPublicPathConfig.output.path, "/bundle.js"), ); - expect(instance.getFilenameFromUrl("/public/path/")).toBe( + expect( + instance.getFilenameFromUrl("/public/path/").filename, + ).toBe( path.join(webpackPublicPathConfig.output.path, "/index.html"), ); expect( - instance.getFilenameFromUrl("/public/path/index.html"), + instance.getFilenameFromUrl("/public/path/index.html").filename, ).toBe( path.join(webpackPublicPathConfig.output.path, "/index.html"), ); - expect(instance.getFilenameFromUrl("/public/path/svg.svg")).toBe( + expect( + instance.getFilenameFromUrl("/public/path/svg.svg").filename, + ).toBe( path.join(webpackPublicPathConfig.output.path, "/svg.svg"), ); - expect(instance.getFilenameFromUrl("/")).toBeUndefined(); + expect(instance.getFilenameFromUrl("/").filename).toBeUndefined(); expect( - instance.getFilenameFromUrl("/unknown.unknown"), + instance.getFilenameFromUrl("/unknown.unknown").filename, ).toBeUndefined(); expect( - instance.getFilenameFromUrl("/unknown/unknown.unknown"), + instance.getFilenameFromUrl("/unknown/unknown.unknown") + .filename, ).toBeUndefined(); done(); @@ -704,49 +711,56 @@ describe.each([ it("should work", (done) => { instance.waitUntilValid(() => { - expect(instance.getFilenameFromUrl("/static-one/bundle.js")).toBe( + expect( + instance.getFilenameFromUrl("/static-one/bundle.js").filename, + ).toBe( path.join(webpackMultiConfig[0].output.path, "/bundle.js"), ); - expect(instance.getFilenameFromUrl("/static-one/")).toBe( + expect(instance.getFilenameFromUrl("/static-one/").filename).toBe( path.join(webpackMultiConfig[0].output.path, "/index.html"), ); expect( - instance.getFilenameFromUrl("/static-one/index.html"), + instance.getFilenameFromUrl("/static-one/index.html").filename, ).toBe( path.join(webpackMultiConfig[0].output.path, "/index.html"), ); - expect(instance.getFilenameFromUrl("/static-one/svg.svg")).toBe( - path.join(webpackMultiConfig[0].output.path, "/svg.svg"), - ); expect( - instance.getFilenameFromUrl("/static-one/unknown.unknown"), + instance.getFilenameFromUrl("/static-one/svg.svg").filename, + ).toBe(path.join(webpackMultiConfig[0].output.path, "/svg.svg")); + expect( + instance.getFilenameFromUrl("/static-one/unknown.unknown") + .filename, ).toBeUndefined(); expect( instance.getFilenameFromUrl( "/static-one/unknown/unknown.unknown", - ), + ).filename, ).toBeUndefined(); - expect(instance.getFilenameFromUrl("/static-two/bundle.js")).toBe( + expect( + instance.getFilenameFromUrl("/static-two/bundle.js").filename, + ).toBe( path.join(webpackMultiConfig[1].output.path, "/bundle.js"), ); expect( - instance.getFilenameFromUrl("/static-two/unknown.unknown"), + instance.getFilenameFromUrl("/static-two/unknown.unknown") + .filename, ).toBeUndefined(); expect( instance.getFilenameFromUrl( "/static-two/unknown/unknown.unknown", - ), + ).filename, ).toBeUndefined(); - expect(instance.getFilenameFromUrl("/")).toBeUndefined(); + expect(instance.getFilenameFromUrl("/").filename).toBeUndefined(); expect( - instance.getFilenameFromUrl("/static-one/unknown.unknown"), + instance.getFilenameFromUrl("/static-one/unknown.unknown") + .filename, ).toBeUndefined(); expect( instance.getFilenameFromUrl( "/static-one/unknown/unknown.unknown", - ), + ).filename, ).toBeUndefined(); done(); diff --git a/types/index.d.ts b/types/index.d.ts index 140425b45..6259d73ab 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -108,8 +108,7 @@ export = wdm; /** * @callback GetFilenameFromUrl * @param {string} url - * @param {Extra=} extra - * @returns {string | undefined} + * @returns {{ filename?: string, extra: Extra, errorCode?: number }} */ /** * @callback WaitUntilValid @@ -460,10 +459,11 @@ type Middleware< next: NextFunction, ) => Promise; type Extra = import("./utils/getFilenameFromUrl").Extra; -type GetFilenameFromUrl = ( - url: string, - extra?: Extra | undefined, -) => string | undefined; +type GetFilenameFromUrl = (url: string) => { + filename?: string; + extra: Extra; + errorCode?: number; +}; type WaitUntilValid = (callback: Callback) => any; type Invalidate = (callback: Callback) => any; type Close = (callback: (err: Error | null | undefined) => void) => any; diff --git a/types/utils/getFilenameFromUrl.d.ts b/types/utils/getFilenameFromUrl.d.ts index be28a0ae0..13f2a3185 100644 --- a/types/utils/getFilenameFromUrl.d.ts +++ b/types/utils/getFilenameFromUrl.d.ts @@ -2,7 +2,6 @@ export = getFilenameFromUrl; /** * @typedef {object} Extra * @property {import("fs").Stats=} stats stats - * @property {number=} errorCode error code * @property {boolean=} immutable true when immutable, otherwise false */ /** @@ -17,8 +16,7 @@ export = getFilenameFromUrl; * @template {ServerResponse} Response * @param {import("../index.js").FilledContext} context context * @param {string} url url - * @param {Extra=} extra extra - * @returns {string | undefined} filename + * @returns {{ filename?: string, extra: Extra, errorCode?: number }} result of get filename from url */ declare function getFilenameFromUrl< Request extends IncomingMessage, @@ -26,8 +24,11 @@ declare function getFilenameFromUrl< >( context: import("../index.js").FilledContext, url: string, - extra?: Extra | undefined, -): string | undefined; +): { + filename?: string; + extra: Extra; + errorCode?: number; +}; declare namespace getFilenameFromUrl { export { IncomingMessage, ServerResponse, Extra }; } @@ -38,10 +39,6 @@ type Extra = { * stats */ stats?: import("fs").Stats | undefined; - /** - * error code - */ - errorCode?: number | undefined; /** * true when immutable, otherwise false */ From d3d6299d2d39822042ee453f0f5a988970ea6d01 Mon Sep 17 00:00:00 2001 From: Sebastian Beltran Date: Tue, 27 Jan 2026 04:28:56 +0000 Subject: [PATCH 3/5] feat: use new URL --- src/utils/getFilenameFromUrl.js | 34 ++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/utils/getFilenameFromUrl.js b/src/utils/getFilenameFromUrl.js index 0c5e2702f..f1ec7d5a2 100644 --- a/src/utils/getFilenameFromUrl.js +++ b/src/utils/getFilenameFromUrl.js @@ -1,7 +1,5 @@ const path = require("node:path"); const querystring = require("node:querystring"); -// eslint-disable-next-line n/no-deprecated-api -const { parse } = require("node:url"); const getPaths = require("./getPaths"); const memorize = require("./memorize"); @@ -17,13 +15,25 @@ function decode(input) { return querystring.unescape(input); } -const memoizedParse = memorize(parse, undefined, (value) => { - if (value.pathname) { - value.pathname = decode(value.pathname); - } +// const memoizedParse = memorize(parse, +// undefined, +// (value) => { +// console.log({encode: value.pathname}); +// if (value.pathname) { +// value.pathname = decode(value.pathname); +// } +// console.log({decode: value.pathname}); + +// return value; +// }, +// ); + +const memoizedParse = memorize((url) => { + const urlObject = new URL(url, "http://localhost"); - return value; -}); + // We cann't change pathname in URL object directly because don't decode correctly + return { ...urlObject, pathname: decode(urlObject.pathname) }; +}, undefined); const UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/; @@ -59,12 +69,12 @@ function getFilenameFromUrl(context, url) { let foundFilename; /** @type {number | undefined} */ let errorCode; - /** @type {import("node:url").Url} */ + /** @type {import("node:url").URL} */ let urlObject; try { // The `url` property of the `request` is contains only `pathname`, `search` and `hash` - urlObject = memoizedParse(url, false, true); + urlObject = memoizedParse(url); } catch { return { errorCode, filename: foundFilename, extra }; } @@ -72,14 +82,12 @@ function getFilenameFromUrl(context, url) { for (const { publicPath, outputPath, assetsInfo } of paths) { /** @type {string | undefined} */ let filename; - /** @type {import("node:url").Url} */ + /** @type {import("node:url").URL} */ let publicPathObject; try { publicPathObject = memoizedParse( publicPath !== "auto" && publicPath ? publicPath : "/", - false, - true, ); } catch { continue; From 967aaca6db50d7c717d6177b701f460d54324571 Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Tue, 27 Jan 2026 16:51:32 +0300 Subject: [PATCH 4/5] refactor: code --- src/index.js | 10 +++--- src/middleware.js | 35 ++++++++++++++------- src/utils/getFilenameFromUrl.js | 49 ++++++++++++++++++----------- types/index.d.ts | 21 +++++++------ types/middleware.d.ts | 4 +++ types/utils/getFilenameFromUrl.d.ts | 49 +++++++++++++++++------------ 6 files changed, 103 insertions(+), 65 deletions(-) diff --git a/src/index.js b/src/index.js index 292fe24ac..de1403f83 100644 --- a/src/index.js +++ b/src/index.js @@ -131,9 +131,9 @@ const noop = () => {}; * @template {IncomingMessage} [RequestInternal=IncomingMessage] * @template {ServerResponse} [ResponseInternal=ServerResponse] * @callback Middleware - * @param {RequestInternal} req - * @param {ResponseInternal} res - * @param {NextFunction} next + * @param {RequestInternal} req request + * @param {ResponseInternal} res response + * @param {NextFunction} next next function * @returns {Promise} */ @@ -141,8 +141,8 @@ const noop = () => {}; /** * @callback GetFilenameFromUrl - * @param {string} url - * @returns {{ filename?: string, extra: Extra, errorCode?: number }} + * @param {string} url request URL + * @returns {{ filename: string, extra: Extra } | undefined} a filename with additional information, or `undefined` if nothing is found */ /** diff --git a/src/middleware.js b/src/middleware.js index 1d87e1d85..078bbead7 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -31,6 +31,8 @@ const ready = require("./utils/ready"); /** @typedef {import("./index.js").IncomingMessage} IncomingMessage */ /** @typedef {import("./index.js").ServerResponse} ServerResponse */ /** @typedef {import("./index.js").NormalizedHeaders} NormalizedHeaders */ +/** @typedef {import("./utils/getFilenameFromUrl.js").FilenameError} FilenameError */ +/** @typedef {import("./utils/getFilenameFromUrl.js").Extra} Extra */ /** @typedef {import("fs").ReadStream} ReadStream */ const BYTES_RANGE_REGEXP = /^ *bytes/i; @@ -498,14 +500,24 @@ function wrapper(context) { */ async function processRequest() { // Pipe and SendFile - const { filename, extra, errorCode } = getFilenameFromUrl( - context, - /** @type {string} */ (getRequestURL(req)), - ); + /** @type {{ filename: string, extra: Extra } | undefined} */ + let resolved; + + const requestUrl = /** @type {string} */ (getRequestURL(req)); + + try { + resolved = getFilenameFromUrl(context, requestUrl); + } catch (err) { + // Fallback to 403 for unknown errors + const errorCode = + typeof err === "object" && + err !== null && + typeof (/** @type {FilenameError} */ (err).code) !== "undefined" + ? /** @type {FilenameError} */ (err).code + : 403; - if (errorCode) { if (errorCode === 403) { - context.logger.error(`Malicious path "${filename}".`); + context.logger.error(`Malicious path "${requestUrl}".`); } await sendError( @@ -518,7 +530,7 @@ function wrapper(context) { return; } - if (!filename) { + if (!resolved) { await goNext(); return; } @@ -528,7 +540,8 @@ function wrapper(context) { return; } - const { size } = /** @type {import("fs").Stats} */ (extra.stats); + const { extra, filename } = resolved; + const { size } = extra.stats; let len = size; let offset = 0; @@ -606,9 +619,7 @@ function wrapper(context) { context.options.lastModified && !getResponseHeader(res, "Last-Modified") ) { - const modified = - /** @type {import("fs").Stats} */ - (extra.stats).mtime.toUTCString(); + const modified = extra.stats.mtime.toUTCString(); setResponseHeader(res, "Last-Modified", modified); } @@ -664,7 +675,7 @@ function wrapper(context) { const result = await getETag()( isStrongETag ? /** @type {Buffer | ReadStream} */ (bufferOrStream) - : /** @type {import("fs").Stats} */ (extra.stats), + : extra.stats, ); // Because we already read stream, we can cache buffer to avoid extra read from fs diff --git a/src/utils/getFilenameFromUrl.js b/src/utils/getFilenameFromUrl.js index f1ec7d5a2..a39729b70 100644 --- a/src/utils/getFilenameFromUrl.js +++ b/src/utils/getFilenameFromUrl.js @@ -39,7 +39,7 @@ const UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/; /** * @typedef {object} Extra - * @property {import("fs").Stats=} stats stats + * @property {import("fs").Stats} stats stats * @property {boolean=} immutable true when immutable, otherwise false */ @@ -51,34 +51,46 @@ const UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/; * @returns {string} */ +class FilenameError extends Error { + /** + * @param {string} message message + * @param {number=} code error code + */ + constructor(message, code) { + super(message); + this.name = "FilenameError"; + this.code = code; + } +} + // TODO fix redirect logic when `/` at the end, like https://github.com/pillarjs/send/blob/master/index.js#L586 /** * @template {IncomingMessage} Request * @template {ServerResponse} Response * @param {import("../index.js").FilledContext} context context * @param {string} url url - * @returns {{ filename?: string, extra: Extra, errorCode?: number }} result of get filename from url + * @returns {{ filename: string, extra: Extra } | undefined} result of get filename from url */ function getFilenameFromUrl(context, url) { - /** @type {Extra} */ - const extra = {}; - const { options } = context; - const paths = getPaths(context); + /** @type {import("node:url").URL} */ + let urlObject; /** @type {string | undefined} */ let foundFilename; - /** @type {number | undefined} */ - let errorCode; - /** @type {import("node:url").URL} */ - let urlObject; try { // The `url` property of the `request` is contains only `pathname`, `search` and `hash` urlObject = memoizedParse(url); } catch { - return { errorCode, filename: foundFilename, extra }; + return; } + const { options } = context; + const paths = getPaths(context); + + /** @type {Extra} */ + const extra = {}; + for (const { publicPath, outputPath, assetsInfo } of paths) { /** @type {string | undefined} */ let filename; @@ -103,16 +115,12 @@ function getFilenameFromUrl(context, url) { ) { // Null byte(s) if (pathname.includes("\0")) { - errorCode = 400; - - return { errorCode, filename: foundFilename, extra }; + throw new FilenameError("Bad Request", 400); } // ".." is malicious if (UP_PATH_REGEXP.test(path.normalize(`./${pathname}`))) { - errorCode = 403; - - return { errorCode, filename: foundFilename, extra }; + throw new FilenameError("Forbidden", 403); } // Strip the `pathname` property from the `publicPath` option from the start of requested url @@ -170,7 +178,12 @@ function getFilenameFromUrl(context, url) { } } - return { filename: foundFilename, extra, errorCode }; + if (!foundFilename) { + return; + } + + return { filename: foundFilename, extra }; } module.exports = getFilenameFromUrl; +module.exports.FilenameError = FilenameError; diff --git a/types/index.d.ts b/types/index.d.ts index 6259d73ab..1ba81cd6c 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -99,16 +99,16 @@ export = wdm; * @template {IncomingMessage} [RequestInternal=IncomingMessage] * @template {ServerResponse} [ResponseInternal=ServerResponse] * @callback Middleware - * @param {RequestInternal} req - * @param {ResponseInternal} res - * @param {NextFunction} next + * @param {RequestInternal} req request + * @param {ResponseInternal} res response + * @param {NextFunction} next next function * @returns {Promise} */ /** @typedef {import("./utils/getFilenameFromUrl").Extra} Extra */ /** * @callback GetFilenameFromUrl - * @param {string} url - * @returns {{ filename?: string, extra: Extra, errorCode?: number }} + * @param {string} url request URL + * @returns {{ filename: string, extra: Extra } | undefined} a filename with additional information, or `undefined` if nothing is found */ /** * @callback WaitUntilValid @@ -459,11 +459,12 @@ type Middleware< next: NextFunction, ) => Promise; type Extra = import("./utils/getFilenameFromUrl").Extra; -type GetFilenameFromUrl = (url: string) => { - filename?: string; - extra: Extra; - errorCode?: number; -}; +type GetFilenameFromUrl = (url: string) => + | { + filename: string; + extra: Extra; + } + | undefined; type WaitUntilValid = (callback: Callback) => any; type Invalidate = (callback: Callback) => any; type Close = (callback: (err: Error | null | undefined) => void) => any; diff --git a/types/middleware.d.ts b/types/middleware.d.ts index cce5820ac..2be44b1de 100644 --- a/types/middleware.d.ts +++ b/types/middleware.d.ts @@ -25,6 +25,8 @@ declare namespace wrapper { IncomingMessage, ServerResponse, NormalizedHeaders, + FilenameError, + Extra, ReadStream, }; } @@ -50,4 +52,6 @@ type NextFunction = import("./index.js").NextFunction; type IncomingMessage = import("./index.js").IncomingMessage; type ServerResponse = import("./index.js").ServerResponse; type NormalizedHeaders = import("./index.js").NormalizedHeaders; +type FilenameError = import("./utils/getFilenameFromUrl.js").FilenameError; +type Extra = import("./utils/getFilenameFromUrl.js").Extra; type ReadStream = import("fs").ReadStream; diff --git a/types/utils/getFilenameFromUrl.d.ts b/types/utils/getFilenameFromUrl.d.ts index 13f2a3185..2385f8646 100644 --- a/types/utils/getFilenameFromUrl.d.ts +++ b/types/utils/getFilenameFromUrl.d.ts @@ -1,22 +1,10 @@ export = getFilenameFromUrl; -/** - * @typedef {object} Extra - * @property {import("fs").Stats=} stats stats - * @property {boolean=} immutable true when immutable, otherwise false - */ -/** - * decodeURIComponent. - * - * Allows V8 to only deoptimize this fn instead of all of send(). - * @param {string} input - * @returns {string} - */ /** * @template {IncomingMessage} Request * @template {ServerResponse} Response * @param {import("../index.js").FilledContext} context context * @param {string} url url - * @returns {{ filename?: string, extra: Extra, errorCode?: number }} result of get filename from url + * @returns {{ filename: string, extra: Extra } | undefined} result of get filename from url */ declare function getFilenameFromUrl< Request extends IncomingMessage, @@ -24,13 +12,34 @@ declare function getFilenameFromUrl< >( context: import("../index.js").FilledContext, url: string, -): { - filename?: string; - extra: Extra; - errorCode?: number; -}; +): + | { + filename: string; + extra: Extra; + } + | undefined; declare namespace getFilenameFromUrl { - export { IncomingMessage, ServerResponse, Extra }; + export { FilenameError, IncomingMessage, ServerResponse, Extra }; +} +/** + * @typedef {object} Extra + * @property {import("fs").Stats} stats stats + * @property {boolean=} immutable true when immutable, otherwise false + */ +/** + * decodeURIComponent. + * + * Allows V8 to only deoptimize this fn instead of all of send(). + * @param {string} input + * @returns {string} + */ +declare class FilenameError extends Error { + /** + * @param {string} message message + * @param {number=} code error code + */ + constructor(message: string, code?: number | undefined); + code: number | undefined; } type IncomingMessage = import("../index.js").IncomingMessage; type ServerResponse = import("../index.js").ServerResponse; @@ -38,7 +47,7 @@ type Extra = { /** * stats */ - stats?: import("fs").Stats | undefined; + stats: import("fs").Stats; /** * true when immutable, otherwise false */ From 3980ba18af209aaf723668e55cca8a5c6e5fdd4a Mon Sep 17 00:00:00 2001 From: alexander-akait Date: Tue, 27 Jan 2026 17:12:34 +0300 Subject: [PATCH 5/5] refactor: fix tests and fix docs --- README.md | 13 ++++++++++++- test/middleware.test.js | 36 +++++++++++++++--------------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 67e50ff9e..113fdc2e5 100644 --- a/README.md +++ b/README.md @@ -459,7 +459,18 @@ const app = new express(); app.use(instance); instance.waitUntilValid(() => { - const { filename } = instance.getFilenameFromUrl("/bundle.js"); + let resolver; + + try { + resolved = instance.getFilenameFromUrl("/bundle.js"); + } catch (err) { + console.log(`Error: ${err}`); + } + + if (!resolved) { + console.log("Not found"); + return; + } console.log(`Filename is ${filename}`); }); diff --git a/test/middleware.test.js b/test/middleware.test.js index ee2018258..5aaece56f 100644 --- a/test/middleware.test.js +++ b/test/middleware.test.js @@ -586,11 +586,10 @@ describe.each([ path.join(webpackConfig.output.path, "/svg.svg"), ); expect( - instance.getFilenameFromUrl("/unknown.unknown").filename, + instance.getFilenameFromUrl("/unknown.unknown"), ).toBeUndefined(); expect( - instance.getFilenameFromUrl("/unknown/unknown.unknown") - .filename, + instance.getFilenameFromUrl("/unknown/unknown.unknown"), ).toBeUndefined(); done(); @@ -622,7 +621,7 @@ describe.each([ path.join(webpackConfig.output.path, "/bundle.js"), ); - expect(instance.getFilenameFromUrl("/").filename).toBeUndefined(); + expect(instance.getFilenameFromUrl("/")).toBeUndefined(); expect(instance.getFilenameFromUrl("/index.html").filename).toBe( path.join(webpackConfig.output.path, "/index.html"), ); @@ -630,11 +629,10 @@ describe.each([ path.join(webpackConfig.output.path, "/svg.svg"), ); expect( - instance.getFilenameFromUrl("/unknown.unknown").filename, + instance.getFilenameFromUrl("/unknown.unknown"), ).toBeUndefined(); expect( - instance.getFilenameFromUrl("/unknown/unknown.unknown") - .filename, + instance.getFilenameFromUrl("/unknown/unknown.unknown"), ).toBeUndefined(); done(); @@ -680,13 +678,12 @@ describe.each([ path.join(webpackPublicPathConfig.output.path, "/svg.svg"), ); - expect(instance.getFilenameFromUrl("/").filename).toBeUndefined(); + expect(instance.getFilenameFromUrl("/")).toBeUndefined(); expect( - instance.getFilenameFromUrl("/unknown.unknown").filename, + instance.getFilenameFromUrl("/unknown.unknown"), ).toBeUndefined(); expect( - instance.getFilenameFromUrl("/unknown/unknown.unknown") - .filename, + instance.getFilenameFromUrl("/unknown/unknown.unknown"), ).toBeUndefined(); done(); @@ -728,13 +725,12 @@ describe.each([ instance.getFilenameFromUrl("/static-one/svg.svg").filename, ).toBe(path.join(webpackMultiConfig[0].output.path, "/svg.svg")); expect( - instance.getFilenameFromUrl("/static-one/unknown.unknown") - .filename, + instance.getFilenameFromUrl("/static-one/unknown.unknown"), ).toBeUndefined(); expect( instance.getFilenameFromUrl( "/static-one/unknown/unknown.unknown", - ).filename, + ), ).toBeUndefined(); expect( @@ -743,24 +739,22 @@ describe.each([ path.join(webpackMultiConfig[1].output.path, "/bundle.js"), ); expect( - instance.getFilenameFromUrl("/static-two/unknown.unknown") - .filename, + instance.getFilenameFromUrl("/static-two/unknown.unknown"), ).toBeUndefined(); expect( instance.getFilenameFromUrl( "/static-two/unknown/unknown.unknown", - ).filename, + ), ).toBeUndefined(); - expect(instance.getFilenameFromUrl("/").filename).toBeUndefined(); + expect(instance.getFilenameFromUrl("/")).toBeUndefined(); expect( - instance.getFilenameFromUrl("/static-one/unknown.unknown") - .filename, + instance.getFilenameFromUrl("/static-one/unknown.unknown"), ).toBeUndefined(); expect( instance.getFilenameFromUrl( "/static-one/unknown/unknown.unknown", - ).filename, + ), ).toBeUndefined(); done();