From ab533de933c6684218172b86992f35c3ca6c58a4 Mon Sep 17 00:00:00 2001 From: Alexander Akait <4567934+alexander-akait@users.noreply.github.com> Date: Tue, 19 Mar 2024 19:41:46 +0300 Subject: [PATCH] feat: prefer to use `fs.createReadStream` over `fs.readFileSync` to read files --- src/index.js | 17 ++-- src/middleware.js | 138 +++++--------------------- src/utils/compatibleAPI.js | 120 ++++++++++++++++++---- src/utils/getFilenameFromUrl.js | 21 ++-- test/middleware.test.js | 149 ++++++++++++++++++++-------- types/index.d.ts | 9 +- types/utils/compatibleAPI.d.ts | 62 +++++++++++- types/utils/getFilenameFromUrl.d.ts | 11 +- 8 files changed, 331 insertions(+), 196 deletions(-) diff --git a/src/index.js b/src/index.js index a6663a1de..a4c41b2dd 100644 --- a/src/index.js +++ b/src/index.js @@ -120,9 +120,12 @@ const noop = () => {}; * @return {Promise} */ +/** @typedef {import("./utils/getFilenameFromUrl").Extra} Extra */ + /** * @callback GetFilenameFromUrl * @param {string} url + * @param {Extra=} extra * @returns {string | undefined} */ @@ -263,18 +266,14 @@ function wdm(compiler, options = {}) { } } - const instance = /** @type {API} */ ( - middleware(context) - ); + const instance = + /** @type {API} */ + (middleware(context)); // API /** @type {API} */ - (instance).getFilenameFromUrl = - /** - * @param {string} url - * @returns {string|undefined} - */ - (url) => getFilenameFromUrl(context, url); + (instance).getFilenameFromUrl = (url, extra) => + getFilenameFromUrl(context, url, extra); /** @type {API} */ (instance).waitUntilValid = (callback = noop) => { diff --git a/src/middleware.js b/src/middleware.js index bca2bce91..a7de58d4e 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -4,12 +4,12 @@ const mime = require("mime-types"); const getFilenameFromUrl = require("./utils/getFilenameFromUrl"); const { - getHeaderNames, getHeaderFromRequest, getHeaderFromResponse, setHeaderForResponse, setStatusCode, send, + sendError, } = require("./utils/compatibleAPI"); const ready = require("./utils/ready"); @@ -27,28 +27,6 @@ function getValueContentRangeHeader(type, size, range) { return `${type} ${range ? `${range.start}-${range.end}` : "*"}/${size}`; } -/** - * @param {string | number} title - * @param {string} body - * @returns {string} - */ -function createHtmlDocument(title, body) { - return ( - `${ - "\n" + - '\n' + - "\n" + - '\n' + - "" - }${title}\n` + - `\n` + - `\n` + - `
${body}
\n` + - `\n` + - `\n` - ); -} - const BYTES_RANGE_REGEXP = /^ *bytes/i; /** @@ -94,9 +72,12 @@ function wrapper(context) { } async function processRequest() { + /** @type {import("./utils/getFilenameFromUrl").Extra} */ + const extra = {}; const filename = getFilenameFromUrl( context, /** @type {string} */ (req.url), + extra, ); if (!filename) { @@ -161,68 +142,29 @@ function wrapper(context) { const rangeHeader = getHeaderFromRequest(req, "range"); - let start; - let end; + let len = /** @type {import("fs").Stats} */ (extra.stats).size; + let offset = 0; if (rangeHeader && BYTES_RANGE_REGEXP.test(rangeHeader)) { - const size = await new Promise((resolve) => { - /** @type {import("fs").lstat} */ - (context.outputFileSystem.lstat)(filename, (error, stats) => { - if (error) { - context.logger.error(error); - - return; - } - - resolve(stats.size); - }); - }); - // eslint-disable-next-line global-require - const parsedRanges = require("range-parser")(size, rangeHeader, { + const parsedRanges = require("range-parser")(len, rangeHeader, { combine: true, }); if (parsedRanges === -1) { - const message = "Unsatisfiable range for 'Range' header."; - - context.logger.error(message); - - const existingHeaders = getHeaderNames(res); + context.logger.error("Unsatisfiable range for 'Range' header."); - for (let i = 0; i < existingHeaders.length; i++) { - res.removeHeader(existingHeaders[i]); - } - - setStatusCode(res, 416); setHeaderForResponse( res, "Content-Range", - getValueContentRangeHeader("bytes", size), + getValueContentRangeHeader("bytes", len), ); - setHeaderForResponse(res, "Content-Type", "text/html; charset=utf-8"); - /** @type {string | Buffer | import("fs").ReadStream} */ - let document = createHtmlDocument(416, `Error: ${message}`); - let byteLength = Buffer.byteLength(document); - - setHeaderForResponse( - res, - "Content-Length", - Buffer.byteLength(document), - ); - - if (context.options.modifyResponseData) { - ({ data: document, byteLength } = - context.options.modifyResponseData( - req, - res, - document, - byteLength, - )); - } - - send(req, res, document, byteLength); + sendError(req, res, 416, { + headers: { + "Content-Range": res.getHeader("Content-Range"), + }, + }); return; } else if (parsedRanges === -2) { @@ -243,57 +185,23 @@ function wrapper(context) { "Content-Range", getValueContentRangeHeader( "bytes", - size, + len, /** @type {import("range-parser").Ranges} */ (parsedRanges)[0], ), ); - [{ start, end }] = parsedRanges; + offset += parsedRanges[0].start; + len = parsedRanges[0].end - parsedRanges[0].start + 1; } } - const isFsSupportsStream = - typeof context.outputFileSystem.createReadStream === "function"; - - let bufferOrStream; - let byteLength; - - try { - if ( - typeof start !== "undefined" && - typeof end !== "undefined" && - isFsSupportsStream - ) { - bufferOrStream = - /** @type {import("fs").createReadStream} */ - (context.outputFileSystem.createReadStream)(filename, { - start, - end, - }); - byteLength = end - start + 1; - } else { - bufferOrStream = /** @type {import("fs").readFileSync} */ ( - context.outputFileSystem.readFileSync - )(filename); - ({ byteLength } = bufferOrStream); - } - } catch (_ignoreError) { - await goNext(); + const start = offset; + const end = Math.max(offset, offset + len - 1); - return; - } - - if (context.options.modifyResponseData) { - ({ data: bufferOrStream, byteLength } = - context.options.modifyResponseData( - req, - res, - bufferOrStream, - byteLength, - )); - } - - send(req, res, bufferOrStream, byteLength); + send(req, res, filename, start, end, goNext, { + modifyResponseData: context.options.modifyResponseData, + outputFileSystem: context.outputFileSystem, + }); } }; } diff --git a/src/utils/compatibleAPI.js b/src/utils/compatibleAPI.js index 83301478f..e8e3f5b5f 100644 --- a/src/utils/compatibleAPI.js +++ b/src/utils/compatibleAPI.js @@ -92,6 +92,24 @@ function setHeaderForResponse(res, name, value) { res.setHeader(name, value); } +/** + * @template {ServerResponse} Response + * @param {Response} res + * @param {Record} headers + */ +function setHeadersForResponse(res, headers) { + const keys = Object.keys(headers); + + for (let i = 0; i < keys.length; i++) { + const key = keys[i]; + const value = headers[key]; + + if (typeof value !== "undefined") { + setHeaderForResponse(res, key, value); + } + } +} + /** * @template {ServerResponse} Response * @param {Response} res @@ -160,51 +178,117 @@ function destroyStream(stream, suppress) { /** @type {Record} */ const statuses = { 404: "Not Found", + 416: "Range Not Satisfiable", 500: "Internal Server Error", }; /** + * @template {IncomingMessage} Request * @template {ServerResponse} Response + * @param {Request} req response * @param {Response} res response * @param {number} status status + * @param {Partial>=} options options * @returns {void} */ -function sendError(res, status) { - const msg = statuses[status] || String(status); - const doc = ` +function sendError(req, res, status, options) { + const content = statuses[status] || String(status); + let document = ` Error -
${escapeHtml(msg)}
+
${escapeHtml(content)}
`; // Clear existing headers clearHeadersForResponse(res); + + if (options && options.headers) { + setHeadersForResponse(res, options.headers); + } + // Send basic response setStatusCode(res, status); setHeaderForResponse(res, "Content-Type", "text/html; charset=UTF-8"); - setHeaderForResponse(res, "Content-Length", Buffer.byteLength(doc)); setHeaderForResponse(res, "Content-Security-Policy", "default-src 'none'"); setHeaderForResponse(res, "X-Content-Type-Options", "nosniff"); - res.end(doc); + let byteLength = Buffer.byteLength(document); + + if (options && options.modifyResponseData) { + ({ data: document, byteLength } = + /** @type {{data: string, byteLength: number }} */ + (options.modifyResponseData(req, res, document, byteLength))); + } + + setHeaderForResponse(res, "Content-Length", byteLength); + + res.end(document); } +/** + * @template {IncomingMessage} Request + * @template {ServerResponse} Response + * @typedef {Object} SendOptions send error options + * @property {Record=} headers headers + * @property {import("../index").ModifyResponseData=} modifyResponseData modify response data callback + * @property {import("../index").OutputFileSystem} outputFileSystem modify response data callback + */ + /** * @template {IncomingMessage} Request * @template {ServerResponse} Response * @param {Request} req * @param {Response} res - * @param {string | Buffer | import("fs").ReadStream} bufferOtStream - * @param {number} byteLength + * @param {string} filename + * @param {number} start + * @param {number} end + * @param {() => Promise} goNext + * @param {SendOptions} options */ -function send(req, res, bufferOtStream, byteLength) { +async function send(req, res, filename, start, end, goNext, options) { + const isFsSupportsStream = + typeof options.outputFileSystem.createReadStream === "function"; + + let bufferOrStream; + let byteLength; + + try { + if (isFsSupportsStream) { + bufferOrStream = + /** @type {import("fs").createReadStream} */ + (options.outputFileSystem.createReadStream)(filename, { + start, + end, + }); + byteLength = end - start + 1; + } else { + bufferOrStream = /** @type {import("fs").readFileSync} */ ( + options.outputFileSystem.readFileSync + )(filename); + ({ byteLength } = bufferOrStream); + } + } catch (_ignoreError) { + await goNext(); + + return; + } + + if (options.modifyResponseData) { + ({ data: bufferOrStream, byteLength } = options.modifyResponseData( + req, + res, + bufferOrStream, + byteLength, + )); + } + if ( - typeof (/** @type {import("fs").ReadStream} */ (bufferOtStream).pipe) === + typeof (/** @type {import("fs").ReadStream} */ (bufferOrStream).pipe) === "function" ) { setHeaderForResponse(res, "Content-Length", byteLength); @@ -215,12 +299,12 @@ function send(req, res, bufferOtStream, byteLength) { } /** @type {import("fs").ReadStream} */ - (bufferOtStream).pipe(res); + (bufferOrStream).pipe(res); // Cleanup const cleanup = () => { destroyStream( - /** @type {import("fs").ReadStream} */ (bufferOtStream), + /** @type {import("fs").ReadStream} */ (bufferOrStream), true, ); }; @@ -230,7 +314,7 @@ function send(req, res, bufferOtStream, byteLength) { // error handling /** @type {import("fs").ReadStream} */ - (bufferOtStream).on("error", (error) => { + (bufferOrStream).on("error", (error) => { // clean up stream early cleanup(); @@ -239,10 +323,10 @@ function send(req, res, bufferOtStream, byteLength) { case "ENAMETOOLONG": case "ENOENT": case "ENOTDIR": - sendError(res, 404); + sendError(req, res, 404, options); break; default: - sendError(res, 500); + sendError(req, res, 500, options); break; } }); @@ -250,12 +334,13 @@ function send(req, res, bufferOtStream, byteLength) { return; } + // Express API if ( typeof (/** @type {Response & ExpectedResponse} */ (res).send) === "function" ) { /** @type {Response & ExpectedResponse} */ - (res).send(bufferOtStream); + (res).send(bufferOrStream); return; } @@ -265,7 +350,7 @@ function send(req, res, bufferOtStream, byteLength) { if (req.method === "HEAD") { res.end(); } else { - res.end(bufferOtStream); + res.end(bufferOrStream); } } @@ -276,4 +361,5 @@ module.exports = { setHeaderForResponse, setStatusCode, send, + sendError, }; diff --git a/src/utils/getFilenameFromUrl.js b/src/utils/getFilenameFromUrl.js index 938564c13..82fab7447 100644 --- a/src/utils/getFilenameFromUrl.js +++ b/src/utils/getFilenameFromUrl.js @@ -43,14 +43,20 @@ const mem = (fn, { cache = new Map() } = {}) => { }; const memoizedParse = mem(parse); +/** + * @typedef {Object} Extra + * @property {import("fs").Stats=} stats + */ + /** * @template {IncomingMessage} Request * @template {ServerResponse} Response * @param {import("../index.js").Context} context * @param {string} url + * @param {Extra=} extra * @returns {string | undefined} */ -function getFilenameFromUrl(context, url) { +function getFilenameFromUrl(context, url, extra = {}) { const { options } = context; const paths = getPaths(context); @@ -95,10 +101,9 @@ function getFilenameFromUrl(context, url) { filename = path.join(outputPath, querystring.unescape(pathname)); } - let fsStats; - try { - fsStats = + // eslint-disable-next-line no-param-reassign + extra.stats = /** @type {import("fs").statSync} */ (context.outputFileSystem.statSync)(filename); } catch (_ignoreError) { @@ -106,12 +111,12 @@ function getFilenameFromUrl(context, url) { continue; } - if (fsStats.isFile()) { + if (extra.stats.isFile()) { foundFilename = filename; break; } else if ( - fsStats.isDirectory() && + extra.stats.isDirectory() && (typeof options.index === "undefined" || options.index) ) { const indexValue = @@ -123,7 +128,7 @@ function getFilenameFromUrl(context, url) { filename = path.join(filename, indexValue); try { - fsStats = + extra.stats = /** @type {import("fs").statSync} */ (context.outputFileSystem.statSync)(filename); } catch (__ignoreError) { @@ -131,7 +136,7 @@ function getFilenameFromUrl(context, url) { continue; } - if (fsStats.isFile()) { + if (extra.stats.isFile()) { foundFilename = filename; break; diff --git a/test/middleware.test.js b/test/middleware.test.js index 97adf5f24..755b92fd7 100644 --- a/test/middleware.test.js +++ b/test/middleware.test.js @@ -263,7 +263,19 @@ describe.each([ `bytes */${codeLength}`, ); expect(response.headers["content-type"]).toEqual( - "text/html; charset=utf-8", + "text/html; charset=UTF-8", + ); + expect(response.text).toEqual( + ` + + + +Error + + +
Range Not Satisfiable
+ +`, ); }); @@ -1962,12 +1974,10 @@ describe.each([ describe("should handle custom fs errors and response 500 code", () => { let compiler; - let codeContent; - let codeLength; const outputPath = path.resolve( __dirname, - "./outputs/basic-test-errors", + "./outputs/basic-test-errors-500", ); beforeAll((done) => { @@ -1985,10 +1995,7 @@ describe.each([ app.use(instance); listen = listenShorthand(() => { - compiler.hooks.afterCompile.tap("wdm-test", (params) => { - codeContent = params.assets["bundle.js"].source(); - codeLength = Buffer.byteLength(codeContent); - + compiler.hooks.afterCompile.tap("wdm-test", () => { done(); }); }); @@ -2020,19 +2027,6 @@ describe.each([ afterAll(close); - it('should return the "200" code for the "GET" request to the bundle file', async () => { - const response = await req.get("/bundle.js"); - - expect(response.statusCode).toEqual(200); - expect(response.headers["content-length"]).toEqual( - String(codeLength), - ); - expect(response.headers["content-type"]).toEqual( - "application/javascript; charset=utf-8", - ); - expect(response.text).toEqual(codeContent); - }); - it('should return the "500" code for the "GET" request to the "image.svg" file', async () => { const response = await req.get("/image.svg").set("Range", "bytes=0-"); @@ -2057,12 +2051,10 @@ describe.each([ describe("should handle known fs errors and response 404 code", () => { let compiler; - let codeContent; - let codeLength; const outputPath = path.resolve( __dirname, - "./outputs/basic-test-errors", + "./outputs/basic-test-errors-404", ); beforeAll((done) => { @@ -2080,10 +2072,7 @@ describe.each([ app.use(instance); listen = listenShorthand(() => { - compiler.hooks.afterCompile.tap("wdm-test", (params) => { - codeContent = params.assets["bundle.js"].source(); - codeLength = Buffer.byteLength(codeContent); - + compiler.hooks.afterCompile.tap("wdm-test", () => { done(); }); }); @@ -2119,19 +2108,6 @@ describe.each([ afterAll(close); - it('should return the "200" code for the "GET" request to the bundle file', async () => { - const response = await req.get("/bundle.js"); - - expect(response.statusCode).toEqual(200); - expect(response.headers["content-length"]).toEqual( - String(codeLength), - ); - expect(response.headers["content-type"]).toEqual( - "application/javascript; charset=utf-8", - ); - expect(response.text).toEqual(codeContent); - }); - it('should return the "404" code for the "GET" request to the "image.svg" file', async () => { const response = await req.get("/image.svg").set("Range", "bytes=0-"); @@ -2153,6 +2129,97 @@ describe.each([ ); }); }); + + describe("should work without `fs.createReadStream`", () => { + let compiler; + let codeContent; + let codeLength; + + const outputPath = path.resolve( + __dirname, + "./outputs/basic-test-no-createReadStream", + ); + + beforeAll((done) => { + compiler = getCompiler({ + ...webpackConfig, + output: { + filename: "bundle.js", + path: outputPath, + }, + }); + + instance = middleware(compiler); + + app = framework(); + app.use(instance); + + listen = listenShorthand(() => { + compiler.hooks.afterCompile.tap("wdm-test", (params) => { + codeContent = params.assets["bundle.js"].source(); + codeLength = Buffer.byteLength(codeContent); + + done(); + }); + }); + + instance.context.outputFileSystem.mkdirSync(outputPath, { + recursive: true, + }); + instance.context.outputFileSystem.writeFileSync( + path.resolve(outputPath, "image.svg"), + "svg image", + ); + + instance.context.outputFileSystem.createReadStream = null; + + req = request(app); + }); + + afterAll(close); + + it('should return the "200" code for the "GET" request to the bundle file', async () => { + const response = await req.get("/bundle.js"); + + expect(response.statusCode).toEqual(200); + expect(response.headers["content-length"]).toEqual( + String(codeLength), + ); + expect(response.headers["content-type"]).toEqual( + "application/javascript; charset=utf-8", + ); + expect(response.text).toEqual(codeContent); + }); + + it('should return the "200" code for the "GET" request to the "image.svg" file', async () => { + const fileData = instance.context.outputFileSystem.readFileSync( + path.resolve(outputPath, "image.svg"), + ); + + const response = await req.get("/image.svg"); + + expect(response.statusCode).toEqual(200); + expect(response.headers["content-length"]).toEqual( + fileData.byteLength.toString(), + ); + expect(response.headers["content-type"]).toEqual("image/svg+xml"); + }); + + it('should return the "200" code for the "HEAD" request to the "image.svg" file', async () => { + const fileData = instance.context.outputFileSystem.readFileSync( + path.resolve(outputPath, "image.svg"), + ); + + const response = await req.head("/image.svg"); + + expect(response.statusCode).toEqual(200); + expect(response.headers["content-length"]).toEqual( + fileData.byteLength.toString(), + ); + expect(response.headers["content-type"]).toEqual("image/svg+xml"); + expect(response.body).toEqual({}); + }); + }); }); describe("mimeTypes option", () => { diff --git a/types/index.d.ts b/types/index.d.ts index 63bfdf375..bbcf41e98 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -94,9 +94,11 @@ export = wdm; * @param {NextFunction} next * @return {Promise} */ +/** @typedef {import("./utils/getFilenameFromUrl").Extra} Extra */ /** * @callback GetFilenameFromUrl * @param {string} url + * @param {Extra=} extra * @returns {string | undefined} */ /** @@ -165,6 +167,7 @@ declare namespace wdm { Headers, Options, Middleware, + Extra, GetFilenameFromUrl, WaitUntilValid, Invalidate, @@ -285,7 +288,11 @@ type Middleware< res: ResponseInternal, next: NextFunction, ) => Promise; -type GetFilenameFromUrl = (url: string) => string | undefined; +type Extra = import("./utils/getFilenameFromUrl").Extra; +type GetFilenameFromUrl = ( + url: string, + extra?: Extra | undefined, +) => string | undefined; type WaitUntilValid = (callback: Callback) => any; type Invalidate = (callback: Callback) => any; type Close = (callback: (err: Error | null | undefined) => void) => any; diff --git a/types/utils/compatibleAPI.d.ts b/types/utils/compatibleAPI.d.ts index 1cb744f54..2e3e404c1 100644 --- a/types/utils/compatibleAPI.d.ts +++ b/types/utils/compatibleAPI.d.ts @@ -10,6 +10,28 @@ export type ExpectedResponse = { status: (status: number) => void; send: (data: any) => void; }; +/** + * send error options + */ +export type SendOptions< + Request extends import("http").IncomingMessage, + Response extends import("../index.js").ServerResponse, +> = { + /** + * headers + */ + headers?: Record | undefined; + /** + * modify response data callback + */ + modifyResponseData?: + | import("../index").ModifyResponseData + | undefined; + /** + * modify response data callback + */ + outputFileSystem: import("../index").OutputFileSystem; +}; /** @typedef {import("../index.js").IncomingMessage} IncomingMessage */ /** @typedef {import("../index.js").ServerResponse} ServerResponse */ /** @@ -67,13 +89,24 @@ export function setHeaderForResponse< export function setStatusCode< Response extends import("../index.js").ServerResponse, >(res: Response, code: number): void; +/** + * @template {IncomingMessage} Request + * @template {ServerResponse} Response + * @typedef {Object} SendOptions send error options + * @property {Record=} headers headers + * @property {import("../index").ModifyResponseData=} modifyResponseData modify response data callback + * @property {import("../index").OutputFileSystem} outputFileSystem modify response data callback + */ /** * @template {IncomingMessage} Request * @template {ServerResponse} Response * @param {Request} req * @param {Response} res - * @param {string | Buffer | import("fs").ReadStream} bufferOtStream - * @param {number} byteLength + * @param {string} filename + * @param {number} start + * @param {number} end + * @param {() => Promise} goNext + * @param {SendOptions} options */ export function send< Request extends import("http").IncomingMessage, @@ -81,6 +114,27 @@ export function send< >( req: Request, res: Response, - bufferOtStream: string | Buffer | import("fs").ReadStream, - byteLength: number, + filename: string, + start: number, + end: number, + goNext: () => Promise, + options: SendOptions, +): Promise; +/** + * @template {IncomingMessage} Request + * @template {ServerResponse} Response + * @param {Request} req response + * @param {Response} res response + * @param {number} status status + * @param {Partial>=} options options + * @returns {void} + */ +export function sendError< + Request extends import("http").IncomingMessage, + Response extends import("../index.js").ServerResponse, +>( + req: Request, + res: Response, + status: number, + options?: Partial> | undefined, ): void; diff --git a/types/utils/getFilenameFromUrl.d.ts b/types/utils/getFilenameFromUrl.d.ts index f2116dcd6..7dfc8035d 100644 --- a/types/utils/getFilenameFromUrl.d.ts +++ b/types/utils/getFilenameFromUrl.d.ts @@ -1,10 +1,15 @@ /// export = getFilenameFromUrl; +/** + * @typedef {Object} Extra + * @property {import("fs").Stats=} stats + */ /** * @template {IncomingMessage} Request * @template {ServerResponse} Response * @param {import("../index.js").Context} context * @param {string} url + * @param {Extra=} extra * @returns {string | undefined} */ declare function getFilenameFromUrl< @@ -13,9 +18,13 @@ declare function getFilenameFromUrl< >( context: import("../index.js").Context, url: string, + extra?: Extra | undefined, ): string | undefined; declare namespace getFilenameFromUrl { - export { IncomingMessage, ServerResponse }; + export { Extra, IncomingMessage, ServerResponse }; } +type Extra = { + stats?: import("fs").Stats | undefined; +}; type IncomingMessage = import("../index.js").IncomingMessage; type ServerResponse = import("../index.js").ServerResponse;