Create Client.Transfer earlier.

On client.request(req) we now immediately wrap the request into a Transfer. This
results in less copying of the Request object. It also makes the transfer.uri
available, so CDP no longer needs to std.Uri(request.url) anymore.

The main advantage is that it's easier to manage resources. There was a use-
after free before due to the sensitive nature of the tranfer's lifetime. There
were also corner cases where some resources might not be freed. This is
hopefully fixed with the lifetime of Transfer being extended.

Author: karlseguin

src/cdp/domains/fetch.zig

@@ -20,8 +20,8 @@ const std = @import("std");
 const Allocator = std.mem.Allocator;
 const Notification = @import("../../notification.zig").Notification;
 const log = @import("../../log.zig");
-const Request = @import("../../http/Client.zig").Request;
 const Method = @import("../../http/Client.zig").Method;
+const Transfer = @import("../../http/Client.zig").Transfer;
 
 pub fn processMessage(cmd: anytype) !void {
     const action = std.meta.stringToEnum(enum {
@@ -42,11 +42,11 @@ pub fn processMessage(cmd: anytype) !void {
 // Stored in CDP
 pub const InterceptState = struct {
     const Self = @This();
-    waiting: std.AutoArrayHashMap(u64, Request),
+    waiting: std.AutoArrayHashMap(u64, *Transfer),
 
     pub fn init(allocator: Allocator) !InterceptState {
         return .{
-            .waiting = std.AutoArrayHashMap(u64, Request).init(allocator),
+            .waiting = std.AutoArrayHashMap(u64, *Transfer).init(allocator),
         };
     }
 
@@ -135,10 +135,11 @@ pub fn requestPaused(arena: Allocator, bc: anytype, intercept: *const Notificati
     // NOTE: we assume whomever created the request created it with a lifetime of the Page.
     // TODO: What to do when receiving replies for a previous page's requests?
 
-    try cdp.intercept_state.waiting.put(intercept.request.id.?, intercept.request.*);
+    const transfer = intercept.transfer;
+    try cdp.intercept_state.waiting.put(transfer.id, transfer);
 
     // NOTE: .request data preparation is duped from network.zig
-    const full_request_url = try std.Uri.parse(intercept.request.url);
+    const full_request_url = transfer.uri;
     const request_url = try @import("network.zig").urlToString(arena, &full_request_url, .{
         .scheme = true,
         .authentication = true,
@@ -149,21 +150,21 @@ pub fn requestPaused(arena: Allocator, bc: anytype, intercept: *const Notificati
     const request_fragment = try @import("network.zig").urlToString(arena, &full_request_url, .{
         .fragment = true,
     });
-    const headers = try intercept.request.headers.asHashMap(arena);
+    const headers = try transfer.req.headers.asHashMap(arena);
     // End of duped code
 
     try cdp.sendEvent("Fetch.requestPaused", .{
-        .requestId = try std.fmt.allocPrint(arena, "INTERCEPT-{d}", .{intercept.request.id.?}),
+        .requestId = try std.fmt.allocPrint(arena, "INTERCEPT-{d}", .{transfer.id}),
         .request = .{
             .url = request_url,
             .urlFragment = request_fragment,
-            .method = @tagName(intercept.request.method),
-            .hasPostData = intercept.request.body != null,
+            .method = @tagName(transfer.req.method),
+            .hasPostData = transfer.req.body != null,
             .headers = std.json.ArrayHashMap([]const u8){ .map = headers },
         },
         .frameId = target_id,
         .resourceType = ResourceType.Document, //  TODO!
-        .networkId = try std.fmt.allocPrint(arena, "REQ-{d}", .{intercept.request.id.?}),
+        .networkId = try std.fmt.allocPrint(arena, "REQ-{d}", .{transfer.id}),
     }, .{ .session_id = session_id });
 
     // Await either continueRequest, failRequest or fulfillRequest
@@ -188,19 +189,20 @@ fn continueRequest(cmd: anytype) !void {
     if (params.postData != null or params.headers != null or params.interceptResponse) return error.NotYetImplementedParams;
 
     const request_id = try idFromRequestId(params.requestId);
-    var waiting_request = (bc.cdp.intercept_state.waiting.fetchSwapRemove(request_id) orelse return error.RequestNotFound).value;
+    const entry = bc.cdp.intercept_state.waiting.fetchSwapRemove(request_id) orelse return error.RequestNotFound;
+    const transfer = entry.value;
 
     // Update the request with the new parameters
     if (params.url) |url| {
         // The request url must be modified in a way that's not observable by page. So page.url is not updated.
-        waiting_request.url = try bc.cdp.browser.page_arena.allocator().dupeZ(u8, url);
+        try transfer.updateURL(try bc.cdp.browser.page_arena.allocator().dupeZ(u8, url));
     }
     if (params.method) |method| {
-        waiting_request.method = std.meta.stringToEnum(Method, method) orelse return error.InvalidParams;
+        transfer.req.method = std.meta.stringToEnum(Method, method) orelse return error.InvalidParams;
     }
 
     log.info(.cdp, "Request continued by intercept", .{ .id = params.requestId });
-    try bc.cdp.browser.http_client.request(waiting_request);
+    try bc.cdp.browser.http_client.process(transfer);
 
     return cmd.sendResult(null, .{});
 }
@@ -214,7 +216,9 @@ fn failRequest(cmd: anytype) !void {
     })) orelse return error.InvalidParams;
 
     const request_id = try idFromRequestId(params.requestId);
-    if (state.waiting.fetchSwapRemove(request_id) == null) return error.RequestNotFound;
+    const entry = state.waiting.fetchSwapRemove(request_id) orelse return error.RequestNotFound;
+    // entry.value is the transfer
+    entry.value.abort();
 
     log.info(.cdp, "Request aborted by intercept", .{ .reason = params.errorReason });
     return cmd.sendResult(null, .{});

src/cdp/domains/network.zig

@@ -215,7 +215,7 @@ pub fn httpRequestFail(arena: Allocator, bc: anytype, data: *const Notification.
 
     // We're missing a bunch of fields, but, for now, this seems like enough
     try bc.cdp.sendEvent("Network.loadingFailed", .{
-        .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{data.request.id.?}),
+        .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{data.transfer.id}),
         // Seems to be what chrome answers with. I assume it depends on the type of error?
         .type = "Ping",
         .errorText = data.err,
@@ -251,7 +251,8 @@ pub fn httpRequestStart(arena: Allocator, bc: anytype, data: *const Notification
         .query = true,
     });
 
-    const full_request_url = try std.Uri.parse(data.request.url);
+    const transfer = data.transfer;
+    const full_request_url = transfer.uri;
     const request_url = try urlToString(arena, &full_request_url, .{
         .scheme = true,
         .authentication = true,
@@ -263,19 +264,19 @@ pub fn httpRequestStart(arena: Allocator, bc: anytype, data: *const Notification
         .fragment = true, // TODO since path is false, this likely does not work as intended
     });
 
-    const headers = try data.request.headers.asHashMap(arena);
+    const headers = try transfer.req.headers.asHashMap(arena);
 
     // We're missing a bunch of fields, but, for now, this seems like enough
     try cdp.sendEvent("Network.requestWillBeSent", .{
-        .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{data.request.id.?}),
+        .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{transfer.id}),
         .frameId = target_id,
         .loaderId = bc.loader_id,
         .documentUrl = document_url,
         .request = .{
             .url = request_url,
             .urlFragment = request_fragment,
-            .method = @tagName(data.request.method),
-            .hasPostData = data.request.body != null,
+            .method = @tagName(transfer.req.method),
+            .hasPostData = transfer.req.body != null,
             .headers = std.json.ArrayHashMap([]const u8){ .map = headers },
         },
     }, .{ .session_id = session_id });