From 96d0f07b150c261af791e22e620e670d78115a23 Mon Sep 17 00:00:00 2001 From: nikneym Date: Mon, 21 Sep 2026 12:13:47 +0300 Subject: [PATCH] `cdp`: setCookie sameSite parity with Chrome --- src/browser/webapi/storage/Cookie.zig | 35 +++---- src/cookies.zig | 79 +++++++++------- src/server/cdp/domains/network.zig | 65 +------------ src/server/cdp/domains/storage.zig | 131 +++++++++++++++++--------- 4 files changed, 152 insertions(+), 158 deletions(-) diff --git a/src/browser/webapi/storage/Cookie.zig b/src/browser/webapi/storage/Cookie.zig index 7fa144362..698e89ebb 100644 --- a/src/browser/webapi/storage/Cookie.zig +++ b/src/browser/webapi/storage/Cookie.zig @@ -43,9 +43,10 @@ expires: ?f64, secure: bool = false, http_only: bool = false, same_site: SameSite = .none, -// True when Set-Cookie carried no SameSite attribute: the cookie is Lax -// through the "Default" enforcement mode (RFC 6265bis 5.6.7.1), which -// makes it eligible for "Lax-allowing-unsafe", see appliesTo. +// True when no SameSite was given, by the Set-Cookie header, the CDP +// driver or the cookie file: the cookie is Lax through the "Default" +// enforcement mode (RFC 6265bis 5.6.7.1), which makes it eligible for +// "Lax-allowing-unsafe", see appliesTo. same_site_default: bool = false, // Seconds since the epoch. Set by Jar.add, and inherited from the cookie it // replaces, so a site re-setting a cookie on every response can't keep it @@ -57,13 +58,20 @@ pub const SameSite = enum { lax, none, - pub fn parse(value: []const u8) error{InvalidSameSite}!?SameSite { - if (std.ascii.eqlIgnoreCase(value, "strict")) return .strict; - if (std.ascii.eqlIgnoreCase(value, "lax")) return .lax; - if (std.ascii.eqlIgnoreCase(value, "none")) return .none; - if (std.ascii.eqlIgnoreCase(value, "no_restriction")) return .none; - if (std.ascii.eqlIgnoreCase(value, "unspecified")) return null; - return error.InvalidSameSite; + // The SameSite a Set-Cookie attribute or a cookie file spells out: + // Strict, Lax or None, case-insensitive (RFC 6265bis 5.6.7; the file + // has both saveToFile's lowercase tags and CDP's casing). Anything + // else, including no value at all, "unspecified" and the + // chrome.cookies "no_restriction", is not a value: the cookie is + // unspecified and Lax by default, see `same_site_default`. CDP takes + // Chrome's exact spelling only, see `parseSameSite` in + // server/cdp/domains/storage.zig. + pub fn parse(value: ?[]const u8) ?SameSite { + const raw = value orelse return null; + if (std.ascii.eqlIgnoreCase(raw, "strict")) return .strict; + if (std.ascii.eqlIgnoreCase(raw, "lax")) return .lax; + if (std.ascii.eqlIgnoreCase(raw, "none")) return .none; + return null; } }; @@ -155,12 +163,7 @@ pub fn parse(allocator: Allocator, url: [:0]const u8, str: []const u8) !Cookie { .@"max-age" => max_age = std.fmt.parseInt(i64, value, 10) catch continue, .expires => expires = value, .httponly => http_only = true, - .samesite => { - if (value.len > scrap.len) { - continue; - } - same_site = std.meta.stringToEnum(Cookie.SameSite, std.ascii.lowerString(&scrap, value)) orelse continue; - }, + .samesite => same_site = SameSite.parse(value) orelse continue, } } diff --git a/src/cookies.zig b/src/cookies.zig index 92dcb7d9d..de8ffff8e 100644 --- a/src/cookies.zig +++ b/src/cookies.zig @@ -20,10 +20,12 @@ const Session = @import("browser/Session.zig"); const Cookie = @import("browser/webapi/storage/Cookie.zig"); const log = lp.log; +const Allocator = std.mem.Allocator; /// Load cookies from a JSON file into the cookie jar. /// The file format is an array of objects with: name, value, domain, path, -/// expires (optional, float), secure (optional, bool), httpOnly (optional, bool). +/// expires (optional, float), secure (optional, bool), httpOnly (optional, bool), +/// sameSite (optional, Strict/Lax/None). /// This matches the CDP Network.Cookie format used by Puppeteer and Playwright. pub fn loadFromFile(session: *Session, path: []const u8) void { _loadFromFile(session, path) catch |err| { @@ -68,27 +70,7 @@ fn _loadFromFile(session: *Session, path: []const u8) !void { var loaded: usize = 0; for (json_cookies) |jc| { - var cookie_arena = std.heap.ArenaAllocator.init(jar.allocator); - errdefer cookie_arena.deinit(); - - const a = cookie_arena.allocator(); - const name = try a.dupe(u8, jc.name); - const value = try a.dupe(u8, jc.value); - const domain = try a.dupe(u8, jc.domain); - const cookie_path = if (jc.path) |p| try a.dupe(u8, p) else "/"; - - const cookie = Cookie{ - .arena = cookie_arena, - .name = name, - .value = value, - .domain = domain, - .path = cookie_path, - .expires = jc.expires, - .secure = jc.secure orelse false, - .http_only = jc.httpOnly orelse false, - .same_site = parseJsonSameSite(jc.sameSite), - }; - + const cookie = try jc.toCookie(jar.allocator); jar.add(cookie, now, true) catch |err| { log.warn(.app, "invalid cookie", .{ .name = jc.name, .err = err }); continue; @@ -114,8 +96,13 @@ fn _saveToFile(jar: *Cookie.Jar, path: []const u8) !void { var buf: [8192]u8 = undefined; var writer = file.writer(lp.io, &buf); - const w = &writer.interface; + try writeJson(jar, &writer.interface); + try writer.end(); + log.info(.app, "Cookie.saveToFile", .{ .path = path, .count = jar.cookies.items.len }); +} + +fn writeJson(jar: *const Cookie.Jar, w: *std.Io.Writer) !void { try w.writeByte('['); for (jar.cookies.items, 0..) |c, i| { if (i > 0) { @@ -131,17 +118,16 @@ fn _saveToFile(jar: *Cookie.Jar, path: []const u8) !void { .expires = c.expires, .secure = c.secure, .httpOnly = c.http_only, - .sameSite = @tagName(c.same_site), - }, .{}, w); + // Left out for an unspecified cookie, as Network.getCookies + // does, so that it's still Lax by default once loaded back. + .sameSite = if (c.same_site_default) null else @tagName(c.same_site), + }, .{ .emit_null_optional_fields = false }, w); } if (jar.cookies.items.len > 0) { try w.writeByte('\n'); } try w.writeAll("]\n"); - try writer.end(); - - log.info(.app, "Cookie.saveToFile", .{ .path = path, .count = jar.cookies.items.len }); } const JsonCookie = struct { @@ -153,12 +139,35 @@ const JsonCookie = struct { secure: ?bool = null, httpOnly: ?bool = null, sameSite: ?[]const u8 = null, -}; -fn parseJsonSameSite(value: ?[]const u8) Cookie.SameSite { - const raw = value orelse return .lax; - return (Cookie.SameSite.parse(raw) catch .lax) orelse .lax; -} + fn toCookie(self: JsonCookie, allocator: Allocator) !Cookie { + var arena = std.heap.ArenaAllocator.init(allocator); + errdefer arena.deinit(); + const a = arena.allocator(); + + // Allocate before the struct literal copies `arena` into the result. + const name = try a.dupe(u8, self.name); + const value = try a.dupe(u8, self.value); + const domain = try a.dupe(u8, self.domain); + const path = if (self.path) |p| try a.dupe(u8, p) else "/"; + + // A missing or unrecognised sameSite leaves the cookie unspecified. + const same_site = Cookie.SameSite.parse(self.sameSite); + + return .{ + .arena = arena, + .name = name, + .value = value, + .domain = domain, + .path = path, + .expires = self.expires, + .secure = self.secure orelse false, + .http_only = self.httpOnly orelse false, + .same_site = same_site orelse .lax, + .same_site_default = same_site == null, + }; + } +}; /// Netscape cookie file format parser with `#HttpOnly_` addition from curl. /// https://docs.cyotek.com/cyowcopy/1.10/netscapecookieformat.html @@ -418,5 +427,7 @@ test "cookies: load JSON accepts CDP SameSite casing" { .{ .ignore_unknown_fields = true }, ); - try std.testing.expectEqual(Cookie.SameSite.lax, parseJsonSameSite(parsed[0].sameSite)); + const cookie = try parsed[0].toCookie(std.testing.allocator); + defer cookie.deinit(); + try std.testing.expectEqual(Cookie.SameSite.lax, cookie.same_site); } diff --git a/src/server/cdp/domains/network.zig b/src/server/cdp/domains/network.zig index 6b1a0e71c..55295ad97 100644 --- a/src/server/cdp/domains/network.zig +++ b/src/server/cdp/domains/network.zig @@ -291,12 +291,9 @@ fn setCookie(cmd: *CDP.Command) !void { )) orelse return error.InvalidParams; const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - CdpStorage.setCdpCookie(&bc.session.cookie_jar, params) catch |err| switch (err) { - error.InvalidSameSite => return CdpStorage.invalidSameSiteError(cmd, params.sameSite), - else => return err, - }; + const stored = try CdpStorage.setCdpCookies(&bc.session.cookie_jar, &.{params}); - try cmd.sendResult(.{ .success = true }, .{}); + try cmd.sendResult(.{ .success = stored == 1 }, .{}); } fn setCookies(cmd: *CDP.Command) !void { @@ -305,12 +302,7 @@ fn setCookies(cmd: *CDP.Command) !void { })) orelse return error.InvalidParams; const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - for (params.cookies) |param| { - CdpStorage.setCdpCookie(&bc.session.cookie_jar, param) catch |err| switch (err) { - error.InvalidSameSite => return CdpStorage.invalidSameSiteError(cmd, param.sameSite), - else => return err, - }; - } + _ = try CdpStorage.setCdpCookies(&bc.session.cookie_jar, params.cookies); try cmd.sendResult(null, .{}); } @@ -980,57 +972,6 @@ test "cdp.Network: cookies" { try ctx.expectSentResult(.{ .cookies = &[_]ResCookie{} }, .{ .id = 10 }); } -test "cdp.Network: setCookie accepts the sameSite spellings drivers send" { - // Issue #3453: cookies bridged from chrome.cookies / other tooling come - // with lowercase or `no_restriction` sameSite values. Accept them on - // input; getCookies keeps reporting the canonical CDP spelling. - const ResCookie = CdpStorage.ResCookie; - - var ctx = try testing.context(); - defer ctx.deinit(); - _ = try ctx.loadBrowserContext(.{ .id = "BID-SS" }); - - try ctx.processMessage( - \\{"id":1,"method":"Network.setCookie","params":{"name":"a","value":"1","url":"https://example.com/","sameSite":"lax"}} - ); - try ctx.expectSentResult(.{ .success = true }, .{ .id = 1 }); - - try ctx.processMessage( - \\{"id":2,"method":"Network.setCookies","params":{"cookies":[ - \\ {"name":"b","value":"2","url":"https://example.com/","sameSite":"no_restriction"}, - \\ {"name":"c","value":"3","url":"https://example.com/","sameSite":"STRICT"}, - \\ {"name":"d","value":"4","url":"https://example.com/","sameSite":"unspecified"} - \\]}} - ); - try ctx.expectSentResult(null, .{ .id = 2 }); - - try ctx.processMessage(.{ - .id = 3, - .method = "Network.getAllCookies", - }); - try ctx.expectSentResult(.{ - .cookies = &[_]ResCookie{ - .{ .name = "a", .value = "1", .domain = "example.com", .size = 2, .secure = true, .sameSite = "Lax" }, - // `no_restriction` is the chrome.cookies spelling of None. - .{ .name = "b", .value = "2", .domain = "example.com", .size = 2, .secure = true, .sameSite = "None" }, - .{ .name = "c", .value = "3", .domain = "example.com", .size = 2, .secure = true, .sameSite = "Strict" }, - // `unspecified` means no SameSite was set, so it takes the default. - .{ .name = "d", .value = "4", .domain = "example.com", .size = 2, .secure = true, .sameSite = "Lax" }, - }, - }, .{ .id = 3 }); -} - -test "cdp.Network: setCookie rejects an unknown sameSite by name" { - var ctx = try testing.context(); - defer ctx.deinit(); - _ = try ctx.loadBrowserContext(.{ .id = "BID-SE" }); - - try ctx.processMessage( - \\{"id":1,"method":"Network.setCookie","params":{"name":"a","value":"1","url":"https://example.com/","sameSite":"sometimes"}} - ); - try ctx.expectSentError(-31998, "Invalid value 'sometimes' for 'sameSite'. Accepted (case-insensitive): Strict, Lax, None, no_restriction, unspecified", .{ .id = 1 }); -} - test "cdp.Network: clearBrowserCookies accepts empty params object" { const CdpCookie = CdpStorage.CdpCookie; const ResCookie = CdpStorage.ResCookie; diff --git a/src/server/cdp/domains/storage.zig b/src/server/cdp/domains/storage.zig index 5e6395730..38f00a928 100644 --- a/src/server/cdp/domains/storage.zig +++ b/src/server/cdp/domains/storage.zig @@ -24,6 +24,7 @@ const URL = @import("../../../browser/URL.zig"); const Cookie = @import("../../../browser/webapi/storage/storage.zig").Cookie; const log = lp.log; +const Allocator = std.mem.Allocator; const CookieJar = Cookie.Jar; pub const PreparedUri = Cookie.PreparedUri; @@ -85,24 +86,11 @@ fn setCookies(cmd: *CDP.Command) !void { } } - for (params.cookies) |param| { - setCdpCookie(&bc.session.cookie_jar, param) catch |err| switch (err) { - error.InvalidSameSite => return invalidSameSiteError(cmd, param.sameSite), - else => return err, - }; - } + _ = try setCdpCookies(&bc.session.cookie_jar, params.cookies); try cmd.sendResult(null, .{}); } -pub fn invalidSameSiteError(cmd: *CDP.Command, value: []const u8) !void { - const message = try std.fmt.allocPrint( - cmd.arena, - "Invalid value '{s}' for 'sameSite'. Accepted (case-insensitive): Strict, Lax, None, no_restriction, unspecified", - .{value}, - ); - return cmd.sendError(-31998, message, .{}); -} const CookiePriority = enum { Low, Medium, @@ -127,7 +115,7 @@ pub const CdpCookie = struct { path: ?[:0]const u8 = null, secure: ?bool = null, // default: https://www.rfc-editor.org/rfc/rfc6265#section-5.3 httpOnly: bool = false, // default: https://www.rfc-editor.org/rfc/rfc6265#section-5.3 - sameSite: []const u8 = "None", // default: https://datatracker.ietf.org/doc/html/draft-west-first-party-cookies + sameSite: ?[]const u8 = null, // Strict, Lax or None; anything else is unspecified, see parseSameSite expires: ?f64 = null, // -1? says google priority: CookiePriority = .Medium, // default: https://datatracker.ietf.org/doc/html/draft-west-cookie-priority-00 sameParty: ?bool = null, @@ -136,46 +124,93 @@ pub const CdpCookie = struct { partitionKey: ?CookiePartitionKey = null, }; -pub fn setCdpCookie(cookie_jar: *CookieJar, param: CdpCookie) !void { +/// Network.setCookie, Network.setCookies and Storage.setCookies. Every +/// cookie is built before any is added: an entry `buildCdpCookie` refuses +/// in the middle of a batch leaves the jar untouched, as Chrome's +/// SetCookies does. `Jar.add`'s own checks can still stop a batch part-way. +/// Returns how many cookies were stored. +pub fn setCdpCookies(cookie_jar: *CookieJar, params: []const CdpCookie) !usize { + var cookies = try std.ArrayList(Cookie).initCapacity(cookie_jar.allocator, params.len); + defer cookies.deinit(cookie_jar.allocator); + + // A cookie handed to `Jar.add` is its to free, stored or not; the ones + // we still hold when something fails are ours. + var added: usize = 0; + errdefer for (cookies.items[added..]) |*cookie| cookie.deinit(); + + for (params) |param| { + cookies.appendAssumeCapacity(try buildCdpCookie(cookie_jar.allocator, param)); + } + + const now = lp.datetime.timestamp(.real); + var stored: usize = 0; + for (cookies.items) |cookie| { + added += 1; + if (cookie.same_site == .none and !cookie.secure) { + // Chrome's store refuses SameSite=None without Secure, as + // `Cookie.parse` does for a Set-Cookie. + cookie.deinit(); + continue; + } + try cookie_jar.add(cookie, now, true); + stored += 1; + } + return stored; +} + +fn buildCdpCookie(allocator: Allocator, param: CdpCookie) !Cookie { // Silently ignore partitionKey since we don't support partitioned cookies (CHIPS). // This allows Puppeteer's frame.setCookie() to work, which may send cookies with // partitionKey as part of its cookie-setting workflow. if (param.partitionKey != null) { - log.warn(.not_implemented, "partition key", .{ .src = "setCdpCookie" }); + log.warn(.not_implemented, "partition key", .{ .src = "buildCdpCookie" }); } // Still reject unsupported features if (param.priority != .Medium or param.sameParty != null or param.sourceScheme != null) { return error.NotImplemented; } - const same_site = (try Cookie.SameSite.parse(param.sameSite)) orelse .lax; + // NOTE: The param.url can affect the default domain, (NOT path), secure, source port, and source scheme. + const secure = if (param.secure) |s| s else if (param.url) |url| URL.isSecure(url) else false; - // The errdefer only protects construction failures. Once we `break :blk` - // with the Cookie value, `Jar.add` owns its lifetime. - const cookie = blk: { - var arena = std.heap.ArenaAllocator.init(cookie_jar.allocator); - errdefer arena.deinit(); - const a = arena.allocator(); + const same_site = parseSameSite(param.sameSite); - // NOTE: The param.url can affect the default domain, (NOT path), secure, source port, and source scheme. - const domain = try Cookie.parseDomain(a, param.url, param.domain); - const path = if (param.path == null) "/" else try Cookie.parsePath(a, null, param.path); + var arena = std.heap.ArenaAllocator.init(allocator); + errdefer arena.deinit(); + const a = arena.allocator(); - const secure = if (param.secure) |s| s else if (param.url) |url| URL.isSecure(url) else false; + // Allocate before the struct literal copies `arena` into the result. + const name = try a.dupe(u8, param.name); + const value = try a.dupe(u8, param.value); + const domain = try Cookie.parseDomain(a, param.url, param.domain); + const path = if (param.path == null) "/" else try Cookie.parsePath(a, null, param.path); - break :blk Cookie{ - .arena = arena, - .name = try a.dupe(u8, param.name), - .value = try a.dupe(u8, param.value), - .path = path, - .domain = domain, - .expires = param.expires, - .secure = secure, - .http_only = param.httpOnly, - .same_site = same_site, - }; + return .{ + .arena = arena, + .name = name, + .value = value, + .path = path, + .domain = domain, + .expires = param.expires, + .secure = secure, + .http_only = param.httpOnly, + .same_site = same_site orelse .lax, + .same_site_default = same_site == null, + }; +} + +// Chrome's MakeCookieFromProtocolValues takes CDP's exact Strict, Lax or +// None. Anything else, "lax" included, or no value leaves the cookie +// unspecified: Lax by default with the Lax-allowing-unsafe window, like a +// Set-Cookie without the attribute (`Cookie.parse`, which, unlike CDP, is +// case-insensitive). +fn parseSameSite(value: ?[]const u8) ?Cookie.SameSite { + const same_site = std.meta.stringToEnum(enum { Strict, Lax, None }, value orelse return null) orelse return null; + return switch (same_site) { + .Strict => .strict, + .Lax => .lax, + .None => .none, }; - try cookie_jar.add(cookie, lp.datetime.timestamp(.real), true); } pub const CookieWriter = struct { @@ -239,11 +274,15 @@ fn writeCookie(cookie: *const Cookie, w: anytype) !void { try w.objectField("session"); try w.write(cookie.expires == null); - try w.objectField("sameSite"); - switch (cookie.same_site) { - .none => try w.write("None"), - .lax => try w.write("Lax"), - .strict => try w.write("Strict"), + // Chrome's BuildCookie reports an explicit Strict/Lax/None only; a + // cookie that is Lax by default has no sameSite. + if (!cookie.same_site_default) { + try w.objectField("sameSite"); + switch (cookie.same_site) { + .none => try w.write("None"), + .lax => try w.write("Lax"), + .strict => try w.write("Strict"), + } } // TODO experimentals @@ -317,5 +356,5 @@ pub const ResCookie = struct { size: usize = 0, httpOnly: bool = false, secure: bool = false, - sameSite: []const u8 = "None", + sameSite: ?[]const u8 = null, };