Skip to content

Commit

Permalink
fix: no crash when headers are already sent (#1929)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexander-akait authored Aug 21, 2024
1 parent 0f9a6c9 commit c20f1d9
Show file tree
Hide file tree
Showing 7 changed files with 180 additions and 42 deletions.
12 changes: 10 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ function wdm(compiler, options = {}) {
/**
* @template S
* @template O
* @typedef {HapiPluginBase<S, O> & { pkg: { name: string } }} HapiPlugin
* @typedef {HapiPluginBase<S, O> & { pkg: { name: string }, multiple: boolean }} HapiPlugin
*/

/**
Expand All @@ -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;

Expand All @@ -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) => {
Expand Down Expand Up @@ -568,6 +574,8 @@ function honoWrapper(compiler, options) {

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

res.getHeadersSent = () => c.env.outgoing.headersSent;

let body;

try {
Expand Down
88 changes: 49 additions & 39 deletions src/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const {
setResponseHeader,
removeResponseHeader,
getResponseHeaders,
getHeadersSent,
send,
finish,
pipe,
Expand Down Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -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()) {
Expand Down
16 changes: 16 additions & 0 deletions src/utils/compatibleAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -305,6 +320,7 @@ module.exports = {
setResponseHeader,
removeResponseHeader,
getResponseHeaders,
getHeadersSent,
pipe,
send,
finish,
Expand Down
1 change: 1 addition & 0 deletions src/utils/getFilenameFromUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
92 changes: 92 additions & 0 deletions test/middleware.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ declare namespace wdm {
/**
* @template S
* @template O
* @typedef {HapiPluginBase<S, O> & { pkg: { name: string } }} HapiPlugin
* @typedef {HapiPluginBase<S, O> & { pkg: { name: string }, multiple: boolean }} HapiPlugin
*/
/**
* @typedef {Options & { compiler: Compiler | MultiCompiler }} HapiOptions
Expand Down Expand Up @@ -409,6 +409,7 @@ type HapiPlugin<S, O> = HapiPluginBase<S, O> & {
pkg: {
name: string;
};
multiple: boolean;
};
type HapiOptions = Options & {
compiler: Compiler | MultiCompiler;
Expand Down
10 changes: 10 additions & 0 deletions types/utils/compatibleAPI.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit c20f1d9

Please sign in to comment.