From 8a76153cbb1a18d2e97921d4e6706ededbc3c903 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Mon, 1 Jun 2026 21:20:44 +0800
Subject: [PATCH] Clone data into the transfer's arena

There can be cases where the data referenced by Request does not live for the
duration of a Transfer. The most obvious case is a QueuedNavigation..this is
generally a problem, but it's particularly problematic under load when the
request gets queued in the HttpClient.

Most of the data is small, so just _always_ duping it is worth it. The body
can be large, so we provide a flag for callers to set to turn off the body
duping (essentially saying "I promise to keep the body alive for the duration
of the transfer). Currently, only XHR is able to make this guarantee.
---
 src/browser/HttpClient.zig                | 64 +++++++++++++++++------
 src/browser/webapi/net/XMLHttpRequest.zig |  1 +
 2 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/src/browser/HttpClient.zig b/src/browser/HttpClient.zig
index 6eb3611e..721ac964 100644
--- a/src/browser/HttpClient.zig
+++ b/src/browser/HttpClient.zig
@@ -524,24 +524,50 @@ fn requestT(self: *Client, req: Request, owner: ?*Owner) !*Transfer {
         return err;
     };
 
-    const transfer = arena.create(Transfer) catch |err| {
-        req.headers.deinit();
-        self.arena_pool.release(arena);
-        return err;
-    };
+    const transfer = blk: {
+        errdefer {
+            req.headers.deinit();
+            self.arena_pool.release(arena);
+        }
 
-    transfer.* = .{
-        .req = req,
-        .client = self,
-        .arena = arena,
-        .id = self.incrReqId(),
-        .start_time = timestamp(.monotonic),
-        // owner is set AFTER we've actually appended to the owner list,
-        // so transfer.deinit's `if (self.owner)` branch only fires when
-        // we're truly linked. Otherwise we'd try to remove a node from
-        // a list it was never in.
-        .owner = null,
-        .owner_node = .{},
+        var owned = req;
+        // Most of the time, the req data will outlive the transfer. But not
+        // always. The most problematic case is with a QueuedNavigation which
+        // is freed quite quickly and would definetly not survive a queued
+        // request.
+        //
+        // These are all small, so duping them into the transfer's arena is
+        // cheap and can solve some nasty UAF.
+        owned.url = try arena.dupeZ(u8, req.url);
+        owned.cookie_origin = try arena.dupeZ(u8, req.cookie_origin);
+        if (req.credentials) |c| {
+            owned.credentials = try arena.dupeZ(u8, c);
+        }
+
+        // The body can be larger, so callers can signal, via the
+        // `body_outlives_request` flag that they guarantee that the body
+        // will outlive the transfer (and thus doesn't need to be duped)
+        if (req.body) |b| {
+            if (req.body_outlives_request == false) {
+                owned.body = try arena.dupe(u8, b);
+            }
+        }
+
+        const t = try arena.create(Transfer);
+        t.* = .{
+            .req = owned,
+            .client = self,
+            .arena = arena,
+            .id = self.incrReqId(),
+            .start_time = timestamp(.monotonic),
+            // owner is set AFTER we've actually appended to the owner list,
+            // so transfer.deinit's `if (self.owner)` branch only fires when
+            // we're truly linked. Otherwise we'd try to remove a node from
+            // a list it was never in.
+            .owner = null,
+            .owner_node = .{},
+        };
+        break :blk t;
     };
 
     // From here, transfer owns req+arena. Any subsequent failure flows
@@ -1229,6 +1255,10 @@ pub const Request = struct {
     timeout_ms: u32 = 0,
     skip_robots: bool = false,
 
+    // When false, the caller does not guarantee that the body outlives the
+    // transfer, and thus we'll need to dupe it.
+    body_outlives_request: bool = false,
+
     // arbitrary data that can be associated with this request
     ctx: *anyopaque = undefined,
 
diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig
index 9dd5de4b..639e64b1 100644
--- a/src/browser/webapi/net/XMLHttpRequest.zig
+++ b/src/browser/webapi/net/XMLHttpRequest.zig
@@ -283,6 +283,7 @@ pub fn send(self: *XMLHttpRequest, body_: ?BodyInit, exec_: *const Execution) !v
         .done_callback = httpDoneCallback,
         .error_callback = httpErrorCallback,
         .shutdown_callback = httpShutdownCallback,
+        .body_outlives_request = true,
     }) catch |err| {
         self.releaseSelfRef();
         return err;