From 0d50f706dbddc40eae912e1e5d75e4025b1251e5 Mon Sep 17 00:00:00 2001 From: Muki Kiboigo Date: Sun, 26 Apr 2026 19:56:13 -0700 Subject: [PATCH] more fixing of hanging in cdp interception --- src/Notification.zig | 2 + src/browser/HttpClient.zig | 39 ++++++----- src/browser/Runner.zig | 4 +- src/cdp/domains/fetch.zig | 87 ++++++++++++------------- src/cdp/domains/page.zig | 2 +- src/network/layer/InterceptionLayer.zig | 56 ++++++++++++++++ 6 files changed, 121 insertions(+), 69 deletions(-) diff --git a/src/Notification.zig b/src/Notification.zig index e5b7c1c4a..96b8f8229 100644 --- a/src/Notification.zig +++ b/src/Notification.zig @@ -23,6 +23,7 @@ const Frame = @import("browser/Frame.zig"); const Transfer = @import("browser/HttpClient.zig").Transfer; const Request = @import("browser/HttpClient.zig").Request; const Response = @import("browser/HttpClient.zig").Response; +const InterceptContext = @import("network/layer/InterceptionLayer.zig").InterceptContext; const log = lp.log; const List = std.DoublyLinkedList; @@ -174,6 +175,7 @@ pub const RequestIntercept = struct { pub const RequestAuthRequired = struct { request: *Request, + intercept_ctx: *InterceptContext, wait_for_interception: *bool, }; diff --git a/src/browser/HttpClient.zig b/src/browser/HttpClient.zig index 6dbb77fff..042b54068 100644 --- a/src/browser/HttpClient.zig +++ b/src/browser/HttpClient.zig @@ -65,15 +65,6 @@ ws_active: usize = 0, // Count of active http requests http_active: usize = 0, -// Count of intercepted requests. This is to help deal with intercepted requests. -// The client doesn't track intercepted transfers. If a request is intercepted, -// the client forgets about it and requires the interceptor to continue or abort -// it. That works well, except if we only rely on active, we might think there's -// no more network activity when, with interecepted requests, there might be more -// in the future. (We really only need this to properly emit a 'networkIdle' and -// 'networkAlmostIdle' Page.lifecycleEvent in CDP). -intercepted: usize = 0, - // Our curl multi handle. handles: http.Handles, @@ -471,7 +462,15 @@ pub fn syncRequest(self: *Client, allocator: Allocator, params: RequestParams) ! }); while (sync_ctx.completion == .in_progress) { - _ = try self.tick(200); + const status = try self.tick(200); + log.debug(.http, "sync request tick", .{ .status = status }); + switch (status) { + .cdp_socket => { + const cdp = self.cdp_client.?; + _ = cdp.blocking_read(cdp.ctx); + }, + .normal => continue, + } } switch (sync_ctx.completion) { @@ -831,6 +830,7 @@ pub const RequestParams = struct { arena: ArenaAllocator = undefined, /// This is unsafe to access until you pass it to `Client.request()` where it gets assigned. request_id: u32 = undefined, + frame_id: u32, loader_id: u32, method: Method, @@ -1312,25 +1312,24 @@ pub const Transfer = struct { } } - fn detectAuthChallenge(transfer: *Transfer, conn: *const http.Connection) void { - const status = conn.getResponseCode() catch return; - const connect_status = conn.getConnectCode() catch return; + pub fn detectAuthChallenge(conn: *const http.Connection) ?http.AuthChallenge { + const status = conn.getResponseCode() catch return null; + const connect_status = conn.getConnectCode() catch return null; if (status != 401 and status != 407 and connect_status != 401 and connect_status != 407) { - transfer._auth_challenge = null; - return; + return null; } if (conn.getResponseHeader("WWW-Authenticate", 0)) |hdr| { - transfer._auth_challenge = http.AuthChallenge.parse(status, .server, hdr.value) catch null; + return http.AuthChallenge.parse(status, .server, hdr.value) catch null; } else if (conn.getConnectHeader("WWW-Authenticate", 0)) |hdr| { - transfer._auth_challenge = http.AuthChallenge.parse(status, .server, hdr.value) catch null; + return http.AuthChallenge.parse(status, .server, hdr.value) catch null; } else if (conn.getResponseHeader("Proxy-Authenticate", 0)) |hdr| { - transfer._auth_challenge = http.AuthChallenge.parse(status, .proxy, hdr.value) catch null; + return http.AuthChallenge.parse(status, .proxy, hdr.value) catch null; } else if (conn.getConnectHeader("Proxy-Authenticate", 0)) |hdr| { - transfer._auth_challenge = http.AuthChallenge.parse(status, .proxy, hdr.value) catch null; + return http.AuthChallenge.parse(status, .proxy, hdr.value) catch null; } else { - transfer._auth_challenge = .{ .status = status, .source = null, .scheme = null, .realm = null }; + return .{ .status = status, .source = null, .scheme = null, .realm = null }; } } diff --git a/src/browser/Runner.zig b/src/browser/Runner.zig index 68a7ad594..2489382ee 100644 --- a/src/browser/Runner.zig +++ b/src/browser/Runner.zig @@ -185,7 +185,7 @@ fn _tick(self: *Runner, comptime is_cdp: bool, opts: TickOpts) !CDPTickResult { try frame.dispatchLoad(); const http_active = http_client.http_active; - const total_network_activity = http_active + http_client.intercepted; + const total_network_activity = http_active + http_client.interception_layer.intercepted; if (frame._notified_network_almost_idle.check(total_network_activity <= 2)) { frame.notifyNetworkAlmostIdle(); } @@ -211,7 +211,7 @@ fn _tick(self: *Runner, comptime is_cdp: bool, opts: TickOpts) !CDPTickResult { // because is_cdp is false, and that can only be // the case when interception isn't possible. if (comptime IS_DEBUG) { - std.debug.assert(http_client.intercepted == 0); + std.debug.assert(http_client.interception_layer.intercepted == 0); } if (browser.hasBackgroundTasks()) { diff --git a/src/cdp/domains/fetch.zig b/src/cdp/domains/fetch.zig index a7c5672fd..2de7ee3d5 100644 --- a/src/cdp/domains/fetch.zig +++ b/src/cdp/domains/fetch.zig @@ -400,54 +400,49 @@ fn failRequest(cmd: *CDP.Command) !void { } pub fn requestAuthRequired(bc: *CDP.BrowserContext, intercept: *const Notification.RequestAuthRequired) !void { - _ = bc; - _ = intercept; - return error.NullAuthChallenge; + // detachTarget could be called, in which case, we still have a frame doing + // things, but no session. + const session_id = bc.session_id orelse return; + + // We keep it around to wait for modifications to the request. + // 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 frame's requests? + + const intercept_ctx = intercept.intercept_ctx; + const request = intercept.request; + try bc.intercept_state.put(request.*); + + const challenge = intercept_ctx.auth_challenge orelse return error.NullAuthChallenge; + + try bc.cdp.sendEvent("Fetch.authRequired", .{ + .requestId = &id.toInterceptId(request.params.request_id), + .frameId = &id.toFrameId(request.params.frame_id), + .request = network.RequestWriter.init(request), + .resourceType = switch (request.params.resource_type) { + .script => "Script", + .xhr => "XHR", + .document => "Document", + .fetch => "Fetch", + }, + .authChallenge = .{ + .origin = "", // TODO get origin, could be the proxy address for example. + .source = if (challenge.source) |s| (if (s == .server) "Server" else "Proxy") else "", + .scheme = if (challenge.scheme) |s| (if (s == .digest) "digest" else "basic") else "", + .realm = challenge.realm orelse "", + }, + .networkId = &id.toRequestId2(request), + }, .{ .session_id = session_id }); + + log.debug(.cdp, "request auth required", .{ + .state = "paused", + .id = request.params.request_id, + .url = request.params.url, + }); + // Await continueWithAuth + + intercept.wait_for_interception.* = true; } -// pub fn requestAuthRequired(bc: *CDP.BrowserContext, intercept: *const Notification.RequestAuthRequired) !void { -// // detachTarget could be called, in which case, we still have a frame doing -// // things, but no session. -// const session_id = bc.session_id orelse return; - -// // We keep it around to wait for modifications to the request. -// // 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 frame's requests? - -// const transfer = intercept.transfer; -// try bc.intercept_state.put(transfer); - -// const challenge = transfer._auth_challenge orelse return error.NullAuthChallenge; - -// try bc.cdp.sendEvent("Fetch.authRequired", .{ -// .requestId = &id.toInterceptId(transfer.id), -// .frameId = &id.toFrameId(transfer.req.params.frame_id), -// .request = network.TransferAsRequestWriter.init(transfer), -// .resourceType = switch (transfer.req.params.resource_type) { -// .script => "Script", -// .xhr => "XHR", -// .document => "Document", -// .fetch => "Fetch", -// }, -// .authChallenge = .{ -// .origin = "", // TODO get origin, could be the proxy address for example. -// .source = if (challenge.source) |s| (if (s == .server) "Server" else "Proxy") else "", -// .scheme = if (challenge.scheme) |s| (if (s == .digest) "digest" else "basic") else "", -// .realm = challenge.realm orelse "", -// }, -// .networkId = &id.toRequestId(transfer), -// }, .{ .session_id = session_id }); - -// log.debug(.cdp, "request auth required", .{ -// .state = "paused", -// .id = transfer.id, -// .url = transfer.url, -// }); -// // Await continueWithAuth - -// intercept.wait_for_interception.* = true; -// } - // Get u32 from requestId which is formatted as: "INT-{d}" fn idFromRequestId(request_id: []const u8) !u32 { if (!std.mem.startsWith(u8, request_id, "INT-")) { diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index c77319aee..239b51229 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -145,7 +145,7 @@ fn setLifecycleEventsEnabled(cmd: *CDP.Command) !void { const http_client = frame._session.browser.http_client; const http_active = http_client.http_active; - const total_network_activity = http_active + http_client.intercepted; + const total_network_activity = http_active + http_client.interception_layer.intercepted; if (frame._notified_network_almost_idle.check(total_network_activity <= 2)) { try sendPageLifecycle(bc, "networkAlmostIdle", now, frame_id, loader_id); } diff --git a/src/network/layer/InterceptionLayer.zig b/src/network/layer/InterceptionLayer.zig index d6905614d..f5bc6aaae 100644 --- a/src/network/layer/InterceptionLayer.zig +++ b/src/network/layer/InterceptionLayer.zig @@ -25,6 +25,7 @@ const IS_DEBUG = builtin.mode == .Debug; const http = @import("../http.zig"); const URL = @import("../../browser/URL.zig"); const Client = @import("../../browser/HttpClient.zig").Client; +const Transfer = @import("../../browser/HttpClient.zig").Transfer; const Request = @import("../../browser/HttpClient.zig").Request; const Response = @import("../../browser/HttpClient.zig").Response; const FulfilledResponse = @import("../../browser/HttpClient.zig").FulfilledResponse; @@ -59,6 +60,7 @@ fn request(ptr: *anyopaque, client: *Client, in_req: Request) anyerror!void { const intercept_ctx = try pre_wrap_req.params.arena.allocator().create(InterceptContext); intercept_ctx.* = .{ .forward = Forward.fromRequest(pre_wrap_req), + .layer = self, .request = pre_wrap_req, }; @@ -96,18 +98,59 @@ fn request(ptr: *anyopaque, client: *Client, in_req: Request) anyerror!void { pub const InterceptContext = struct { forward: Forward, + layer: *InterceptionLayer, request: Request, content_length: usize = 0, + auth_challenge: ?http.AuthChallenge = null, + tries: usize = 0, + fn startCallback(response: Response) anyerror!void { const self: *InterceptContext = @ptrCast(@alignCast(response.ctx)); + log.debug(.http, "intercept start", .{ .url = self.request.params.url }); return self.forward.forwardStart(response); } fn headerCallback(response: Response) anyerror!bool { const self: *InterceptContext = @ptrCast(@alignCast(response.ctx)); + log.debug(.http, "intercept header", .{ + .url = self.request.params.url, + .status = response.status(), + .content_length = response.contentLength(), + }); + self.content_length = response.contentLength() orelse 0; + switch (response.inner) { + .transfer => |t| { + const status = t.response_header.?.status; + if (status == 401 or status == 407) { + self.auth_challenge = Transfer.detectAuthChallenge(t._conn.?); + + if (self.auth_challenge != null and self.tries < 10) { + var wait_for_interception = false; + + self.request.params.notification.dispatch(.http_request_auth_required, &.{ + .request = &self.request, + .intercept_ctx = self, + .wait_for_interception = &wait_for_interception, + }); + + if (wait_for_interception) { + log.debug(.http, "intercept auth required", .{ + .url = self.request.params.url, + .status = status, + .intercepted = self.layer.intercepted, + }); + self.layer.intercepted += 1; + return false; + } + } + } + }, + else => {}, + } + self.request.params.notification.dispatch(.http_response_header_done, &.{ .request = &self.request, .response = &response, @@ -117,6 +160,10 @@ pub const InterceptContext = struct { fn dataCallback(response: Response, chunk: []const u8) anyerror!void { const self: *InterceptContext = @ptrCast(@alignCast(response.ctx)); + log.debug(.http, "intercept data", .{ + .url = self.request.params.url, + .len = chunk.len, + }); self.request.params.notification.dispatch(.http_response_data, &.{ .data = chunk, @@ -128,6 +175,10 @@ pub const InterceptContext = struct { fn doneCallback(ctx: *anyopaque) anyerror!void { const self: *InterceptContext = @ptrCast(@alignCast(ctx)); + log.debug(.http, "intercept done", .{ + .url = self.request.params.url, + .content_length = self.content_length, + }); self.request.params.notification.dispatch(.http_request_done, &.{ .request = &self.request, .content_length = self.content_length, @@ -137,6 +188,10 @@ pub const InterceptContext = struct { fn errorCallback(ctx: *anyopaque, err: anyerror) void { const self: *InterceptContext = @ptrCast(@alignCast(ctx)); + log.debug(.http, "intercept error", .{ + .url = self.request.params.url, + .err = err, + }); self.request.params.notification.dispatch(.http_request_fail, &.{ .request = &self.request, .err = err, @@ -146,6 +201,7 @@ pub const InterceptContext = struct { fn shutdownCallback(ctx: *anyopaque) void { const self: *InterceptContext = @ptrCast(@alignCast(ctx)); + log.debug(.http, "intercept shutdown", .{ .url = self.request.params.url }); self.forward.forwardShutdown(); } };