From 1258fdd3d9c175dbacf6bc3b36d5c3c545738f13 Mon Sep 17 00:00:00 2001 From: Alexander Akait <4567934+alexander-akait@users.noreply.github.com> Date: Tue, 19 Mar 2024 16:43:33 +0300 Subject: [PATCH] fix: cleaup stream and handle errors (#1769) --- .cspell.json | 3 +- package-lock.json | 15 ++- package.json | 2 + src/utils/compatibleAPI.js | 118 ++++++++++++++++++++- src/utils/escapeHtml.js | 58 ++++++++++ test/middleware.test.js | 194 ++++++++++++++++++++++++++++++++++ test/utils/escapeHtml.test.js | 11 ++ types/utils/escapeHtml.d.ts | 6 ++ 8 files changed, 401 insertions(+), 6 deletions(-) create mode 100644 src/utils/escapeHtml.js create mode 100644 test/utils/escapeHtml.test.js create mode 100644 types/utils/escapeHtml.d.ts diff --git a/.cspell.json b/.cspell.json index c1064bc80..490b85431 100644 --- a/.cspell.json +++ b/.cspell.json @@ -17,7 +17,8 @@ "myhtml", "configurated", "mycustom", - "commitlint" + "commitlint", + "nosniff" ], "ignorePaths": [ "CHANGELOG.md", diff --git a/package-lock.json b/package-lock.json index 430876678..4ec367f7c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "colorette": "^2.0.10", "memfs": "^4.6.0", "mime-types": "^2.1.31", + "on-finished": "^2.4.1", "range-parser": "^1.2.1", "schema-utils": "^4.0.0" }, @@ -25,6 +26,7 @@ "@types/express": "^4.17.13", "@types/mime-types": "^2.1.1", "@types/node": "^20.11.16", + "@types/on-finished": "^2.3.4", "@webpack-contrib/eslint-config-webpack": "^3.0.0", "babel-jest": "^29.3.1", "chokidar": "^3.5.1", @@ -4259,6 +4261,15 @@ "integrity": "sha512-37i+OaWTh9qeK4LSHPsyRC7NahnGotNuZvjLSgcPzblpHB3rrCJxAOgI5gCdKm7coonsaX1Of0ILiTcnZjbfxA==", "dev": true }, + "node_modules/@types/on-finished": { + "version": "2.3.4", + "resolved": "https://registry.npmjs.org/@types/on-finished/-/on-finished-2.3.4.tgz", + "integrity": "sha512-Ld4UQD3udYcKPaAWlI1EYXKhefkZcTlpqOLkQRmN3u5Ml/tUypMivUHbNH8LweP4H4FlhGGO+uBjJI1Y1dkE1g==", + "dev": true, + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/qs": { "version": "6.9.12", "resolved": "https://registry.npmjs.org/@types/qs/-/qs-6.9.12.tgz", @@ -8205,8 +8216,7 @@ "node_modules/ee-first": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/ee-first/-/ee-first-1.1.1.tgz", - "integrity": "sha512-WMwm9LhRUo+WUaRN+vRuETqG89IgZphVSNkdFgeb6sS/E4OrDIN7t48CAewSHXc6C8lefD8KKfr5vY61brQlow==", - "dev": true + "integrity": "sha512-WMwm9LhRUo+WUaRN+vRuETqG89IgZphVSNkdFgeb6sS/E4OrDIN7t48CAewSHXc6C8lefD8KKfr5vY61brQlow==" }, "node_modules/electron-to-chromium": { "version": "1.4.695", @@ -13847,7 +13857,6 @@ "version": "2.4.1", "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.4.1.tgz", "integrity": "sha512-oVlzkg3ENAhCk2zdv7IJwd/QUD4z2RxRwpkcGY8psCVcCYZNq4wYnVWALHM+brtuJjePWiYF/ClmuDr8Ch5+kg==", - "dev": true, "dependencies": { "ee-first": "1.1.1" }, diff --git a/package.json b/package.json index 5b8aed90b..333cf31b6 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "colorette": "^2.0.10", "memfs": "^4.6.0", "mime-types": "^2.1.31", + "on-finished": "^2.4.1", "range-parser": "^1.2.1", "schema-utils": "^4.0.0" }, @@ -69,6 +70,7 @@ "@types/express": "^4.17.13", "@types/mime-types": "^2.1.1", "@types/node": "^20.11.16", + "@types/on-finished": "^2.3.4", "@webpack-contrib/eslint-config-webpack": "^3.0.0", "babel-jest": "^29.3.1", "chokidar": "^3.5.1", diff --git a/src/utils/compatibleAPI.js b/src/utils/compatibleAPI.js index bd094daa3..83301478f 100644 --- a/src/utils/compatibleAPI.js +++ b/src/utils/compatibleAPI.js @@ -1,3 +1,7 @@ +const onFinishedStream = require("on-finished"); + +const escapeHtml = require("./escapeHtml"); + /** @typedef {import("../index.js").IncomingMessage} IncomingMessage */ /** @typedef {import("../index.js").ServerResponse} ServerResponse */ @@ -88,6 +92,18 @@ function setHeaderForResponse(res, name, value) { res.setHeader(name, value); } +/** + * @template {ServerResponse} Response + * @param {Response} res + */ +function clearHeadersForResponse(res) { + const headers = getHeaderNames(res); + + for (let i = 0; i < headers.length; i++) { + res.removeHeader(headers[i]); + } +} + /** * @template {ServerResponse} Response * @param {Response} res @@ -108,6 +124,76 @@ function setStatusCode(res, code) { res.statusCode = code; } +/** + * @param {import("fs").ReadStream} stream stream + * @param {boolean} suppress do need suppress? + * @returns {void} + */ +function destroyStream(stream, suppress) { + if (typeof stream.destroy === "function") { + stream.destroy(); + } + + if (typeof stream.close === "function") { + // Node.js core bug workaround + stream.on( + "open", + /** + * @this {import("fs").ReadStream} + */ + function onOpenClose() { + // @ts-ignore + if (typeof this.fd === "number") { + // actually close down the fd + this.close(); + } + }, + ); + } + + if (typeof stream.addListener === "function" && suppress) { + stream.removeAllListeners("error"); + stream.addListener("error", () => {}); + } +} + +/** @type {Record} */ +const statuses = { + 404: "Not Found", + 500: "Internal Server Error", +}; + +/** + * @template {ServerResponse} Response + * @param {Response} res response + * @param {number} status status + * @returns {void} + */ +function sendError(res, status) { + const msg = statuses[status] || String(status); + const doc = ` + + + +Error + + +
${escapeHtml(msg)}
+ +`; + + // Clear existing headers + clearHeadersForResponse(res); + // 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); +} + /** * @template {IncomingMessage} Request * @template {ServerResponse} Response @@ -125,13 +211,42 @@ function send(req, res, bufferOtStream, byteLength) { if (req.method === "HEAD") { res.end(); - return; } /** @type {import("fs").ReadStream} */ (bufferOtStream).pipe(res); + // Cleanup + const cleanup = () => { + destroyStream( + /** @type {import("fs").ReadStream} */ (bufferOtStream), + true, + ); + }; + + // Response finished, cleanup + onFinishedStream(res, cleanup); + + // error handling + /** @type {import("fs").ReadStream} */ + (bufferOtStream).on("error", (error) => { + // clean up stream early + cleanup(); + + // Handle Error + switch (/** @type {NodeJS.ErrnoException} */ (error).code) { + case "ENAMETOOLONG": + case "ENOENT": + case "ENOTDIR": + sendError(res, 404); + break; + default: + sendError(res, 500); + break; + } + }); + return; } @@ -141,7 +256,6 @@ function send(req, res, bufferOtStream, byteLength) { ) { /** @type {Response & ExpectedResponse} */ (res).send(bufferOtStream); - return; } diff --git a/src/utils/escapeHtml.js b/src/utils/escapeHtml.js new file mode 100644 index 000000000..4a131ef30 --- /dev/null +++ b/src/utils/escapeHtml.js @@ -0,0 +1,58 @@ +const matchHtmlRegExp = /["'&<>]/; + +/** + * @param {string} string raw HTML + * @returns {string} escaped HTML + */ +function escapeHtml(string) { + const str = `${string}`; + const match = matchHtmlRegExp.exec(str); + + if (!match) { + return str; + } + + let escape; + let html = ""; + let index = 0; + let lastIndex = 0; + + for ({ index } = match; index < str.length; index++) { + switch (str.charCodeAt(index)) { + // " + case 34: + escape = """; + break; + // & + case 38: + escape = "&"; + break; + // ' + case 39: + escape = "'"; + break; + // < + case 60: + escape = "<"; + break; + // > + case 62: + escape = ">"; + break; + default: + // eslint-disable-next-line no-continue + continue; + } + + if (lastIndex !== index) { + html += str.substring(lastIndex, index); + } + + lastIndex = index + 1; + html += escape; + } + + return lastIndex !== index ? html + str.substring(lastIndex, index) : html; +} + +module.exports = escapeHtml; diff --git a/test/middleware.test.js b/test/middleware.test.js index 160114aac..97adf5f24 100644 --- a/test/middleware.test.js +++ b/test/middleware.test.js @@ -1959,6 +1959,200 @@ describe.each([ expect(response.statusCode).toEqual(200); }); }); + + 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", + ); + + 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 = + function createReadStream(...args) { + const brokenStream = new this.ReadStream(...args); + + // eslint-disable-next-line no-underscore-dangle + brokenStream._read = function _read() { + this.emit("error", new Error("test")); + this.end(); + this.destroy(); + }; + + return brokenStream; + }; + + 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 "500" code for the "GET" request to the "image.svg" file', async () => { + const response = await req.get("/image.svg").set("Range", "bytes=0-"); + + expect(response.statusCode).toEqual(500); + expect(response.headers["content-type"]).toEqual( + "text/html; charset=UTF-8", + ); + expect(response.text).toEqual( + "\n" + + '\n' + + "\n" + + '\n' + + "Error\n" + + "\n" + + "\n" + + "
Internal Server Error
\n" + + "\n" + + "", + ); + }); + }); + + 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", + ); + + 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 = + function createReadStream(...args) { + const brokenStream = new this.ReadStream(...args); + + // eslint-disable-next-line no-underscore-dangle + brokenStream._read = function _read() { + const error = new Error("test"); + + error.code = "ENAMETOOLONG"; + + this.emit("error", error); + this.end(); + this.destroy(); + }; + + return brokenStream; + }; + + 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 "404" code for the "GET" request to the "image.svg" file', async () => { + const response = await req.get("/image.svg").set("Range", "bytes=0-"); + + expect(response.statusCode).toEqual(404); + expect(response.headers["content-type"]).toEqual( + "text/html; charset=UTF-8", + ); + expect(response.text).toEqual( + "\n" + + '\n' + + "\n" + + '\n' + + "Error\n" + + "\n" + + "\n" + + "
Not Found
\n" + + "\n" + + "", + ); + }); + }); }); describe("mimeTypes option", () => { diff --git a/test/utils/escapeHtml.test.js b/test/utils/escapeHtml.test.js new file mode 100644 index 000000000..f89d5799d --- /dev/null +++ b/test/utils/escapeHtml.test.js @@ -0,0 +1,11 @@ +import escapeHtml from "../../src/utils/escapeHtml"; + +describe("escapeHtml", () => { + it("should work", () => { + expect(escapeHtml("")).toBe(""); + expect(escapeHtml("test")).toBe("test"); + expect(escapeHtml("\"&'<>test")).toBe(""&'<>test"); + expect(escapeHtml("\"&'test<>")).toBe(""&'test<>"); + expect(escapeHtml("test\"&'<>")).toBe("test"&'<>"); + }); +}); diff --git a/types/utils/escapeHtml.d.ts b/types/utils/escapeHtml.d.ts new file mode 100644 index 000000000..f7982bda1 --- /dev/null +++ b/types/utils/escapeHtml.d.ts @@ -0,0 +1,6 @@ +export = escapeHtml; +/** + * @param {string} string raw HTML + * @returns {string} escaped HTML + */ +declare function escapeHtml(string: string): string;