diff --git a/src/runtime/webcore/Request.rs b/src/runtime/webcore/Request.rs index b0789fd6011b..06a652d47472 100644 --- a/src/runtime/webcore/Request.rs +++ b/src/runtime/webcore/Request.rs @@ -3,7 +3,6 @@ use core::cell::Cell; use core::ffi::c_uint; use core::ptr::NonNull; -use std::borrow::Cow; use bun_jsc::JsCell; use enumset::EnumSet; @@ -848,7 +847,7 @@ 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()); + let req_url = req.url(); if !req_url.is_empty() && req_url[0] == b'/' { if let Some(host) = req.header(b"host") { // With `port: None`, HostFormatter always emits exactly `host`, so the @@ -872,30 +871,10 @@ impl Request { b"http://" } - fn request_target_path(target: &[u8]) -> Cow<'_, [u8]> { - let scheme_len = if strings::has_prefix_case_insensitive(target, b"https://") { - b"https://".len() - } else if strings::has_prefix_case_insensitive(target, b"http://") { - b"http://".len() - } else { - return Cow::Borrowed(target); - }; - - let path_start = strings::index_of_char_pos(target, b'/', scheme_len); - let query_start = strings::index_of_char_pos(target, b'?', scheme_len); - match (path_start, query_start) { - (Some(path_start), None) => Cow::Borrowed(&target[path_start..]), - (Some(path_start), Some(query_start)) if path_start < query_start => { - Cow::Borrowed(&target[path_start..]) - } - (_, Some(query_start)) => { - let mut path = Vec::with_capacity(1 + target.len() - query_start); - path.push(b'/'); - path.extend_from_slice(&target[query_start..]); - Cow::Owned(path) - } - _ => Cow::Borrowed(b"/"), - } + /// RFC 9112 section 3.2.2: an `http(s)://` absolute-form request-target. + fn is_absolute_form_target(target: &[u8]) -> bool { + strings::has_prefix_case_insensitive(target, b"http://") + || strings::has_prefix_case_insensitive(target, b"https://") } pub fn ensure_url(&self) -> Result<(), AllocError> { @@ -906,7 +885,7 @@ 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()); + let req_url = req.url(); if !req_url.is_empty() && req_url[0] == b'/' { if let Some(host) = req.header(b"host") { // With `port: None`, HostFormatter always emits exactly `host`. Compute the @@ -927,7 +906,7 @@ impl Request { 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); + buffer[at..at + req_url.len()].copy_from_slice(req_url); at += req_url.len(); &buffer[..at] }; @@ -951,7 +930,7 @@ impl Request { return Ok(()); } - if strings::is_all_ascii(host) && strings::is_all_ascii(&req_url) { + 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); @@ -960,13 +939,13 @@ impl Request { 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); + c.copy_from_slice(req_url); } else { // slow path let mut temp_url: Vec = Vec::with_capacity(url_bytelength); temp_url.extend_from_slice(protocol); temp_url.extend_from_slice(host); - temp_url.extend_from_slice(&req_url); + 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)); } @@ -983,7 +962,16 @@ impl Request { #[cfg(debug_assertions)] debug_assert!(self.size_of_url() == req_url.len()); - self.url.set(BunString::clone_utf8(&req_url)); + self.url.set(BunString::clone_utf8(req_url)); + + // Absolute-form targets carry their own authority: the request-target is + // the target URI and the Host field is ignored (RFC 9112 section 3.2.2). + if Self::is_absolute_form_target(req_url) { + let href = bun_url::href_from_string(&self.url.get()); + if !href.is_empty() { + self.url.set(href); + } + } } Ok(()) } diff --git a/test/js/bun/http/bun-serve-routes.test.ts b/test/js/bun/http/bun-serve-routes.test.ts index 79e0185e7b8f..cde185b633e0 100644 --- a/test/js/bun/http/bun-serve-routes.test.ts +++ b/test/js/bun/http/bun-serve-routes.test.ts @@ -870,75 +870,32 @@ it("route precedence for mix of method-specific routes and any routes", async () ); }); -it("routes absolute-form request targets by path and derives request.url from the Host header", async () => { - const seen: { matched: string; url: string }[] = []; +// RFC 9112 section 3.2.2: an absolute-form request-target carries its own authority, +// so request.url is the request-target itself and the Host field is ignored for it. +// Routing still matches on the target's path. +it("routes absolute-form request targets by path and keeps the target authority in request.url", async () => { + const seen: { matched: string; url: string; host: string | null }[] = []; await using server = Bun.serve({ port: 0, hostname: "127.0.0.1", routes: { "/admin/secret": req => { - seen.push({ matched: "route", url: req.url }); + seen.push({ matched: "route", url: req.url, host: req.headers.get("host") }); return new Response("named route"); }, }, fetch(req) { - seen.push({ matched: "fallback", url: req.url }); + seen.push({ matched: "fallback", url: req.url, host: req.headers.get("host") }); return new Response("fallback"); }, }); const hostHeader = `127.0.0.1:${server.port}`; - // Send an absolute-form request-target (RFC 9112 ยง3.2.2) over a raw socket; - // fetch() always uses origin-form so we have to write the request line ourselves. - const responseText = await new Promise((resolve, reject) => { - let received = ""; - Bun.connect({ - hostname: "127.0.0.1", - port: server.port, - socket: { - open(socket) { - socket.write( - `GET https://spoofed.example/admin/secret HTTP/1.1\r\nHost: ${hostHeader}\r\nConnection: close\r\n\r\n`, - ); - }, - data(socket, chunk) { - received += chunk.toString(); - }, - close() { - resolve(received); - }, - error(socket, err) { - reject(err); - }, - }, - }).catch(reject); - }); - - // The named route handles the request, not the catch-all fetch handler. - expect(responseText).toContain("named route"); - expect(responseText).toContain("200"); - expect(seen).toHaveLength(1); - expect(seen[0].matched).toBe("route"); - - // request.url is derived from the Host header, not from the authority in the request line. - expect(seen[0].url).not.toContain("spoofed.example"); - const url = new URL(seen[0].url); - expect(url.protocol).toBe("http:"); - expect(url.host).toBe(hostHeader); - expect(url.pathname).toBe("/admin/secret"); - - // A normal origin-form request still hits the same named route. - seen.length = 0; - const res = await fetch(new URL("/admin/secret", server.url)); - expect(await res.text()).toBe("named route"); - expect(seen).toHaveLength(1); - expect(seen[0].matched).toBe("route"); - expect(new URL(seen[0].url).pathname).toBe("/admin/secret"); - - for (const target of ["http://spoofed.example?a=b", "http://spoofed.example?redirect=/elsewhere"]) { - seen.length = 0; - const rawResponse = await new Promise((resolve, reject) => { + // fetch() always uses origin-form, so the absolute-form request line has to be + // written over a raw socket. + const requestWithTarget = (target: string) => + new Promise((resolve, reject) => { let received = ""; Bun.connect({ hostname: "127.0.0.1", @@ -960,13 +917,36 @@ it("routes absolute-form request targets by path and derives request.url from th }).catch(reject); }); + const responseText = await requestWithTarget("https://target.example/admin/secret"); + + // The named route handles the request, not the catch-all fetch handler. + expect(responseText).toContain("named route"); + expect(responseText).toContain("200"); + + // request.url is the request-target; the Host field is still visible in headers, + // so the handler can detect the mismatch. + expect(seen).toEqual([{ matched: "route", url: "https://target.example/admin/secret", host: hostHeader }]); + + // The usual proxy-issued shape: target authority and Host agree. + seen.length = 0; + await requestWithTarget(`http://${hostHeader}/admin/secret`); + expect(seen).toEqual([{ matched: "route", url: `http://${hostHeader}/admin/secret`, host: hostHeader }]); + + // A normal origin-form request still hits the same named route, with the + // authority taken from the Host header. + seen.length = 0; + const res = await fetch(new URL("/admin/secret", server.url)); + expect(await res.text()).toBe("named route"); + expect(seen).toEqual([{ matched: "route", url: `http://${hostHeader}/admin/secret`, host: hostHeader }]); + + // Absolute-form targets without a path are routed as "/" and normalized. + for (const target of ["http://target.example?a=b", "http://target.example?redirect=/elsewhere"]) { + seen.length = 0; + const rawResponse = await requestWithTarget(target); + expect(rawResponse).toContain("fallback"); - expect(seen).toHaveLength(1); - expect(seen[0].matched).toBe("fallback"); - expect(seen[0].url).not.toContain("spoofed.example"); - const rawUrl = new URL(seen[0].url); - expect(rawUrl.host).toBe(hostHeader); - expect(rawUrl.pathname).toBe("/"); - expect(rawUrl.search).toBe(new URL(target).search); + expect(seen).toEqual([ + { matched: "fallback", url: `http://target.example/${new URL(target).search}`, host: hostHeader }, + ]); } });