webcore: copy internal state in new Request(request) instead of calling getters

src/runtime/webcore/Request.rs

@@ -18,7 +18,7 @@ use crate::webcore::jsc::{
 };
 use crate::webcore::{AbortSignal, Blob, CookieMap, FetchHeaders, ReadableStream, Response};
 use bun_alloc::AllocError;
-use bun_core::{Output, fmt as bun_fmt};
+use bun_core::{Output, UnwrapOrOom, fmt as bun_fmt};
 use bun_core::{OwnedStringCell, String as BunString, ZigString, strings};
 use bun_http_jsc::fetch_enums_jsc::{
     fetch_cache_mode_to_js, fetch_redirect_to_js, fetch_request_mode_to_js,
@@ -1121,15 +1121,42 @@ impl Request {
         let values_to_try = &values_to_try_[0..((!is_first_argument_a_url) as usize
             + (arguments.len() > 1 && arguments[1].is_object()) as usize)];
 
-        for &value in values_to_try {
+        for (value_index, &value) in values_to_try.iter().enumerate() {
             let value_type = value.js_type();
+            // The last entry is the `input` argument when it was an object
+            // rather than a URL; other entries are `init` dictionaries.
+            let is_input_argument =
+                !is_first_argument_a_url && value_index == values_to_try.len() - 1;
             let explicit_check = values_to_try.len() == 2
                 && value_type == bun_jsc::JSType::FinalObject
                 && values_to_try[1].js_type() == bun_jsc::JSType::DOMWrapper;
             if value_type == bun_jsc::JSType::DOMWrapper {
-                if let Some(request) = value.as_direct::<Request>() {
-                    // SAFETY: as_direct returns a live *mut Request payload (m_ctx)
+                // Fetch spec: a Request `input` is copied from internal state,
+                // never via JS getters, so subclasses and shadowed instances
+                // (rejected by `as_direct`'s pristine-structure check) must
+                // match too. A Request `init` keeps dictionary semantics.
+                let request_ptr = if is_input_argument {
+                    value.as_::<Request>()
+                } else {
+                    value.as_direct::<Request>()
+                };
+                if let Some(request) = request_ptr {
+                    // SAFETY: the cast returns a live *mut Request payload (m_ctx)
                     let request = unsafe { &*request };
+                    // Fetch spec: `init.body: null`/absent contributes no
+                    // body, so the input's body applies and must be usable.
+                    let input_body_applies = is_input_argument
+                        && (!fields.contains(Fields::Body)
+                            || matches!(req.body_value(), BodyValue::Null));
+                    if input_body_applies
+                        && request.body_stream_check(global_this, |s, g| {
+                            s.is_disturbed(g) || s.is_locked(g)
+                        })
+                    {
+                        bail!(Err(global_this.throw_type_error(format_args!(
+                            "Cannot construct a Request with a Request object that has already been used."
+                        ))));
+                    }
                     if values_to_try.len() == 1 {
                         match Request::clone_into(
                             request,
@@ -1180,9 +1207,13 @@ impl Request {
                         }
                     }
 
-                    if !fields.contains(Fields::Body) {
+                    if !fields.contains(Fields::Body) || input_body_applies {
                         match request.body_value() {
-                            BodyValue::Null | BodyValue::Empty | BodyValue::Used => {}
+                            BodyValue::Null | BodyValue::Used => {}
+                            // `Empty` is a non-null body; an input copy keeps
+                            // it, while an `init` entry leaves it to the
+                            // dictionary fallback.
+                            BodyValue::Empty if !is_input_argument => {}
                             _ => {
                                 match request.clone_body_value_via_cached_stream(global_this) {
                                     Ok(v) => {
@@ -1194,6 +1225,31 @@ impl Request {
                             }
                         }
                     }
+
+                    if is_input_argument {
+                        // Mark every remaining field consumed so the dictionary
+                        // fallbacks below never run JS getters against the input.
+                        if !fields.contains(Fields::Url) {
+                            // A Bun.serve request materializes its URL lazily;
+                            // a detached one stays empty and reaches the "url
+                            // is required" throw, with the field consumed.
+                            request.ensure_url().unwrap_or_oom();
+                            let url = request.url.get();
+                            if !url.is_empty() {
+                                req.url.set(url.dupe_ref());
+                            }
+                            fields.insert(Fields::Url);
+                        }
+                        if !fields.contains(Fields::Signal) {
+                            if let Some(signal) = request.signal.get() {
+                                // `AbortSignalRef::clone` is a C++ `ref()`.
+                                req.signal.set(Some(signal.clone()));
+                            }
+                            fields.insert(Fields::Signal);
+                        }
+                        fields.insert(Fields::Headers);
+                        fields.insert(Fields::Body);
+                    }
                 }
 
                 if let Some(response) = value.as_direct::<Response>() {
@@ -1552,7 +1608,7 @@ impl Request {
         preserve_url: bool,
     ) -> JsResult<()> {
         // allocator param dropped (global mimalloc)
-        let _ = self.ensure_url();
+        self.ensure_url().unwrap_or_oom();
         let body_ = self.clone_body_value_via_cached_stream(global_this)?;
         // BodyValue's Drop frees `body_` on the `?` error path
         let body = body::hive_alloc(body_);

test/js/web/fetch/body-clone.test.ts

@@ -630,18 +630,18 @@ test.each(["Request", "Response"])(
   },
 );
 
-// clone()'s usability check now fires before the stream is teed, so the
-// readableStreamTee C++ bridge's exception propagation (which used to be
-// covered by the test above) is exercised via `new Request(lockedRequest)`,
-// which still tees. It must throw a single catchable TypeError, not also
-// report it as uncaught (exit code 1) or surface a bogus follow-up error.
-test("new Request(request) with a locked stream body throws a catchable TypeError from the tee and does not fail the process", async () => {
+// The usability checks in clone() and in Request(input) fire before the
+// stream is teed, so the readableStreamTee C++ bridge's exception propagation
+// is exercised via a locked Request passed as init (second argument), which
+// still tees. It must throw a single catchable TypeError, not also report it
+// as uncaught (exit code 1) or surface a bogus follow-up error.
+test("new Request(url, lockedRequest) throws a catchable TypeError from the tee and does not fail the process", async () => {
   const script = `
     const stream = new ReadableStream({ start() {} });
     const source = new Request("http://example.com/", { method: "POST", body: stream, duplex: "half" });
     source.body.getReader(); // lock the body stream
     try {
-      new Request(source);
+      new Request("http://other.example/", source);
       console.log("no throw");
     } catch (e) {
       console.log("caught " + e.constructor.name + ": " + e.message);