serve: build request.url through one function on every transport

src/runtime/server/server_body.rs

@@ -3263,32 +3263,10 @@ where
                 ))
             }));
             // NOTE: `ReqLike::{url,header}` both borrow `&mut req`; the
-            // returned slices alias the same uWS-owned header buffer. Format
-            // the `https://{host}` prefix while `host` is borrowed so the
-            // second `&mut req` borrow for `url` is unconflicted.
-            let prefix: Option<Vec<u8>> = ReqLike::header(req, b"host")
-                .filter(|host| Request::is_valid_host_header(host))
-                .map(|host| {
-                    let fmt = bun_fmt::HostFormatter {
-                        is_https: true,
-                        host,
-                        port: None,
-                    };
-                    let mut s = Vec::new();
-                    let _ = write!(&mut s, "https://{}", fmt);
-                    s
-                });
-            let path = ReqLike::url(req);
-            if !path.is_empty() && path[0] == b'/' {
-                if let Some(mut s) = prefix {
-                    s.extend_from_slice(path);
-                    request_object.url.set(BunString::clone_utf8(&s));
-                } else {
-                    request_object.url.set(BunString::clone_utf8(path));
-                }
-            } else {
-                request_object.url.set(BunString::clone_utf8(path));
-            }
+            // returned slices alias the same uWS-owned header buffer, so copy
+            // `host` out before borrowing again for the target.
+            let host: Option<Vec<u8>> = ReqLike::header(req, b"host").map(<[u8]>::to_vec);
+            request_object.set_synthesized_url(host.as_deref(), ReqLike::url(req));
             ctx.clear_req();
         }
 

src/runtime/webcore/Request.rs

