Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cleaup stream and handle errors #1769

Merged
merged 5 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"myhtml",
"configurated",
"mycustom",
"commitlint"
"commitlint",
"nosniff"
],
"ignorePaths": [
"CHANGELOG.md",
Expand Down
15 changes: 12 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand All @@ -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",
Expand Down
118 changes: 116 additions & 2 deletions src/utils/compatibleAPI.js
Original file line number Diff line number Diff line change
@@ -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 */

Expand Down Expand Up @@ -88,6 +92,18 @@
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
Expand All @@ -108,6 +124,76 @@
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();

Check warning on line 148 in src/utils/compatibleAPI.js

View check run for this annotation

Codecov / codecov/patch

src/utils/compatibleAPI.js#L148

Added line #L148 was not covered by tests
}
},
);
}

if (typeof stream.addListener === "function" && suppress) {
stream.removeAllListeners("error");
stream.addListener("error", () => {});
}
}

/** @type {Record<number, string>} */
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 = `<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>${escapeHtml(msg)}</pre>
</body>
</html>`;

// 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
Expand All @@ -125,13 +211,42 @@

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;
}

Expand All @@ -141,7 +256,6 @@
) {
/** @type {Response & ExpectedResponse} */
(res).send(bufferOtStream);

return;
}

Expand Down
58 changes: 58 additions & 0 deletions src/utils/escapeHtml.js
Original file line number Diff line number Diff line change
@@ -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 = "&quot;";
break;
// &
case 38:
escape = "&amp;";
break;
// '
case 39:
escape = "&#39;";
break;
// <
case 60:
escape = "&lt;";
break;
// >
case 62:
escape = "&gt;";
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;