support CDP proxy override

src/cdp/cdp.zig

@@ -338,10 +338,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
         inspector: Inspector,
         isolated_world: ?IsolatedWorld,
 
-        // Used to restore the proxy after the CDP session ends. If CDP never over-wrote it, it won't restore it (the first null).
-        // If the CDP is restoring it, but the original value was null, that's the 2nd null.
-        // If you only have 1 null it would be ambiguous, does null mean it shouldn't be restored, or should it be restored to null?
-        http_proxy_before: ??std.Uri = null,
+        http_proxy_changed: bool = false,
 
         const Self = @This();
 
@@ -397,7 +394,13 @@ pub fn BrowserContext(comptime CDP_T: type) type {
             self.node_search_list.deinit();
             self.cdp.browser.notification.unregisterAll(self);
 
-            if (self.http_proxy_before) |prev_proxy| self.cdp.browser.http_client.http_proxy = prev_proxy;
+            if (self.http_proxy_changed) {
+                // has to be called after browser.closeSession, since it won't
+                // work if there are active connections.
+                self.cdp.browser.http_client.restoreOriginalProxy() catch |err| {
+                    log.warn(.http, "restoreOriginalProxy", .{ .err = err });
+                };
+            }
         }
 
         pub fn reset(self: *Self) void {

src/cdp/domains/target.zig

@@ -68,7 +68,7 @@ fn getBrowserContexts(cmd: anytype) !void {
 fn createBrowserContext(cmd: anytype) !void {
     const params = try cmd.params(struct {
         disposeOnDetach: bool = false,
-        proxyServer: ?[]const u8 = null,
+        proxyServer: ?[:0]const u8 = null,
         proxyBypassList: ?[]const u8 = null,
         originsWithUniversalNetworkAccess: ?[]const []const u8 = null,
     });
@@ -84,9 +84,8 @@ fn createBrowserContext(cmd: anytype) !void {
     if (params) |p| {
         if (p.proxyServer) |proxy| {
             // For now the http client is not in the browser context so we assume there is just 1.
-            bc.http_proxy_before = cmd.cdp.browser.http_client.http_proxy;
-            const proxy_cp = try cmd.cdp.browser.http_client.allocator.dupe(u8, proxy);
-            cmd.cdp.browser.http_client.http_proxy = try std.Uri.parse(proxy_cp);
+            try cmd.cdp.browser.http_client.changeProxy(proxy);
+            bc.http_proxy_changed = true;
         }
     }
 

src/http/Client.zig

@@ -71,9 +71,6 @@ allocator: Allocator,
 // request. These wil come and go with each request.
 transfer_pool: std.heap.MemoryPool(Transfer),
 
-//@newhttp
-http_proxy: ?std.Uri = null,
-
 // see ScriptManager.blockingGet
 blocking: Handle,
 
@@ -87,6 +84,10 @@ blocking_active: if (builtin.mode == .Debug) bool else void = if (builtin.mode =
 // can result in makeRequest being re-called (from a doneCallback).
 arena: ArenaAllocator,
 
+// only needed for CDP which can change the proxy and then restore it. When
+// restoring, this originally-configured value is what it goes to.
+http_proxy: ?[:0]const u8 = null,
+
 const RequestQueue = std.DoublyLinkedList(Request);
 
 pub fn init(allocator: Allocator, ca_blob: ?c.curl_blob, opts: Http.Opts) !*Client {
@@ -117,6 +118,7 @@ pub fn init(allocator: Allocator, ca_blob: ?c.curl_blob, opts: Http.Opts) !*Clie
         .handles = handles,
         .blocking = blocking,
         .allocator = allocator,
+        .http_proxy = opts.http_proxy,
         .transfer_pool = transfer_pool,
         .queue_node_pool = queue_node_pool,
         .arena = ArenaAllocator.init(allocator),
@@ -208,6 +210,36 @@ pub fn blockingRequest(self: *Client, req: Request) !void {
     return self.makeRequest(&self.blocking, req);
 }
 
+// Restrictive since it'll only work if there are no inflight requests. In some
+// cases, the libcurl documentation is clear that changing settings while a
+// connection is inflight is undefined. It doesn't say anything about CURLOPT_PROXY,
+// but better to be safe than sorry.
+// For now, this restriction is ok, since it's only called by CDP on
+// createBrowserContext, at which point, if we do have an active connection,
+// that's probably a bug (a previous abort failed?). But if we need to call this
+// at any point in time, it could be worth digging into libcurl to see if this
+// can be changed at any point in the easy's lifecycle.
+pub fn changeProxy(self: *Client, proxy: [:0]const u8) !void {
+    try self.ensureNoActiveConnection();
+
+    for (self.handles.handles) |h| {
+        try errorCheck(c.curl_easy_setopt(h.conn.easy, c.CURLOPT_PROXY, proxy.ptr));
+    }
+    try errorCheck(c.curl_easy_setopt(self.blocking.conn.easy, c.CURLOPT_PROXY, proxy.ptr));
+}
+
+// Same restriction as changeProxy. Should be ok since this is only called on
+// BrowserContext deinit.
+pub fn restoreOriginalProxy(self: *Client) !void {
+    try self.ensureNoActiveConnection();
+
+    const proxy = if (self.http_proxy) |p| p.ptr else null;
+    for (self.handles.handles) |h| {
+        try errorCheck(c.curl_easy_setopt(h.conn.easy, c.CURLOPT_PROXY, proxy));
+    }
+    try errorCheck(c.curl_easy_setopt(self.blocking.conn.easy, c.CURLOPT_PROXY, proxy));
+}
+
 fn makeRequest(self: *Client, handle: *Handle, req: Request) !void {
     const conn = handle.conn;
     const easy = conn.easy;
@@ -339,6 +371,17 @@ fn endTransfer(self: *Client, transfer: *Transfer) void {
     self.active -= 1;
 }
 
+fn ensureNoActiveConnection(self: *const Client) !void {
+    if (self.active > 0) {
+        return error.InflightConnection;
+    }
+    if (comptime builtin.mode == .Debug) {
+        if (self.blocking_active) {
+            return error.InflightConnection;
+        }
+    }
+}
+
 const Handles = struct {
     handles: []Handle,
     in_use: HandleList,

src/main.zig

@@ -768,7 +768,7 @@ fn serveCDP(address: std.net.Address, platform: *const Platform) !void {
         .run_mode = .serve,
         .tls_verify_host = false,
         .platform = platform,
-        .max_concurrent_transfers = 2,
+        .http_max_concurrent = 2,
     });
     defer app.deinit();