From ed802c0404a1a6c5e44b3204a95abae7041f4311 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Thu, 12 Feb 2026 11:29:47 +0800
Subject: [PATCH] Remove potential recursive abort call in curl

Curl doesn't like recursive calls. For example, you can't call
curl_multi_remove_handle from within a dataCallback.

This specifically means that, as-is, transfer.abort() calls aren't safe to be
called during a libcurl callback. Consider this code:

```
req.open('GET', 'http://127.0.0.1:9582/xhr');
req.onreadystatechange = (e) => {
  req.abort();
}
req.send();
```

onreadystatechange is triggered by network events, i.e. it executes in libcurl
callback. Thus, the above code fails to truly "abort" the request with
`curl_multi_remove_handle` error, saying it's a recursive call.

To solve this, transfer.abort() now sets an `aborted = true` flag. Callbacks can
now use this flag to signal to libcurl to stop the transfer.

A test was added which reproduced this issue, but this comes from:
https://github.com/lightpanda-io/browser/issues/1527  which I wasn't able to
reliably reproduce. I did see it happen regularly, just not always. It seems
like this commit fixes that issue.
---
 src/browser/tests/net/xhr.html | 31 +++++++++++++++++++++++++++++++
 src/http/Client.zig            | 22 ++++++++++++++++++----
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/src/browser/tests/net/xhr.html b/src/browser/tests/net/xhr.html
index b7132f6e..61cfb1ee 100644
--- a/src/browser/tests/net/xhr.html
+++ b/src/browser/tests/net/xhr.html
@@ -222,3 +222,34 @@
     testing.expectEqual(XMLHttpRequest.UNSENT, req.readyState);
   });
 </script>
+
+<script id=xhr_abort_callback>
+  testing.async(async (restore) => {
+    const req = new XMLHttpRequest();
+    let abortFired = false;
+    let errorFired = false;
+    let loadEndFired = false;
+
+    await new Promise((resolve) => {
+      req.onabort = () => { abortFired = true; };
+      req.onerror = () => { errorFired = true; };
+      req.onloadend = () => {
+        loadEndFired = true;
+        resolve();
+      };
+
+      req.open('GET', 'http://127.0.0.1:9582/xhr');
+      req.onreadystatechange = (e) => {
+        req.abort();
+      }
+      req.send();
+    });
+
+    restore();
+    testing.expectEqual(true, abortFired);
+    testing.expectEqual(true, errorFired);
+    testing.expectEqual(true, loadEndFired);
+    testing.expectEqual(XMLHttpRequest.UNSENT, req.readyState);
+  });
+</script>
+ -->
diff --git a/src/http/Client.zig b/src/http/Client.zig
index 3cce2ddb..4072b548 100644
--- a/src/http/Client.zig
+++ b/src/http/Client.zig
@@ -1092,6 +1092,8 @@ pub const Transfer = struct {
     // the headers, and the [encoded] body.
     bytes_received: usize = 0,
 
+    aborted: bool = false,
+
     max_response_size: ?usize = null,
 
     // We'll store the response header here
@@ -1224,10 +1226,18 @@ pub const Transfer = struct {
 
     pub fn abort(self: *Transfer, err: anyerror) void {
         requestFailed(self, err, true);
-        if (self._handle != null) {
-            self.client.endTransfer(self);
+        if (self._handle == null) {
+            self.deinit();
+            return;
         }
-        self.deinit();
+
+        // abort can be called from a libcurl callback, e.g. we get data, we
+        // do the header done callback, the client aborts.
+        // libcurl doesn't support re-entrant calls. We can't remove the
+        // handle from the multi during a callback. Instead, we flag this as
+        // aborted, which will signal the callbacks to stop processing the
+        // transfer
+        self.aborted = true;
     }
 
     pub fn terminate(self: *Transfer) void {
@@ -1359,7 +1369,7 @@ pub const Transfer = struct {
             .transfer = transfer,
         });
 
-        return proceed;
+        return proceed and transfer.aborted == false;
     }
 
     // headerCallback is called by curl on each request's header line read.
@@ -1519,6 +1529,10 @@ pub const Transfer = struct {
             .transfer = transfer,
         });
 
+        if (transfer.aborted) {
+            return -1;
+        }
+
         return @intCast(chunk_len);
     }