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.
timvandermeij · Feb 11, 2024 · af57642 · af57642
1 parent 336fcff
commit af57642
Showing 1 changed file with 71 additions and 73 deletions.
diff --git a/test/webserver.mjs b/test/webserver.mjs
@@ -162,7 +162,7 @@ class WebServer {
         return;
       }
       if (isDir) {
-        serveDirectoryIndex(filePath);
+        self.#serveDirectoryIndex(res, pathPart, queryPart, filePath);
         return;
       }
 
@@ -196,97 +196,95 @@ class WebServer {
       }
       self.#serveFile(res, filePath, fileSize);
     }
+  }
 
-    function escapeHTML(untrusted) {
+  #serveDirectoryIndex(response, pathPart, queryPart, directory) {
+    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("&", "&amp;")
         .replaceAll("<", "&lt;")
         .replaceAll(">", "&gt;")
         .replaceAll('"', "&quot;")
         .replaceAll("'", "&#39;");
-    }
 
-    function serveDirectoryIndex(dir) {
-      res.setHeader("Content-Type", "text/html");
-      res.writeHead(200);
+    response.setHeader("Content-Type", "text/html");
+    response.writeHead(200);
 
-      if (queryPart === "frame") {
-        res.end(
-          "<html><frameset cols=*,200><frame name=pdf>" +
-            '<frame src="' +
-            encodeURI(pathPart) +
-            '?side"></frameset></html>',
-          "utf8"
-        );
+    if (queryPart === "frame") {
+      response.end(
+        `<html>
+          <frameset cols=*,200>
+            <frame name=pdf>
+            <frame src="${encodeURI(pathPart)}?side">
+          </frameset>
+        </html>`,
+        "utf8"
+      );
+      return;
+    }
+
+    const all = queryPart === "all";
+    fs.readdir(directory, function (error, files) {
+      if (error) {
+        response.end();
         return;
       }
-      var all = queryPart === "all";
-      fs.readdir(dir, function (err, files) {
-        if (err) {
-          res.end();
-          return;
-        }
-        res.write(
-          '<html><head><meta charset="utf-8"></head><body>' +
-            "<h1>PDFs of " +
-            pathPart +
-            "</h1>\n"
-        );
-        if (pathPart !== "/") {
-          res.write('<a href="..">..</a><br>\n');
+      response.write(
+        `<html>
+           <head>
+             <meta charset="utf-8">
+           </head>
+           <body>
+             <h1>Index of ${pathPart}</h1>
+        `
+      );
+      if (pathPart !== "/") {
+        response.write('<a href="..">..</a><br>');
+      }
+      files.forEach(file => {
+        let stat;
+        const item = pathPart + file;
+        let href = "";
+        let label = "";
+        let extraAttributes = "";
+        try {
+          stat = fs.statSync(path.join(directory, file));
+        } catch (ex) {
+          href = encodeURI(item);
+          label = `${file} (${ex})`;
+          extraAttributes = ' style="color:red"';
         }
-        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) {
+        if (stat) {
+          if (stat.isDirectory()) {
             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(
-              '<a href="' +
-                escapeHTML(href) +
-                '"' +
-                extraAttributes +
-                ">" +
-                escapeHTML(label) +
-                "</a><br>\n"
-            );
+            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 (files.length === 0) {
-          res.write("<p>no files found</p>\n");
         }
-        if (!all && queryPart !== "side") {
-          res.write(
-            "<hr><p>(only PDF files are shown, " +
-              '<a href="?all">show all</a>)</p>\n'
+        if (label) {
+          response.write(
+            `<a href="${escapeHTML(href)}"${extraAttributes}>${escapeHTML(label)}</a><br>`
           );
         }
-        res.end("</body></html>");
       });
-    }
+      if (files.length === 0) {
+        response.write("<p>No files found</p>");
+      }
+      if (!all && queryPart !== "side") {
+        response.write(
+          '<hr><p>(only PDF files are shown, <a href="?all">show all</a>)</p>'
+        );
+      }
+      response.end("</body></html>");
+    });
   }
 
   #serveFile(response, filePath, fileSize) {