Skip to content

Commit c20f1d9

Browse files
alexander-akaitalexander-akait
authored
fix: no crash when headers are already sent (#1929)
1 parent 0f9a6c9 commit c20f1d9

7 files changed

+180
-42
lines changed

‎src/index.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ function wdm(compiler, options = {}) {
305305
/**
306306
* @template S
307307
* @template O
308-
* @typedef {HapiPluginBase<S, O> & { pkg: { name: string } }} HapiPlugin
308+
* @typedef {HapiPluginBase<S, O> & { pkg: { name: string }, multiple: boolean }} HapiPlugin
309309
*/
310310

311311
/**
@@ -322,6 +322,8 @@ function hapiWrapper() {
322322
pkg: {
323323
name: "webpack-dev-middleware",
324324
},
325+
// Allow to have multiple middleware
326+
multiple: true,
325327
register(server, options) {
326328
const { compiler, ...rest } = options;
327329

@@ -332,7 +334,11 @@ function hapiWrapper() {
332334
const devMiddleware = wdm(compiler, rest);
333335

334336
// @ts-ignore
335-
server.decorate("server", "webpackDevMiddleware", devMiddleware);
337+
if (!server.decorations.server.includes("webpackDevMiddleware")) {
338+
// @ts-ignore
339+
server.decorate("server", "webpackDevMiddleware", devMiddleware);
340+
}
341+
336342
// @ts-ignore
337343
server.ext("onRequest", (request, h) =>
338344
new Promise((resolve, reject) => {
@@ -568,6 +574,8 @@ function honoWrapper(compiler, options) {
568574

569575
res.getReadyReadableStreamState = () => "readable";
570576

577+
res.getHeadersSent = () => c.env.outgoing.headersSent;
578+
571579
let body;
572580

573581
try {

‎src/middleware.js

+49-39
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const {
1515
setResponseHeader,
1616
removeResponseHeader,
1717
getResponseHeaders,
18+
getHeadersSent,
1819
send,
1920
finish,
2021
pipe,
@@ -494,56 +495,44 @@ function wrapper(context) {
494495
return;
495496
}
496497

498+
if (getHeadersSent(res)) {
499+
await goNext();
500+
return;
501+
}
502+
497503
const { size } = /** @type {import("fs").Stats} */ (extra.stats);
498504

499505
let len = size;
500506
let offset = 0;
501507

502508
// Send logic
503-
let { headers } = context.options;
509+
if (context.options.headers) {
510+
let { headers } = context.options;
504511

505-
if (typeof headers === "function") {
506-
headers = /** @type {NormalizedHeaders} */ (headers(req, res, context));
507-
}
508-
509-
/**
510-
* @type {{key: string, value: string | number}[]}
511-
*/
512-
const allHeaders = [];
513-
514-
if (typeof headers !== "undefined") {
515-
if (!Array.isArray(headers)) {
516-
// eslint-disable-next-line guard-for-in
517-
for (const name in headers) {
518-
allHeaders.push({ key: name, value: headers[name] });
519-
}
520-
521-
headers = allHeaders;
512+
if (typeof headers === "function") {
513+
headers = /** @type {NormalizedHeaders} */ (
514+
headers(req, res, context)
515+
);
522516
}
523517

524-
for (const { key, value } of headers) {
525-
setResponseHeader(res, key, value);
526-
}
527-
}
518+
/**
519+
* @type {{key: string, value: string | number}[]}
520+
*/
521+
const allHeaders = [];
528522

529-
if (
530-
!getResponseHeader(res, "Content-Type") ||
531-
getStatusCode(res) === 404
532-
) {
533-
removeResponseHeader(res, "Content-Type");
534-
// content-type name(like application/javascript; charset=utf-8) or false
535-
const contentType = mime.contentType(path.extname(filename));
523+
if (typeof headers !== "undefined") {
524+
if (!Array.isArray(headers)) {
525+
// eslint-disable-next-line guard-for-in
526+
for (const name in headers) {
527+
allHeaders.push({ key: name, value: headers[name] });
528+
}
536529

537-
// Only set content-type header if media type is known
538-
// https://tools.ietf.org/html/rfc7231#section-3.1.1.5
539-
if (contentType) {
540-
setResponseHeader(res, "Content-Type", contentType);
541-
} else if (context.options.mimeTypeDefault) {
542-
setResponseHeader(
543-
res,
544-
"Content-Type",
545-
context.options.mimeTypeDefault,
546-
);
530+
headers = allHeaders;
531+
}
532+
533+
for (const { key, value } of headers) {
534+
setResponseHeader(res, key, value);
535+
}
547536
}
548537
}
549538

@@ -667,6 +656,27 @@ function wrapper(context) {
667656
}
668657
}
669658

659+
if (
660+
!getResponseHeader(res, "Content-Type") ||
661+
getStatusCode(res) === 404
662+
) {
663+
removeResponseHeader(res, "Content-Type");
664+
// content-type name(like application/javascript; charset=utf-8) or false
665+
const contentType = mime.contentType(path.extname(filename));
666+
667+
// Only set content-type header if media type is known
668+
// https://tools.ietf.org/html/rfc7231#section-3.1.1.5
669+
if (contentType) {
670+
setResponseHeader(res, "Content-Type", contentType);
671+
} else if (context.options.mimeTypeDefault) {
672+
setResponseHeader(
673+
res,
674+
"Content-Type",
675+
context.options.mimeTypeDefault,
676+
);
677+
}
678+
}
679+
670680
// Conditional GET support
671681
if (isConditionalGET()) {
672682
if (isPreconditionFailure()) {

‎src/utils/compatibleAPI.js

+16
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* @property {(data: string | Buffer) => void} [send]
2020
* @property {(data?: string | Buffer) => void} [finish]
2121
* @property {() => string[]} [getResponseHeaders]
22+
* @property {() => boolean} [getHeadersSent]
2223
* @property {(data: any) => void} [stream]
2324
* @property {() => any} [getOutgoing]
2425
* @property {(name: string, value: any) => void} [setState]
@@ -147,6 +148,20 @@ function getResponseHeaders(res) {
147148
return res.getHeaderNames();
148149
}
149150

151+
/**
152+
* @template {ServerResponse & ExpectedServerResponse} Response
153+
* @param {Response} res
154+
* @returns {boolean}
155+
*/
156+
function getHeadersSent(res) {
157+
// Pseudo API
158+
if (typeof res.getHeadersSent === "function") {
159+
return res.getHeadersSent();
160+
}
161+
162+
return res.headersSent;
163+
}
164+
150165
/**
151166
* @template {ServerResponse & ExpectedServerResponse} Response
152167
* @param {Response} res
@@ -305,6 +320,7 @@ module.exports = {
305320
setResponseHeader,
306321
removeResponseHeader,
307322
getResponseHeaders,
323+
getHeadersSent,
308324
pipe,
309325
send,
310326
finish,

‎src/utils/getFilenameFromUrl.js

+1
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ function getFilenameFromUrl(context, url, extra = {}) {
129129
pathname.slice(publicPathObject.pathname.length),
130130
);
131131

132+
// eslint-disable-next-line no-param-reassign
132133
extra.immutable = assetInfo ? assetInfo.immutable : false;
133134
}
134135

‎test/middleware.test.js

+92
Original file line numberDiff line numberDiff line change
@@ -3287,6 +3287,97 @@ describe.each([
32873287
);
32883288
});
32893289
});
3290+
3291+
describe("should work when headers are already sent", () => {
3292+
let compiler;
3293+
3294+
const outputPath = path.resolve(
3295+
__dirname,
3296+
"./outputs/basic-test-errors-headers-sent",
3297+
);
3298+
3299+
beforeAll(async () => {
3300+
compiler = getCompiler({
3301+
...webpackConfig,
3302+
output: {
3303+
filename: "bundle.js",
3304+
path: outputPath,
3305+
},
3306+
});
3307+
3308+
[server, req, instance] = await frameworkFactory(
3309+
name,
3310+
framework,
3311+
compiler,
3312+
{},
3313+
{
3314+
setupMiddlewares: (middlewares) => {
3315+
if (name === "hapi") {
3316+
// There's no such thing as "the next route handler" in hapi. One request is matched to one or no route handlers.
3317+
} else if (name === "koa") {
3318+
middlewares.push(async (ctx, next) => {
3319+
// eslint-disable-next-line no-param-reassign
3320+
ctx.url = "/index.html";
3321+
3322+
await next();
3323+
});
3324+
middlewares.push(middleware.koaWrapper(compiler, {}));
3325+
} else if (name === "hono") {
3326+
middlewares.unshift(async (c, next) => {
3327+
await next();
3328+
3329+
return new Response("Hello Node.js!");
3330+
});
3331+
middlewares.push(middleware.honoWrapper(compiler, {}));
3332+
} else {
3333+
middlewares.push({
3334+
route: "/",
3335+
fn: (req, res, next) => {
3336+
// eslint-disable-next-line no-param-reassign
3337+
req.url = "/index.html";
3338+
next();
3339+
},
3340+
});
3341+
middlewares.push(middleware(compiler, {}));
3342+
}
3343+
3344+
return middlewares;
3345+
},
3346+
},
3347+
);
3348+
3349+
instance.context.outputFileSystem.mkdirSync(outputPath, {
3350+
recursive: true,
3351+
});
3352+
instance.context.outputFileSystem.writeFileSync(
3353+
path.resolve(outputPath, "index.html"),
3354+
"HTML",
3355+
);
3356+
});
3357+
3358+
afterAll(async () => {
3359+
await close(server, instance);
3360+
});
3361+
3362+
it('should return the "200" code for the "GET" request to the bundle file', async () => {
3363+
const response = await req.get("/");
3364+
3365+
expect(response.statusCode).toEqual(200);
3366+
expect(response.headers["content-type"]).toEqual(
3367+
"text/html; charset=utf-8",
3368+
);
3369+
});
3370+
3371+
it('should return the "200" code for the "HEAD" request to the bundle file', async () => {
3372+
const response = await req.head("/");
3373+
3374+
expect(response.statusCode).toEqual(200);
3375+
expect(response.headers["content-type"]).toEqual(
3376+
"text/html; charset=utf-8",
3377+
);
3378+
expect(response.text).toBeUndefined();
3379+
});
3380+
});
32903381
});
32913382

32923383
describe("mimeTypes option", () => {
@@ -4467,6 +4558,7 @@ describe.each([
44674558
middlewares.push(async (ctx, next) => {
44684559
locals = ctx.state;
44694560

4561+
// eslint-disable-next-line no-param-reassign
44704562
ctx.status = 200;
44714563

44724564
await next();

‎types/index.d.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ declare namespace wdm {
213213
/**
214214
* @template S
215215
* @template O
216-
* @typedef {HapiPluginBase<S, O> & { pkg: { name: string } }} HapiPlugin
216+
* @typedef {HapiPluginBase<S, O> & { pkg: { name: string }, multiple: boolean }} HapiPlugin
217217
*/
218218
/**
219219
* @typedef {Options & { compiler: Compiler | MultiCompiler }} HapiOptions
@@ -409,6 +409,7 @@ type HapiPlugin<S, O> = HapiPluginBase<S, O> & {
409409
pkg: {
410410
name: string;
411411
};
412+
multiple: boolean;
412413
};
413414
type HapiOptions = Options & {
414415
compiler: Compiler | MultiCompiler;

‎types/utils/compatibleAPI.d.ts

+10
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export type ExpectedServerResponse = {
2222
send?: ((data: string | Buffer) => void) | undefined;
2323
finish?: ((data?: string | Buffer) => void) | undefined;
2424
getResponseHeaders?: (() => string[]) | undefined;
25+
getHeadersSent?: (() => boolean) | undefined;
2526
stream?: ((data: any) => void) | undefined;
2627
getOutgoing?: (() => any) | undefined;
2728
setState?: ((name: string, value: any) => void) | undefined;
@@ -64,6 +65,7 @@ export function getStatusCode<
6465
* @property {(data: string | Buffer) => void} [send]
6566
* @property {(data?: string | Buffer) => void} [finish]
6667
* @property {() => string[]} [getResponseHeaders]
68+
* @property {() => boolean} [getHeadersSent]
6769
* @property {(data: any) => void} [stream]
6870
* @property {() => any} [getOutgoing]
6971
* @property {(name: string, value: any) => void} [setState]
@@ -133,6 +135,14 @@ export function removeResponseHeader<
133135
export function getResponseHeaders<
134136
Response extends ServerResponse & ExpectedServerResponse,
135137
>(res: Response): string[];
138+
/**
139+
* @template {ServerResponse & ExpectedServerResponse} Response
140+
* @param {Response} res
141+
* @returns {boolean}
142+
*/
143+
export function getHeadersSent<
144+
Response extends ServerResponse & ExpectedServerResponse,
145+
>(res: Response): boolean;
136146
/**
137147
* @template {ServerResponse & ExpectedServerResponse} Response
138148
* @param {Response} res

0 commit comments

Comments
 (0)