From c20f1d98dff9b51931fae44a44fbc53387768673 Mon Sep 17 00:00:00 2001 From: Alexander Akait <4567934+alexander-akait@users.noreply.github.com> Date: Wed, 21 Aug 2024 18:25:45 +0300 Subject: [PATCH] fix: no crash when headers are already sent (#1929) --- src/index.js | 12 ++++- src/middleware.js | 88 +++++++++++++++++-------------- src/utils/compatibleAPI.js | 16 ++++++ src/utils/getFilenameFromUrl.js | 1 + test/middleware.test.js | 92 +++++++++++++++++++++++++++++++++ types/index.d.ts | 3 +- types/utils/compatibleAPI.d.ts | 10 ++++ 7 files changed, 180 insertions(+), 42 deletions(-) diff --git a/src/index.js b/src/index.js index 6f9e8ad10..fb9007ac0 100644 --- a/src/index.js +++ b/src/index.js @@ -305,7 +305,7 @@ function wdm(compiler, options = {}) { /** * @template S * @template O - * @typedef {HapiPluginBase & { pkg: { name: string } }} HapiPlugin + * @typedef {HapiPluginBase & { pkg: { name: string }, multiple: boolean }} HapiPlugin */ /** @@ -322,6 +322,8 @@ function hapiWrapper() { pkg: { name: "webpack-dev-middleware", }, + // Allow to have multiple middleware + multiple: true, register(server, options) { const { compiler, ...rest } = options; @@ -332,7 +334,11 @@ function hapiWrapper() { const devMiddleware = wdm(compiler, rest); // @ts-ignore - server.decorate("server", "webpackDevMiddleware", devMiddleware); + if (!server.decorations.server.includes("webpackDevMiddleware")) { + // @ts-ignore + server.decorate("server", "webpackDevMiddleware", devMiddleware); + } + // @ts-ignore server.ext("onRequest", (request, h) => new Promise((resolve, reject) => { @@ -568,6 +574,8 @@ function honoWrapper(compiler, options) { res.getReadyReadableStreamState = () => "readable"; + res.getHeadersSent = () => c.env.outgoing.headersSent; + let body; try { diff --git a/src/middleware.js b/src/middleware.js index 2b4bdcb60..ae6ea0e9c 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -15,6 +15,7 @@ const { setResponseHeader, removeResponseHeader, getResponseHeaders, + getHeadersSent, send, finish, pipe, @@ -494,56 +495,44 @@ function wrapper(context) { return; } + if (getHeadersSent(res)) { + await goNext(); + return; + } + const { size } = /** @type {import("fs").Stats} */ (extra.stats); let len = size; let offset = 0; // Send logic - let { headers } = context.options; + if (context.options.headers) { + let { headers } = context.options; - if (typeof headers === "function") { - headers = /** @type {NormalizedHeaders} */ (headers(req, res, context)); - } - - /** - * @type {{key: string, value: string | number}[]} - */ - const allHeaders = []; - - if (typeof headers !== "undefined") { - if (!Array.isArray(headers)) { - // eslint-disable-next-line guard-for-in - for (const name in headers) { - allHeaders.push({ key: name, value: headers[name] }); - } - - headers = allHeaders; + if (typeof headers === "function") { + headers = /** @type {NormalizedHeaders} */ ( + headers(req, res, context) + ); } - for (const { key, value } of headers) { - setResponseHeader(res, key, value); - } - } + /** + * @type {{key: string, value: string | number}[]} + */ + const allHeaders = []; - if ( - !getResponseHeader(res, "Content-Type") || - getStatusCode(res) === 404 - ) { - removeResponseHeader(res, "Content-Type"); - // content-type name(like application/javascript; charset=utf-8) or false - const contentType = mime.contentType(path.extname(filename)); + if (typeof headers !== "undefined") { + if (!Array.isArray(headers)) { + // eslint-disable-next-line guard-for-in + for (const name in headers) { + allHeaders.push({ key: name, value: headers[name] }); + } - // Only set content-type header if media type is known - // https://tools.ietf.org/html/rfc7231#section-3.1.1.5 - if (contentType) { - setResponseHeader(res, "Content-Type", contentType); - } else if (context.options.mimeTypeDefault) { - setResponseHeader( - res, - "Content-Type", - context.options.mimeTypeDefault, - ); + headers = allHeaders; + } + + for (const { key, value } of headers) { + setResponseHeader(res, key, value); + } } } @@ -667,6 +656,27 @@ function wrapper(context) { } } + if ( + !getResponseHeader(res, "Content-Type") || + getStatusCode(res) === 404 + ) { + removeResponseHeader(res, "Content-Type"); + // content-type name(like application/javascript; charset=utf-8) or false + const contentType = mime.contentType(path.extname(filename)); + + // Only set content-type header if media type is known + // https://tools.ietf.org/html/rfc7231#section-3.1.1.5 + if (contentType) { + setResponseHeader(res, "Content-Type", contentType); + } else if (context.options.mimeTypeDefault) { + setResponseHeader( + res, + "Content-Type", + context.options.mimeTypeDefault, + ); + } + } + // Conditional GET support if (isConditionalGET()) { if (isPreconditionFailure()) { diff --git a/src/utils/compatibleAPI.js b/src/utils/compatibleAPI.js index b86e32481..7e5518b15 100644 --- a/src/utils/compatibleAPI.js +++ b/src/utils/compatibleAPI.js @@ -19,6 +19,7 @@ * @property {(data: string | Buffer) => void} [send] * @property {(data?: string | Buffer) => void} [finish] * @property {() => string[]} [getResponseHeaders] + * @property {() => boolean} [getHeadersSent] * @property {(data: any) => void} [stream] * @property {() => any} [getOutgoing] * @property {(name: string, value: any) => void} [setState] @@ -147,6 +148,20 @@ function getResponseHeaders(res) { return res.getHeaderNames(); } +/** + * @template {ServerResponse & ExpectedServerResponse} Response + * @param {Response} res + * @returns {boolean} + */ +function getHeadersSent(res) { + // Pseudo API + if (typeof res.getHeadersSent === "function") { + return res.getHeadersSent(); + } + + return res.headersSent; +} + /** * @template {ServerResponse & ExpectedServerResponse} Response * @param {Response} res @@ -305,6 +320,7 @@ module.exports = { setResponseHeader, removeResponseHeader, getResponseHeaders, + getHeadersSent, pipe, send, finish, diff --git a/src/utils/getFilenameFromUrl.js b/src/utils/getFilenameFromUrl.js index 0fe40c232..d3fd80e7e 100644 --- a/src/utils/getFilenameFromUrl.js +++ b/src/utils/getFilenameFromUrl.js @@ -129,6 +129,7 @@ function getFilenameFromUrl(context, url, extra = {}) { pathname.slice(publicPathObject.pathname.length), ); + // eslint-disable-next-line no-param-reassign extra.immutable = assetInfo ? assetInfo.immutable : false; } diff --git a/test/middleware.test.js b/test/middleware.test.js index d17db2715..a9f6ac17a 100644 --- a/test/middleware.test.js +++ b/test/middleware.test.js @@ -3287,6 +3287,97 @@ describe.each([ ); }); }); + + describe("should work when headers are already sent", () => { + let compiler; + + const outputPath = path.resolve( + __dirname, + "./outputs/basic-test-errors-headers-sent", + ); + + beforeAll(async () => { + compiler = getCompiler({ + ...webpackConfig, + output: { + filename: "bundle.js", + path: outputPath, + }, + }); + + [server, req, instance] = await frameworkFactory( + name, + framework, + compiler, + {}, + { + setupMiddlewares: (middlewares) => { + if (name === "hapi") { + // There's no such thing as "the next route handler" in hapi. One request is matched to one or no route handlers. + } else if (name === "koa") { + middlewares.push(async (ctx, next) => { + // eslint-disable-next-line no-param-reassign + ctx.url = "/index.html"; + + await next(); + }); + middlewares.push(middleware.koaWrapper(compiler, {})); + } else if (name === "hono") { + middlewares.unshift(async (c, next) => { + await next(); + + return new Response("Hello Node.js!"); + }); + middlewares.push(middleware.honoWrapper(compiler, {})); + } else { + middlewares.push({ + route: "/", + fn: (req, res, next) => { + // eslint-disable-next-line no-param-reassign + req.url = "/index.html"; + next(); + }, + }); + middlewares.push(middleware(compiler, {})); + } + + return middlewares; + }, + }, + ); + + instance.context.outputFileSystem.mkdirSync(outputPath, { + recursive: true, + }); + instance.context.outputFileSystem.writeFileSync( + path.resolve(outputPath, "index.html"), + "HTML", + ); + }); + + afterAll(async () => { + await close(server, instance); + }); + + it('should return the "200" code for the "GET" request to the bundle file', async () => { + const response = await req.get("/"); + + expect(response.statusCode).toEqual(200); + expect(response.headers["content-type"]).toEqual( + "text/html; charset=utf-8", + ); + }); + + it('should return the "200" code for the "HEAD" request to the bundle file', async () => { + const response = await req.head("/"); + + expect(response.statusCode).toEqual(200); + expect(response.headers["content-type"]).toEqual( + "text/html; charset=utf-8", + ); + expect(response.text).toBeUndefined(); + }); + }); }); describe("mimeTypes option", () => { @@ -4467,6 +4558,7 @@ describe.each([ middlewares.push(async (ctx, next) => { locals = ctx.state; + // eslint-disable-next-line no-param-reassign ctx.status = 200; await next(); diff --git a/types/index.d.ts b/types/index.d.ts index f6886e1bd..18c9971dd 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -213,7 +213,7 @@ declare namespace wdm { /** * @template S * @template O - * @typedef {HapiPluginBase & { pkg: { name: string } }} HapiPlugin + * @typedef {HapiPluginBase & { pkg: { name: string }, multiple: boolean }} HapiPlugin */ /** * @typedef {Options & { compiler: Compiler | MultiCompiler }} HapiOptions @@ -409,6 +409,7 @@ type HapiPlugin = HapiPluginBase & { pkg: { name: string; }; + multiple: boolean; }; type HapiOptions = Options & { compiler: Compiler | MultiCompiler; diff --git a/types/utils/compatibleAPI.d.ts b/types/utils/compatibleAPI.d.ts index 5a3ab777a..7500fcd0c 100644 --- a/types/utils/compatibleAPI.d.ts +++ b/types/utils/compatibleAPI.d.ts @@ -22,6 +22,7 @@ export type ExpectedServerResponse = { send?: ((data: string | Buffer) => void) | undefined; finish?: ((data?: string | Buffer) => void) | undefined; getResponseHeaders?: (() => string[]) | undefined; + getHeadersSent?: (() => boolean) | undefined; stream?: ((data: any) => void) | undefined; getOutgoing?: (() => any) | undefined; setState?: ((name: string, value: any) => void) | undefined; @@ -64,6 +65,7 @@ export function getStatusCode< * @property {(data: string | Buffer) => void} [send] * @property {(data?: string | Buffer) => void} [finish] * @property {() => string[]} [getResponseHeaders] + * @property {() => boolean} [getHeadersSent] * @property {(data: any) => void} [stream] * @property {() => any} [getOutgoing] * @property {(name: string, value: any) => void} [setState] @@ -133,6 +135,14 @@ export function removeResponseHeader< export function getResponseHeaders< Response extends ServerResponse & ExpectedServerResponse, >(res: Response): string[]; +/** + * @template {ServerResponse & ExpectedServerResponse} Response + * @param {Response} res + * @returns {boolean} + */ +export function getHeadersSent< + Response extends ServerResponse & ExpectedServerResponse, +>(res: Response): boolean; /** * @template {ServerResponse & ExpectedServerResponse} Response * @param {Response} res