refactor: rework how httpclient headers work

src/Config.zig

@@ -786,7 +786,7 @@ pub const WaitUntil = enum {
     done,
 };
 
-/// Pre-formatted HTTP headers for reuse across Http and Client.
+/// HTTP header values shared across Http and Client.
 /// Must be initialized with an allocator that outlives all HTTP connections.
 pub const HttpHeaders = struct {
     const user_agent_base: [:0]const u8 = "Lightpanda/1.0";
@@ -804,9 +804,9 @@ pub const HttpHeaders = struct {
     };
 
     pub const sec_ch_ua: [:0]const u8 = blk: {
-        var out: [:0]const u8 = "Sec-Ch-Ua:";
+        var out: [:0]const u8 = "";
         for (brands, 0..) |b, i| {
-            const sep = if (i == 0) " " else ", ";
+            const sep = if (i == 0) "" else ", ";
             out = out ++ sep ++ "\"" ++ b.brand ++ "\";v=\"" ++ b.version ++ "\"";
         }
         break :blk out;
@@ -816,13 +816,12 @@ pub const HttpHeaders = struct {
     // stream when a client sends Accept-Encoding without Accept-Language,
     // treating it as a bot signal. Ship a neutral default so we look like a
     // normal client.
-    pub const accept_language: [:0]const u8 = "Accept-Language: en-US,en;q=0.9";
+    pub const accept_language: [:0]const u8 = "en-US,en;q=0.9";
 
     // Document-navigation Accept value Chrome sends.
-    pub const navigation_accept: [:0]const u8 = "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8";
+    pub const navigation_accept: [:0]const u8 = "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8";
 
     user_agent: [:0]const u8, // User agent value (e.g. "Lightpanda/1.0")
-    user_agent_header: [:0]const u8,
 
     proxy_bearer_header: ?[:0]const u8,
 
@@ -835,17 +834,13 @@ pub const HttpHeaders = struct {
             user_agent_base;
         errdefer if (config.userAgent() != null or config.userAgentSuffix() != null) allocator.free(user_agent);
 
-        const user_agent_header = try std.fmt.allocPrintSentinel(allocator, "User-Agent: {s}", .{user_agent}, 0);
-        errdefer allocator.free(user_agent_header);
-
         const proxy_bearer_header: ?[:0]const u8 = if (config.proxyBearerToken()) |token|
             try std.fmt.allocPrintSentinel(allocator, "Proxy-Authorization: Bearer {s}", .{token}, 0)
         else
             null;
 
         return .{
             .user_agent = user_agent,
-            .user_agent_header = user_agent_header,
             .proxy_bearer_header = proxy_bearer_header,
         };
     }
@@ -854,7 +849,6 @@ pub const HttpHeaders = struct {
         if (self.proxy_bearer_header) |hdr| {
             allocator.free(hdr);
         }
-        allocator.free(self.user_agent_header);
         if (self.user_agent.ptr != user_agent_base.ptr) {
             allocator.free(self.user_agent);
         }

src/Updater.zig

@@ -52,7 +52,7 @@ pub fn deinit(self: *Updater) void {
 /// Sends running Lightpanda version to remote to get update information.
 /// Outputs directly to given `Writer`.
 pub fn inform(self: *Updater, writer: *std.Io.Writer) !void {
-    const conn = try http.Connection.init(self.x509_store, self.config, null);
+    var conn = try http.Connection.init(self.x509_store, self.config, null);
     defer conn.deinit();
 
     const url = std.fmt.comptimePrint("https://telemetry.lightpanda.io/v/{s}", .{lp.build_config.version});

src/browser/Frame.zig

@@ -277,9 +277,6 @@ origin: ?[]const u8 = null,
 // If null the url must be used.
 base_url: ?[:0]const u8 = null,
 
-// referer header cache.
-referer_header: ?[:0]const u8 = null,
-
 // Document charset (canonical name from encoding_rs, static lifetime)
 charset: []const u8 = "UTF-8",
 
@@ -595,24 +592,9 @@ pub fn httpMetadata(self: *const Frame) HttpMetadata {
 
 // Add common headers for a request:
 // * referer
-pub fn headersForRequest(self: *Frame, headers: *HttpClient.Headers) !void {
-    // Build the referer
-    const referer = blk: {
-        if (self.referer_header == null) {
-            // build the cache
-            if (std.mem.startsWith(u8, self.url, "http")) {
-                self.referer_header = try std.mem.concatWithSentinel(self.arena, u8, &.{ "Referer: ", self.url }, 0);
-            } else {
-                self.referer_header = "";
-            }
-        }
-
-        break :blk self.referer_header.?;
-    };
-
-    // If the referer is empty, ignore the header.
-    if (referer.len > 0) {
-        try headers.add(referer);
+pub fn headersForRequest(self: *Frame, transfer: *HttpClient.Transfer) !void {
+    if (std.mem.startsWith(u8, self.url, "http")) {
+        try transfer.addHeader("Referer", self.url, .{});
     }
 }
 
@@ -802,13 +784,15 @@ pub fn navigate(self: *Frame, request_url: [:0]const u8, opts: NavigateOpts) !vo
     {
         // Ours until submit; clean up if header setup fails.
         errdefer transfer.deinit();
-        try transfer.req.headers.add(lp.Config.HttpHeaders.navigation_accept);
+        try transfer.addHeader("Accept", lp.Config.HttpHeaders.navigation_accept, .{});
         if (opts.header) |hdr| {
-            try transfer.req.headers.add(hdr);
+            // Arrives pre-joined ("Name: Value"), e.g. from the CLI.
+            if (HttpClient.Header.parse(hdr)) |parsed| {
+                try transfer.addHeader(parsed.name, parsed.value, .{});
+            }
         }
         if (opts.referer) |ref| {
-            const ref_header = try std.mem.concatWithSentinel(transfer.arena.allocator(), u8, &.{ "Referer: ", ref }, 0);
-            try transfer.req.headers.add(ref_header);
+            try transfer.addHeader("Referer", ref, .{});
         }
     }
 
@@ -1040,7 +1024,12 @@ fn canScheduleNavigation(self: *Frame, new_target_type: NavigationType) bool {
 }
 
 pub fn makeRequest(self: *Frame, req: HttpClient.Request) !void {
-    return self._session.browser.http_client.request(req, &self._http_owner);
+    const transfer = try self._session.browser.http_client.newRequest(req, &self._http_owner);
+    {
+        errdefer transfer.deinit();
+        try self.headersForRequest(transfer);
+    }
+    return transfer.submit();
 }
 
 // Two-phase variant; see HttpClient.newRequest for the ownership contract.
@@ -2195,9 +2184,25 @@ pub fn loadExternalStylesheet(self: *Frame, link: *Element.Html.Link, href: []co
     // the frame while it's registered (they'd run JS on the parser's stack)
     // and delivers them on the next tick after the sync fetch returns.
 
-    var headers = try http_client.newHeaders();
-    try headers.add("Accept: text/css,*/*;q=0.1");
-    try self.headersForRequest(&headers);
+    const transfer = http_client.newRequest(.{
+        .url = resolved,
+        .method = .GET,
+        .frame_id = self._frame_id,
+        .loader_id = self._loader_id,
+        .cookie_jar = &session.cookie_jar,
+        .cookie_origin = self.url,
+        .resource_type = .stylesheet,
+        .notification = session.notification,
+        .shutdown_callback = HttpClient.noopShutdown, // syncRequest installs its own
+    }, &self._http_owner) catch |err| {
+        log.warn(.http, "external stylesheet fetch", .{ .err = err, .url = resolved });
+        return self.fireElementEvent(element, comptime .wrap("error"));
+    };
+    {
+        errdefer transfer.deinit();
+        try transfer.addHeader("Accept", "text/css,*/*;q=0.1", .{});
+        try self.headersForRequest(transfer);
+    }
 
     // Set the script-manager `is_evaluating` flag for the same reason
     // `ScriptManager.addFromElement` does: `syncRequest` pumps the CDP
@@ -2211,18 +2216,7 @@ pub fn loadExternalStylesheet(self: *Frame, link: *Element.Html.Link, href: []co
     sm.is_evaluating = true;
     defer sm.endEvaluationWindow(was_evaluating);
 
-    var response = http_client.syncRequest(.{
-        .url = resolved,
-        .method = .GET,
-        .frame_id = self._frame_id,
-        .loader_id = self._loader_id,
-        .headers = headers,
-        .cookie_jar = &session.cookie_jar,
-        .cookie_origin = self.url,
-        .resource_type = .stylesheet,
-        .notification = session.notification,
-        .shutdown_callback = HttpClient.noopShutdown, // syncRequest installs its own
-    }, &self._http_owner) catch |err| {
+    var response = http_client.syncRequest(transfer) catch |err| {
         log.warn(.http, "external stylesheet fetch", .{ .err = err, .url = resolved });
         return self.fireElementEvent(element, comptime .wrap("error"));
     };

src/browser/ScriptManager.zig

@@ -97,10 +97,6 @@ pub fn tailHook(base: *ScriptManagerBase) void {
     }
 }
 
-fn getHeaders(self: *ScriptManager) !HttpClient.Headers {
-    return self.base.getHeaders();
-}
-
 // Returns true when a fetch was started: the link's load/error event fires
 // when the fetch settles. false (duplicate hint) = no event will fire.
 // element is null when the hint came from the prescan rather than a <link>.
@@ -140,7 +136,6 @@ pub fn preloadScript(self: *ScriptManager, element: ?*Element.Html, url: []const
         .method = .GET,
         .frame_id = frame._frame_id,
         .loader_id = frame._loader_id,
-        .headers = try self.base.getHeaders(),
         .cookie_jar = &frame._session.cookie_jar,
         .cookie_origin = frame.url,
         .resource_type = .script,
@@ -352,18 +347,22 @@ pub fn addFromElement(self: *ScriptManager, comptime from_parser: bool, script_e
                 script.status = pre.status;
                 script.complete = true;
             } else {
-                const response = try self.base.client.syncRequest(.{
+                const transfer = try self.base.client.newRequest(.{
                     .url = remote_url,
                     .method = .GET,
                     .frame_id = frame._frame_id,
                     .loader_id = frame._loader_id,
-                    .headers = try self.getHeaders(),
                     .cookie_jar = &frame._session.cookie_jar,
                     .cookie_origin = frame.url,
                     .resource_type = .script,
                     .notification = frame._session.notification,
                     .shutdown_callback = HttpClient.noopShutdown, // syncRequest installs its own
                 }, &frame._http_owner);
+                {
+                    errdefer transfer.deinit();
+                    try frame.headersForRequest(transfer);
+                }
+                const response = try self.base.client.syncRequest(transfer);
 
                 // Take the body's arena rather than releasing it: `source`
                 // has to outlive this call, up to script.deinit().
@@ -400,7 +399,6 @@ pub fn addFromElement(self: *ScriptManager, comptime from_parser: bool, script_e
         .method = .GET,
         .frame_id = frame._frame_id,
         .loader_id = frame._loader_id,
-        .headers = try self.getHeaders(),
         .cookie_jar = &frame._session.cookie_jar,
         .cookie_origin = frame.url,
         .resource_type = .script,

src/browser/ScriptManagerBase.zig

@@ -73,12 +73,6 @@ pub const Owner = union(enum) {
         };
     }
 
-    pub fn addHeaders(self: Owner, headers: *HttpClient.Headers) !void {
-        return switch (self) {
-            inline else => |g| g.headersForRequest(headers),
-        };
-    }
-
     pub fn makeRequest(self: Owner, req: HttpClient.Request) !void {
         return switch (self) {
             inline else => |g| g.makeRequest(req),
@@ -177,12 +171,6 @@ fn clearList(list: *std.DoublyLinkedList) void {
     }
 }
 
-pub fn getHeaders(self: *ScriptManagerBase) !http.Headers {
-    var headers = try self.client.newHeaders();
-    try self.owner.addHeaders(&headers);
-    return headers;
-}
-
 fn acquireArena(self: *ScriptManagerBase, size_or_bucket: anytype, debug: []const u8) !*lp.Arena {
     return self.owner.session().getArena(size_or_bucket, debug);
 }
@@ -276,7 +264,6 @@ pub fn preloadImport(self: *ScriptManagerBase, url: [:0]const u8, referrer: []co
         .method = .GET,
         .frame_id = owner.frameId(),
         .loader_id = owner.loaderId(),
-        .headers = try self.getHeaders(),
         .cookie_jar = &session.cookie_jar,
         .cookie_origin = owner.url(),
         .resource_type = .script,
@@ -471,7 +458,6 @@ pub fn getAsyncImport(self: *ScriptManagerBase, url: [:0]const u8, cb: ImportAsy
         .method = .GET,
         .frame_id = owner.frameId(),
         .loader_id = owner.loaderId(),
-        .headers = try self.getHeaders(),
         .resource_type = .script,
         .cookie_jar = &session.cookie_jar,
         .cookie_origin = owner.url(),

src/browser/js/Execution.zig

@@ -84,9 +84,9 @@ pub fn getPinnedArena(self: *const Execution, size_or_bucket: anytype, debug: []
     return self.page.getPinnedArena(size_or_bucket, debug);
 }
 
-pub fn headersForRequest(self: *const Execution, headers: *HttpClient.Headers) !void {
+pub fn headersForRequest(self: *const Execution, transfer: *HttpClient.Transfer) !void {
     return switch (self.js.global) {
-        inline else => |g| g.headersForRequest(headers),
+        inline else => |g| g.headersForRequest(transfer),
     };
 }
 

src/browser/webapi/WorkerGlobalScope.zig

@@ -256,8 +256,8 @@ pub fn hasDirectListeners(self: *WorkerGlobalScope, target: *EventTarget, typ: [
 
 // Workers don't have their own Referer; per spec, dedicated worker requests
 // use the parent document's URL. Delegate to the owning frame.
-pub fn headersForRequest(self: *WorkerGlobalScope, headers: *HttpClient.Headers) !void {
-    return self._frame.headersForRequest(headers);
+pub fn headersForRequest(self: *WorkerGlobalScope, transfer: *HttpClient.Transfer) !void {
+    return self._frame.headersForRequest(transfer);
 }
 
 pub fn isSameOrigin(self: *const WorkerGlobalScope, url: [:0]const u8) bool {
@@ -270,7 +270,12 @@ pub fn isSameOrigin(self: *const WorkerGlobalScope, url: [:0]const u8) bool {
 }
 
 pub fn makeRequest(self: *WorkerGlobalScope, req: HttpClient.Request) !void {
-    return self._session.browser.http_client.request(req, &self._http_owner);
+    const transfer = try self._session.browser.http_client.newRequest(req, &self._http_owner);
+    {
+        errdefer transfer.deinit();
+        try self.headersForRequest(transfer);
+    }
+    return transfer.submit();
 }
 
 // Two-phase variant; see HttpClient.newRequest for the ownership contract.
@@ -416,16 +421,12 @@ fn importScript(self: *WorkerGlobalScope, arena: Allocator, url: [:0]const u8) !
 
     const http_client = &session.browser.http_client;
 
-    var headers = try http_client.newHeaders();
-    try self.headersForRequest(&headers);
-
-    var response = http_client.syncRequest(.{
+    const transfer = http_client.newRequest(.{
         .url = resolved_url,
         .method = .GET,
         .frame_id = self._frame_id,
         .document_frame_id = self._frame._frame_id,
         .loader_id = self._loader_id,
-        .headers = headers,
         .cookie_jar = &session.cookie_jar,
         .cookie_origin = self.url,
         .resource_type = .script,
@@ -435,6 +436,15 @@ fn importScript(self: *WorkerGlobalScope, arena: Allocator, url: [:0]const u8) !
         log.warn(.http, "importScript", .{ .url = resolved_url, .err = err });
         return error.NetworkError;
     };
+    {
+        errdefer transfer.deinit();
+        try self.headersForRequest(transfer);
+    }
+
+    var response = http_client.syncRequest(transfer) catch |err| {
+        log.warn(.http, "importScript", .{ .url = resolved_url, .err = err });
+        return error.NetworkError;
+    };
     defer response.deinit();
 
     if (response.status != 200) {