@@ -822,20 +822,10 @@ impl Request {
         if let Some(req) = self.request_context.get_request() {
             // S008: `uws::Request` is an `opaque_ffi!` ZST handle — safe deref.
             let req = bun_opaque::opaque_deref(req);
-            let req_url = Self::request_target_path(req.url());
-            if !req_url.is_empty() && req_url[0] == b'/' {
-                if let Some(host) = req
-                    .header(b"host")
-                    .filter(|host| Self::is_valid_host_header(host))
-                {
-                    // With `port: None`, HostFormatter always emits exactly `host`, so the
-                    // formatted byte-count is just `host.len()`. Avoid the `core::fmt::write`
-                    // vtable dispatch that `bun_fmt::count(format_args!(...))` incurs — this
-                    // runs once per request via JSC extra-memory accounting.
-                    return self.get_protocol().len() + host.len() + req_url.len();
-                }
-            }
-            return req_url.len();
+            let (host, path) = Self::url_parts(req.header(b"host"), req.url());
+            // Canonicalizing can change the length slightly; this is an estimate for
+            // JSC's extra-memory accounting, so the joined length is close enough.
+            return host.map_or(0, |host| self.get_protocol().len() + host.len()) + path.len();
         }
 
         0
@@ -876,9 +866,11 @@ impl Request {
     }
 
     /// RFC 3986 3.2.2 `uri-host [ ":" port ]` byte set. A Host value outside it, or an empty
-    /// one, cannot form a URL authority, so `request.url` synthesis falls back to the
-    /// configured host instead of pasting the client bytes into the URL.
-    pub(crate) fn is_valid_host_header(host: &[u8]) -> bool {
+    /// one, cannot form a URL authority, so `request.url` synthesis falls back to the bare
+    /// request-target instead of pasting the client bytes into the URL. A value inside the
+    /// set can still be rejected by the URL parser (`host:abc`, bad percent-encoding); the
+    /// callers apply the same fallback in that case.
+    fn is_valid_host_header(host: &[u8]) -> bool {
         !host.is_empty()
             && host.iter().all(|&c| {
                 c.is_ascii_alphanumeric()
@@ -906,6 +898,66 @@ impl Request {
             })
     }
 
+    /// The one decision behind `request.url` on every transport: the path the request
+    /// asked for, plus the client's Host / `:authority` if it can be a URL authority.
+    /// A host is dropped when it is missing, when the path is not origin-form, or when it
+    /// contains a byte that cannot appear in an authority (`/`, `?`, `#`, `@`, `\`, ...),
+    /// so client bytes never end up in the URL anywhere but the host position.
+    fn url_parts<'a>(
+        host: Option<&'a [u8]>,
+        target: &'a [u8],
+    ) -> (Option<&'a [u8]>, Cow<'a, [u8]>) {
+        let path = Self::request_target_path(target);
+        let host = host
+            .filter(|host| Self::is_valid_host_header(host))
+            .filter(|_| path.first() == Some(&b'/'));
+        (host, path)
+    }
+
+    /// `{protocol}{host}{path}` as the URL parser canonicalizes it, or the bare path when
+    /// there is no usable host or the parser still rejects it (`host:abc`, `exa%zzmple`).
+    fn synthesize_url(protocol: &'static [u8], host: Option<&[u8]>, target: &[u8]) -> BunString {
+        let (host, path) = Self::url_parts(host, target);
+        let Some(host) = host else {
+            return BunString::clone_utf8(&path);
+        };
+
+        // Join straight into a WTF string: parsing one is a refcount bump and an
+        // already-canonical URL comes back as the same string, so this is one allocation
+        // per request regardless of target length. Joining into a scratch buffer first adds
+        // a second target-sized allocation per request (req-url-leak.test.ts catches it).
+        // The host is ASCII by construction, so only the path decides the encoding.
+        let joined = if strings::is_all_ascii(&path) {
+            let (joined, bytes) =
+                BunString::create_uninitialized_latin1(protocol.len() + host.len() + path.len());
+            let (a, rest) = bytes.split_at_mut(protocol.len());
+            let (b, c) = rest.split_at_mut(host.len());
+            a.copy_from_slice(protocol);
+            b.copy_from_slice(host);
+            c.copy_from_slice(&path);
+            joined
+        } else {
+            let mut bytes = Vec::with_capacity(protocol.len() + host.len() + path.len());
+            bytes.extend_from_slice(protocol);
+            bytes.extend_from_slice(host);
+            bytes.extend_from_slice(&path);
+            BunString::clone_utf8(&bytes)
+        };
+        let href = bun_url::href_from_string(&joined);
+        joined.deref();
+        if href.is_empty() {
+            return BunString::clone_utf8(&path);
+        }
+        href
+    }
+
+    /// For transports whose request goes away before JS can read `request.url` lazily
+    /// (HTTP/3), so it is populated up front from the same policy `ensure_url` uses.
+    pub(crate) fn set_synthesized_url(&self, host: Option<&[u8]>, target: &[u8]) {
+        self.url
+            .set(Self::synthesize_url(self.get_protocol(), host, target));
+    }
+
     pub(crate) fn ensure_url(&self) -> Result<(), AllocError> {
         if !self.url.get().is_empty() {
             return Ok(());
@@ -914,84 +966,11 @@ impl Request {
         if let Some(req) = self.request_context.get_request() {
             // S008: `uws::Request` is an `opaque_ffi!` ZST handle — safe deref.
             let req = bun_opaque::opaque_deref(req);
-            let req_url = Self::request_target_path(req.url());
-            if !req_url.is_empty() && req_url[0] == b'/' {
-                if let Some(host) = req
-                    .header(b"host")
-                    .filter(|host| Self::is_valid_host_header(host))
-                {
-                    // With `port: None`, HostFormatter always emits exactly `host`. Compute the
-                    // length and assemble the URL with straight slice copies instead of going
-                    // through `core::fmt::write` (which is not monomorphized and shows up in
-                    // per-request profiles).
-                    let protocol = self.get_protocol();
-                    let url_bytelength = protocol.len() + host.len() + req_url.len();
-
-                    debug_assert!(self.size_of_url() == url_bytelength);
-
-                    if url_bytelength < 128 {
-                        let mut buffer = [0u8; 128];
-                        let url = {
-                            let mut at = 0;
-                            buffer[at..at + protocol.len()].copy_from_slice(protocol);
-                            at += protocol.len();
-                            buffer[at..at + host.len()].copy_from_slice(host);
-                            at += host.len();
-                            buffer[at..at + req_url.len()].copy_from_slice(&req_url);
-                            at += req_url.len();
-                            &buffer[..at]
-                        };
-
-                        debug_assert!(self.size_of_url() == url.len());
-
-                        let href = bun_url::href_from_string(&BunString::from_bytes(url));
-                        if !href.is_empty() {
-                            if core::ptr::eq(href.byte_slice().as_ptr(), url.as_ptr()) {
-                                self.url.set(BunString::clone_latin1(&url[..href.length()]));
-                                href.deref();
-                            } else {
-                                self.url.set(href);
-                            }
-                        } else {
-                            // TODO: what is the right thing to do for invalid URLS?
-                            self.url.set(BunString::clone_utf8(url));
-                        }
-
-                        return Ok(());
-                    }
-
-                    if strings::is_all_ascii(host) && strings::is_all_ascii(&req_url) {
-                        let (new_url, bytes) =
-                            BunString::create_uninitialized_latin1(url_bytelength);
-                        self.url.set(new_url);
-                        // exact space was counted above
-                        let (a, rest) = bytes.split_at_mut(protocol.len());
-                        let (b, c) = rest.split_at_mut(host.len());
-                        a.copy_from_slice(protocol);
-                        b.copy_from_slice(host);
-                        c.copy_from_slice(&req_url);
-                    } else {
-                        // slow path
-                        let mut temp_url: Vec<u8> = Vec::with_capacity(url_bytelength);
-                        temp_url.extend_from_slice(protocol);
-                        temp_url.extend_from_slice(host);
-                        temp_url.extend_from_slice(&req_url);
-                        // `defer bun.default_allocator.free(temp_url)` → Vec drops at scope end
-                        self.url.set(BunString::clone_utf8(&temp_url));
-                    }
-
-                    let href = bun_url::href_from_string(&self.url.get());
-                    // TODO: what is the right thing to do for invalid URLS?
-                    if !href.is_empty() {
-                        self.url.set(href);
-                    }
-
-                    return Ok(());
-                }
-            }
-
-            debug_assert!(self.size_of_url() == req_url.len());
-            self.url.set(BunString::clone_utf8(&req_url));
+            self.url.set(Self::synthesize_url(
+                self.get_protocol(),
+                req.header(b"host"),
+                req.url(),
+            ));
         }
         Ok(())
     }

test/js/bun/http/request-smuggling.test.ts

@@ -1682,6 +1682,32 @@ describe("Host header field values in request.url", () => {
     expect(response.slice(response.indexOf("\r\n\r\n") + 4)).toBe("/index");
   });
 
+  test.each([
+    ["1.2.3.4.5"],
+    ["[::1"],
+    // The same fallback for a joined URL much longer than the other cases.
+    [`${Buffer.alloc(130, "x").toString()}.example:abc`],
+  ])(
+    "request.url is the request-target when the Host header %j is in the authority byte set but does not parse",
+    async host => {
+      await using server = Bun.serve({
+        port: 0,
+        hostname: "127.0.0.1",
+        fetch(req) {
+          return new Response(req.url);
+        },
+      });
+
+      const response = await sendRawRequest(
+        server,
+        `GET /index HTTP/1.1\r\nHost: ${host}\r\nConnection: close\r\n\r\n`,
+      );
+      expect(response).toStartWith("HTTP/1.1 200");
+      // Not `http://<host>/index`, which `new URL()` rejects.
+      expect(response.slice(response.indexOf("\r\n\r\n") + 4)).toBe("/index");
+    },
+  );
+
   test.each([
     ["example.com", "http://example.com/index"],
     ["example.com:8080", "http://example.com:8080/index"],
@@ -1754,13 +1780,15 @@ describe("Host header field values in request.url", () => {
 
       // RFC 3986 `uri-host [ ":" port ]`: unreserved / sub-delims / "%" / ":" / "[" / "]".
       // Every byte in [0x7f, 0xff] is outside that set, so neither URL uses any of them.
-      const isHostByte = (char: string) => /^[A-Za-z0-9._~%!$&'()*+,;=:\[\]-]$/.test(char);
+      // "%", ":", "[" and "]" are inside it, but `a%b`, `a:b`, `a[b` and `a]b` are not
+      // authorities the URL parser accepts, so those fall back to the request-target too.
+      const formsUrlAuthority = (char: string) => /^[A-Za-z0-9._~!$&'()*+,;=-]$/.test(char);
 
       async function checkByte(byte: number) {
         const char = String.fromCharCode(byte);
         const host = `a${char}b`;
-        // Request::is_valid_host_header decides whether the Host header becomes the
-        // request URL's authority; the request itself is served either way.
+        // Request::is_valid_host_header and then the URL parser decide whether the Host
+        // header becomes the request URL's authority; the request itself is served either way.
         // The two probes run sequentially so each batch keeps at most one socket per byte open.
         const http11 = await sendRawRequest(server, `GET /p HTTP/1.1\r\nHost: ${host}\r\nConnection: close\r\n\r\n`);
         const http10 = await sendRawRequest(server, `GET /p HTTP/1.0\r\nHost: ${host}\r\n\r\n`);
@@ -1784,7 +1812,7 @@ describe("Host header field values in request.url", () => {
         bytes.map(byte => {
           const char = String.fromCharCode(byte);
           // `req.url` carries the lowercased host (URL host normalization).
-          const url = isHostByte(char) ? `http://a${char.toLowerCase()}b/p` : "/p";
+          const url = formsUrlAuthority(char) ? `http://a${char.toLowerCase()}b/p` : "/p";
           return {
             char,
             http11Accepted: true,

test/js/bun/http/serve-http3.test.ts

@@ -53,6 +53,7 @@ const server = serve({
     "/file-route": Bun.file(process.env.BIG_FILE),
   },
   async fetch(req) {
+    if (req.headers.get("x-raw-url")) return new Response(req.url);
     const url = new URL(req.url);
     if (url.pathname === "/hello") {
       return new Response("hello over h3", {
@@ -222,6 +223,37 @@ describe("Bun.serve HTTP/3", () => {
     });
   });
 
+  // request.url is synthesized from Host + target. A Host that can't be a URL
+  // authority must not be pasted into it (HTTP/1 already refused; HTTP/3 used
+  // to produce "https://evil.example/x#/admin/secret" here), and a valid one is
+  // canonicalized the same way on both transports.
+  test("request.url applies the same Host policy as HTTP/1", async () => {
+    await withServer(async port => {
+      const viaH1 = (path: string, headers: Record<string, string>) =>
+        fetch(`https://127.0.0.1:${port}${path}`, { headers, tls: { rejectUnauthorized: false } } as RequestInit).then(
+          r => r.text(),
+        );
+      const viaH3 = (path: string, headers: Record<string, string>) =>
+        fetchH3(port, path, { headers }).then(r => r.text());
+
+      const evil = { host: "evil.example/x#", "x-raw-url": "1" };
+      expect(await viaH3("/admin/secret", evil)).toBe("/admin/secret");
+      expect(await viaH1("/admin/secret", evil)).toBe("/admin/secret");
+
+      const odd = { host: "EXAMPLE.com:443", "x-raw-url": "1" };
+      expect(await viaH3("/p", odd)).toBe("https://example.com/p");
+      expect(await viaH1("/p", odd)).toBe("https://example.com/p");
+
+      // Inside the Host byte set but still not a URL authority: falls back to
+      // the path on both transports rather than an unparsable absolute URL.
+      for (const host of ["example.com:abc", "example.com:99999", "exa%zzmple.com"]) {
+        const hdrs = { host, "x-raw-url": "1" };
+        expect(await viaH3("/p", hdrs)).toBe("/p");
+        expect(await viaH1("/p", hdrs)).toBe("/p");
+      }
+    });
+  });
+
   test("POST echoes body, status, request headers", async () => {
     await withServer(async port => {
       const body = "the quick brown fox jumps over the lazy dog";

test/js/bun/http/serve.test.ts

@@ -597,6 +597,9 @@ it("request.url should be based on the Host header", async () => {
 it.each([
   ["HTTP/1.0", "GET /helloooo HTTP/1.0\r\nHost: a/b\r\n\r\n"],
   ["HTTP/1.1", "GET /helloooo HTTP/1.1\r\nHost: a b\r\nConnection: close\r\n\r\n"],
+  // In the authority byte set, but the URL parser still rejects them.
+  ["HTTP/1.1 (bad port)", "GET /helloooo HTTP/1.1\r\nHost: example.com:abc\r\nConnection: close\r\n\r\n"],
+  ["HTTP/1.1 (bad escape)", "GET /helloooo HTTP/1.1\r\nHost: exa%zzmple.com\r\nConnection: close\r\n\r\n"],
 ])("request.url is the request-target when the %s Host header is not a valid authority", async (_version, payload) => {
   using server = Bun.serve({
     port: 0,