fix(Deno.serve): accessing .url on cloned request throws (#19869)

This PR fixes #19818. The problem was that the new InnerRequest class does not initialize the fields urlList and urlListProcessed that are used during a request clone. The solution aims to be straightforward by simply initializing the missing properties during the clone process. I also implemented a "cache" to the url getter of the new InnerRequest, avoiding the cost of calling op_http_get_request_method_and_url.

cli/tests/unit/serve_test.ts

@@ -2789,6 +2789,7 @@ Deno.test(
 
 // https://github.com/denoland/deno/issues/15858
 Deno.test(
+  "Clone should work",
   { permissions: { net: true } },
   async function httpServerCanCloneRequest() {
     const ac = new AbortController();
@@ -2799,6 +2800,20 @@ Deno.test(
         const cloned = req.clone();
         assertEquals(req.headers, cloned.headers);
 
+        assertEquals(cloned.url, req.url);
+        assertEquals(cloned.cache, req.cache);
+        assertEquals(cloned.destination, req.destination);
+        assertEquals(cloned.headers, req.headers);
+        assertEquals(cloned.integrity, req.integrity);
+        assertEquals(cloned.isHistoryNavigation, req.isHistoryNavigation);
+        assertEquals(cloned.isReloadNavigation, req.isReloadNavigation);
+        assertEquals(cloned.keepalive, req.keepalive);
+        assertEquals(cloned.method, req.method);
+        assertEquals(cloned.mode, req.mode);
+        assertEquals(cloned.redirect, req.redirect);
+        assertEquals(cloned.referrer, req.referrer);
+        assertEquals(cloned.referrerPolicy, req.referrerPolicy);
+
         // both requests can read body
         await req.text();
         await cloned.json();
@@ -2826,6 +2841,87 @@ Deno.test(
   },
 );
 
+// https://fetch.spec.whatwg.org/#dom-request-clone
+Deno.test(
+  "Throw if disturbed",
+  { permissions: { net: true } },
+  async function shouldThrowIfBodyIsUnusableDisturbed() {
+    const ac = new AbortController();
+    const listeningPromise = deferred();
+
+    const server = Deno.serve({
+      handler: async (req) => {
+        await req.text();
+
+        try {
+          req.clone();
+        } catch (error) {
+          assert(error instanceof TypeError);
+          assert(
+            error.message.endsWith("Body is unusable."),
+          );
+
+          ac.abort();
+          await server.finished;
+        }
+
+        return new Response("ok");
+      },
+      signal: ac.signal,
+      onListen: ({ port }: { port: number }) => listeningPromise.resolve(port),
+    });
+
+    try {
+      const port = await listeningPromise;
+      await fetch(`http://localhost:${port}/`, {
+        headers: { connection: "close" },
+        method: "POST",
+        body: '{"bar":true}',
+      });
+    } catch (e) {
+      assert(e instanceof TypeError);
+    } finally {
+      ac.abort();
+      await server.finished;
+    }
+  },
+);
+
+// TODO(fbaltor): As it is this test should throw and fail
+// https://fetch.spec.whatwg.org/#dom-request-clone
+Deno.test({
+  name: "Throw if locked",
+  ignore: true,
+  permissions: { net: true },
+  fn: async function shouldThrowIfBodyIsUnusableLocked() {
+    const ac = new AbortController();
+    const listeningPromise = deferred();
+
+    const server = Deno.serve({
+      handler: (req) => {
+        const _reader = req.body?.getReader();
+
+        req.clone();
+
+        return new Response("ok");
+      },
+      signal: ac.signal,
+      onListen: ({ port }: { port: number }) => listeningPromise.resolve(port),
+    });
+
+    const port = await listeningPromise;
+    const resp = await fetch(`http://localhost:${port}/`, {
+      headers: { connection: "close" },
+      method: "POST",
+      body: '{"bar":true}',
+    });
+
+    ac.abort();
+    await resp.body?.cancel();
+    await server.finished;
+  },
+});
+
 // Checks large streaming response
 // https://github.com/denoland/deno/issues/16567
 Deno.test(

ext/fetch/23_request.js

@@ -174,8 +174,8 @@ function cloneInnerRequest(request, skipBody = false) {
     body,
     redirectMode: request.redirectMode,
     redirectCount: request.redirectCount,
-    urlList: request.urlList,
-    urlListProcessed: request.urlListProcessed,
+    urlList: [() => request.url()],
+    urlListProcessed: [request.url()],
     clientRid: request.clientRid,
     blobUrlEntry: request.blobUrlEntry,
     url() {

ext/http/00_serve.js

@@ -129,6 +129,7 @@ class InnerRequest {
   #streamRid;
   #body;
   #upgraded;
+  #urlValue;
 
   constructor(slabId, context) {
     this.#slabId = slabId;
@@ -236,6 +237,10 @@ class InnerRequest {
   }
 
   url() {
+    if (this.#urlValue !== undefined) {
+      return this.#urlValue;
+    }
+
     if (this.#methodAndUri === undefined) {
       if (this.#slabId === undefined) {
         throw new TypeError("request closed");
@@ -249,27 +254,28 @@ class InnerRequest {
 
     // * is valid for OPTIONS
     if (path === "*") {
-      return "*";
+      return this.#urlValue = "*";
     }
 
     // If the path is empty, return the authority (valid for CONNECT)
     if (path == "") {
-      return this.#methodAndUri[1];
+      return this.#urlValue = this.#methodAndUri[1];
     }
 
     // CONNECT requires an authority
     if (this.#methodAndUri[0] == "CONNECT") {
-      return this.#methodAndUri[1];
+      return this.#urlValue = this.#methodAndUri[1];
     }
 
     const hostname = this.#methodAndUri[1];
     if (hostname) {
       // Construct a URL from the scheme, the hostname, and the path
-      return this.#context.scheme + hostname + path;
+      return this.#urlValue = this.#context.scheme + hostname + path;
     }
 
     // Construct a URL from the scheme, the fallback hostname, and the path
-    return this.#context.scheme + this.#context.fallbackHost + path;
+    return this.#urlValue = this.#context.scheme + this.#context.fallbackHost +
+      path;
   }
 
   get remoteAddr() {