From 12b96857141fba3002ed2762eed9aa1076501a9c Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 11 Feb 2024 13:37:01 +0100 Subject: [PATCH 1/4] Convert the webserver to a proper class with private methods --- test/webserver.mjs | 50 +++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/test/webserver.mjs b/test/webserver.mjs index b5b1e7c30..25d7f1ed0 100644 --- a/test/webserver.mjs +++ b/test/webserver.mjs @@ -38,33 +38,36 @@ var mimeTypes = { var defaultMimeType = "application/octet-stream"; -function WebServer() { - this.root = "."; - this.host = "localhost"; - this.port = 0; - this.server = null; - this.verbose = false; - this.cacheExpirationTime = 0; - this.disableRangeRequests = false; - this.hooks = { - GET: [crossOriginHandler], - POST: [], - }; -} -WebServer.prototype = { +class WebServer { + constructor() { + this.root = "."; + this.host = "localhost"; + this.port = 0; + this.server = null; + this.verbose = false; + this.cacheExpirationTime = 0; + this.disableRangeRequests = false; + this.hooks = { + GET: [crossOriginHandler], + POST: [], + }; + } + start(callback) { - this._ensureNonZeroPort(); - this.server = http.createServer(this._handler.bind(this)); + this.#ensureNonZeroPort(); + this.server = http.createServer(this.#handler.bind(this)); this.server.listen(this.port, this.host, callback); console.log( "Server running at http://" + this.host + ":" + this.port + "/" ); - }, + } + stop(callback) { this.server.close(callback); this.server = null; - }, - _ensureNonZeroPort() { + } + + #ensureNonZeroPort() { if (!this.port) { // If port is 0, a random port will be chosen instead. Do not set a host // name to make sure that the port is synchronously set by .listen(). @@ -76,8 +79,9 @@ WebServer.prototype = { this.port = address ? address.port : 8000; server.close(); } - }, - _handler(req, res) { + } + + #handler(req, res) { var url = req.url.replaceAll("//", "/"); var urlParts = /([^?]*)((?:\?(.*))?)/.exec(url); try { @@ -334,8 +338,8 @@ WebServer.prototype = { stream.pipe(res); } - }, -}; + } +} // This supports the "Cross-origin" test in test/unit/api_spec.js // It is here instead of test.js so that when the test will still complete as From 56d9930a7bd2fec5fe5cdf6f761533ba7b5cd4f8 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 11 Feb 2024 13:45:06 +0100 Subject: [PATCH 2/4] Extract and modernize the webserver's file serving code The `handler` method contained this code in an inline function, which made the `handler` method big and harder to read. Moreover, this code relied on variables from the outer scope, which made it harder to reason about because the inputs and outputs weren't easily visible. This commit fixes the problems by extracting the file serving code into a dedicated private method, and modernizing it to use e.g. `const`/`let` instead of `var` and using template strings. --- test/webserver.mjs | 54 ++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/test/webserver.mjs b/test/webserver.mjs index 25d7f1ed0..0149eadc5 100644 --- a/test/webserver.mjs +++ b/test/webserver.mjs @@ -82,6 +82,7 @@ class WebServer { } #handler(req, res) { + var self = this; var url = req.url.replaceAll("//", "/"); var urlParts = /([^?]*)((?:\?(.*))?)/.exec(url); try { @@ -126,7 +127,6 @@ class WebServer { } var disableRangeRequests = this.disableRangeRequests; - var cacheExpirationTime = this.cacheExpirationTime; var filePath; fs.realpath(path.join(this.root, pathPart), checkFile); @@ -192,7 +192,7 @@ class WebServer { if (verbose) { console.log(url); } - serveRequestedFile(filePath); + self.#serveFile(res, filePath, fileSize); } function escapeHTML(untrusted) { @@ -286,32 +286,6 @@ class WebServer { }); } - function serveRequestedFile(reqFilePath) { - var stream = fs.createReadStream(reqFilePath, { flags: "rs" }); - - stream.on("error", function (error) { - res.writeHead(500); - res.end(); - }); - - var ext = path.extname(reqFilePath).toLowerCase(); - var contentType = mimeTypes[ext] || defaultMimeType; - - if (!disableRangeRequests) { - res.setHeader("Accept-Ranges", "bytes"); - } - res.setHeader("Content-Type", contentType); - res.setHeader("Content-Length", fileSize); - if (cacheExpirationTime > 0) { - var expireTime = new Date(); - expireTime.setSeconds(expireTime.getSeconds() + cacheExpirationTime); - res.setHeader("Expires", expireTime.toUTCString()); - } - res.writeHead(200); - - stream.pipe(res); - } - function serveRequestedFileRange(reqFilePath, start, end) { var stream = fs.createReadStream(reqFilePath, { flags: "rs", @@ -339,6 +313,30 @@ class WebServer { stream.pipe(res); } } + + #serveFile(response, filePath, fileSize) { + const stream = fs.createReadStream(filePath, { flags: "rs" }); + stream.on("error", error => { + response.writeHead(500); + response.end(); + }); + + const extension = path.extname(filePath).toLowerCase(); + const contentType = mimeTypes[extension] || defaultMimeType; + + if (!this.disableRangeRequests) { + response.setHeader("Accept-Ranges", "bytes"); + } + response.setHeader("Content-Type", contentType); + response.setHeader("Content-Length", fileSize); + if (this.cacheExpirationTime > 0) { + const expireTime = new Date(); + expireTime.setSeconds(expireTime.getSeconds() + this.cacheExpirationTime); + response.setHeader("Expires", expireTime.toUTCString()); + } + response.writeHead(200); + stream.pipe(response); + } } // This supports the "Cross-origin" test in test/unit/api_spec.js From 336fcffd283f39b4a2663edcbe101cbb4843a8c5 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 11 Feb 2024 13:49:42 +0100 Subject: [PATCH 3/4] Extract and modernize the webserver's range file serving code The `handler` method contained this code in an inline function, which made the `handler` method big and harder to read. Moreover, this code relied on variables from the outer scope, which made it harder to reason about because the inputs and outputs weren't easily visible. This commit fixes the problems by extracting the range file serving code into a dedicated private method, and modernizing it to use e.g. `const`/ `let` instead of `var` and using template strings. --- test/webserver.mjs | 56 +++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/test/webserver.mjs b/test/webserver.mjs index 0149eadc5..57c913d66 100644 --- a/test/webserver.mjs +++ b/test/webserver.mjs @@ -182,8 +182,10 @@ class WebServer { if (verbose) { console.log(url + ": range " + start + " - " + end); } - serveRequestedFileRange( + self.#serveFileRange( + res, filePath, + fileSize, start, isNaN(end) ? fileSize : end + 1 ); @@ -285,33 +287,6 @@ class WebServer { res.end(""); }); } - - function serveRequestedFileRange(reqFilePath, start, end) { - var stream = fs.createReadStream(reqFilePath, { - flags: "rs", - start, - end: end - 1, - }); - - stream.on("error", function (error) { - res.writeHead(500); - res.end(); - }); - - var ext = path.extname(reqFilePath).toLowerCase(); - var contentType = mimeTypes[ext] || defaultMimeType; - - res.setHeader("Accept-Ranges", "bytes"); - res.setHeader("Content-Type", contentType); - res.setHeader("Content-Length", end - start); - res.setHeader( - "Content-Range", - "bytes " + start + "-" + (end - 1) + "/" + fileSize - ); - res.writeHead(206); - - stream.pipe(res); - } } #serveFile(response, filePath, fileSize) { @@ -337,6 +312,31 @@ class WebServer { response.writeHead(200); stream.pipe(response); } + + #serveFileRange(response, filePath, fileSize, start, end) { + const stream = fs.createReadStream(filePath, { + flags: "rs", + start, + end: end - 1, + }); + stream.on("error", error => { + response.writeHead(500); + response.end(); + }); + + const extension = path.extname(filePath).toLowerCase(); + const contentType = mimeTypes[extension] || defaultMimeType; + + response.setHeader("Accept-Ranges", "bytes"); + response.setHeader("Content-Type", contentType); + response.setHeader("Content-Length", end - start); + response.setHeader( + "Content-Range", + `bytes ${start}-${end - 1}/${fileSize}` + ); + response.writeHead(206); + stream.pipe(response); + } } // This supports the "Cross-origin" test in test/unit/api_spec.js From ce4fe0c23415d5a29eaa59cf75837e14b1795cb8 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 11 Feb 2024 16:46:01 +0100 Subject: [PATCH 4/4] Extract and modernize the webserver's directory listing code The `handler` method contained this code in an inline function, which made the `handler` method big and harder to read. Moreover, this code relied on variables from the outer scope, which made it harder to reason about because the inputs and outputs weren't easily visible. This commit fixes the problems by extracting the directory listing code into a dedicated private method, and modernizing it to use e.g. `const`/ `let` instead of `var` and using template strings. --- test/webserver.mjs | 161 +++++++++++++++++++++++---------------------- 1 file changed, 83 insertions(+), 78 deletions(-) diff --git a/test/webserver.mjs b/test/webserver.mjs index 57c913d66..f920165cf 100644 --- a/test/webserver.mjs +++ b/test/webserver.mjs @@ -16,6 +16,7 @@ /* eslint-disable no-var */ import fs from "fs"; +import fsPromises from "fs/promises"; import http from "http"; import path from "path"; @@ -162,7 +163,7 @@ class WebServer { return; } if (isDir) { - serveDirectoryIndex(filePath); + self.#serveDirectoryIndex(res, pathPart, queryPart, filePath); return; } @@ -196,97 +197,101 @@ class WebServer { } self.#serveFile(res, filePath, fileSize); } + } - function escapeHTML(untrusted) { + async #serveDirectoryIndex(response, pathPart, queryPart, directory) { + response.setHeader("Content-Type", "text/html"); + response.writeHead(200); + + if (queryPart === "frame") { + response.end( + ` + + + + + `, + "utf8" + ); + return; + } + + let files; + try { + files = await fsPromises.readdir(directory); + } catch { + response.end(); + return; + } + + response.write( + ` + + + + +

Index of ${pathPart}

` + ); + if (pathPart !== "/") { + response.write('..
'); + } + + const all = queryPart === "all"; + const escapeHTML = untrusted => // Escape untrusted input so that it can safely be used in a HTML response // in HTML and in HTML attributes. - return untrusted + untrusted .replaceAll("&", "&") .replaceAll("<", "<") .replaceAll(">", ">") .replaceAll('"', """) .replaceAll("'", "'"); - } - function serveDirectoryIndex(dir) { - res.setHeader("Content-Type", "text/html"); - res.writeHead(200); + for (const file of files) { + let stat; + const item = pathPart + file; + let href = ""; + let label = ""; + let extraAttributes = ""; - if (queryPart === "frame") { - res.end( - "" + - '', - "utf8" - ); - return; + try { + stat = fs.statSync(path.join(directory, file)); + } catch (ex) { + href = encodeURI(item); + label = `${file} (${ex})`; + extraAttributes = ' style="color:red"'; } - var all = queryPart === "all"; - fs.readdir(dir, function (err, files) { - if (err) { - res.end(); - return; + + if (stat) { + if (stat.isDirectory()) { + href = encodeURI(item); + label = file; + } else if (path.extname(file).toLowerCase() === ".pdf") { + href = `/web/viewer.html?file=${encodeURIComponent(item)}`; + label = file; + extraAttributes = ' target="pdf"'; + } else if (all) { + href = encodeURI(item); + label = file; } - res.write( - '' + - "

PDFs of " + - pathPart + - "

\n" + } + + if (label) { + response.write( + `${escapeHTML(label)}
` ); - if (pathPart !== "/") { - res.write('..
\n'); - } - files.forEach(function (file) { - var stat; - var item = pathPart + file; - var href = ""; - var label = ""; - var extraAttributes = ""; - try { - stat = fs.statSync(path.join(dir, file)); - } catch (e) { - href = encodeURI(item); - label = file + " (" + e + ")"; - extraAttributes = ' style="color:red"'; - } - if (stat) { - if (stat.isDirectory()) { - href = encodeURI(item); - label = file; - } else if (path.extname(file).toLowerCase() === ".pdf") { - href = "/web/viewer.html?file=" + encodeURIComponent(item); - label = file; - extraAttributes = ' target="pdf"'; - } else if (all) { - href = encodeURI(item); - label = file; - } - } - if (label) { - res.write( - '" + - escapeHTML(label) + - "
\n" - ); - } - }); - if (files.length === 0) { - res.write("

no files found

\n"); - } - if (!all && queryPart !== "side") { - res.write( - "

(only PDF files are shown, " + - 'show all)

\n' - ); - } - res.end(""); - }); + } } + + if (files.length === 0) { + response.write("

No files found

"); + } + if (!all && queryPart !== "side") { + response.write( + '

(only PDF files are shown, show all)

' + ); + } + response.end(""); } #serveFile(response, filePath, fileSize) {