yukey

e2e: a malformed percent-escape is a 404, not a crash

commit 0c5f1e6583eb6f43524d4095b6c17ff7cc768cc5 · parent 7defb72359 · browse this tree
Ximianth, 2026-10-10 05:50:33 UTC

decodeURIComponent throws on /%zz; it was outside the try, so the request
handler threw and took the whole run down (Chrome and profile dir orphaned).
Reproduced with a fetch against a started harness; 404 now.

The comment credited startsWith with the .. protection; path.normalize
is what drops leading .. from an absolute path.

diff --git a/packages/e2e/harness.ts b/packages/e2e/harness.ts
index abf1dfc..a485a96 100644
--- a/packages/e2e/harness.ts
+++ b/packages/e2e/harness.ts
@@ -43,17 +43,19 @@ async function pollUntil(
 	throw new Error(`timed out waiting for ${what}`);
 }
 
-// Serves STATIC_DIR read-only on an ephemeral port.  Paths are normalized and
-// pinned under STATIC_DIR; anything else is a 404 (a test server, not a product).
+// Serves STATIC_DIR read-only on an ephemeral port.  Anything that isn't a
+// readable file under it is a 404 (a test server, not a product), including a
+// malformed percent-escape.  `..` can't escape: normalize() drops leading `..`
+// from an absolute path, so the startsWith check below is a pin, not the guard.
 function serveStatic(): Promise<{ baseUrl: string; server: Server }> {
 	const srv = createServer((req, res) => {
-		const rel = normalize(decodeURIComponent(new URL(req.url ?? "/", "http://x").pathname));
-		const file = join(STATIC_DIR, rel === "/" ? "gallery.html" : rel);
-		if (!file.startsWith(STATIC_DIR)) {
-			res.writeHead(404).end();
-			return;
-		}
 		try {
+			const rel = normalize(decodeURIComponent(new URL(req.url ?? "/", "http://x").pathname));
+			const file = join(STATIC_DIR, rel === "/" ? "gallery.html" : rel);
+			if (!file.startsWith(STATIC_DIR)) {
+				res.writeHead(404).end();
+				return;
+			}
 			const body = readFileSync(file);
 			res.writeHead(200, { "content-type": MIME[extname(file)] ?? "application/octet-stream" });
 			res.end(body);