From 4cdf8b441ea91a5c0edc872e1410aefdbfa12e10 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Mon, 20 Jul 2026 11:55:07 +0800
Subject: [PATCH] webapi: Support max-age for CookieStore

Normalize get name so that it matches the normalize stored cookie name.

Don't queue cookie change event if there's no listener. This one's a bit tricky
since we do this check inside the callback already. But that means a listener
added after the event is created (but before it's delivered) gets a change
event it should not. So we need to check it both at event-creation time and
at delivery time.
---
 src/browser/tests/cookie_store.html        | 65 +++++++++++++++++++
 src/browser/webapi/storage/CookieStore.zig | 72 ++++++++++++++++++----
 2 files changed, 124 insertions(+), 13 deletions(-)

diff --git a/src/browser/tests/cookie_store.html b/src/browser/tests/cookie_store.html
index b688d6506..b1420a0d3 100644
--- a/src/browser/tests/cookie_store.html
+++ b/src/browser/tests/cookie_store.html
@@ -73,6 +73,71 @@
   });
 </script>
 
+<script id=change-event-binds-listeners-at-mutation type=module>
+  const state = await testing.async();
+  // A change enqueued while nobody is subscribed must not be delivered to a
+  // listener that registers afterwards but before the task drains.
+  await cookieStore.set('pre-listener', 'v');
+
+  const events = [];
+  const handler = (e) => events.push(...e.changed.map(c => c.name));
+  cookieStore.addEventListener('change', handler);
+
+  await cookieStore.set('post-listener', 'v');
+  await new Promise(r => setTimeout(r, 0));
+
+  cookieStore.removeEventListener('change', handler);
+  await cookieStore.delete('pre-listener');
+  await cookieStore.delete('post-listener');
+  state.resolve();
+  await state.done(() => {
+    testing.expectEqual(false, events.includes('pre-listener'));
+    testing.expectEqual(true, events.includes('post-listener'));
+  });
+</script>
+
+<script id=set-max-age type=module>
+  const state = await testing.async();
+  const events = [];
+  const handler = (e) => events.push(e);
+  cookieStore.addEventListener('change', handler);
+
+  // A positive maxAge stores the cookie.
+  await cookieStore.set({ name: 'ma-pos', value: 'v', maxAge: 60 });
+  const posItem = await cookieStore.get('ma-pos');
+
+  // maxAge <= 0 expires immediately: nothing is stored and no change event
+  // fires for it.
+  await cookieStore.set({ name: 'ma-zero', value: 'v', maxAge: 0 });
+  await cookieStore.set({ name: 'ma-neg', value: 'v', maxAge: -60 });
+  await new Promise(r => setTimeout(r, 0));
+  const zeroItem = await cookieStore.get('ma-zero');
+  const negItem = await cookieStore.get('ma-neg');
+
+  // maxAge and expires are mutually exclusive.
+  let bothRejected = false;
+  try {
+    await cookieStore.set({ name: 'ma-both', value: 'v', maxAge: 60, expires: Date.now() + 60000 });
+  } catch (err) {
+    bothRejected = true;
+  }
+
+  cookieStore.removeEventListener('change', handler);
+  await cookieStore.delete('ma-pos');
+  state.resolve();
+  await state.done(() => {
+    testing.expectEqual('v', posItem.value);
+    testing.expectEqual(null, zeroItem);
+    testing.expectEqual(null, negItem);
+    testing.expectEqual(true, bothRejected);
+
+    // Only the stored (positive) cookie should have produced an event.
+    const names = events.flatMap(e => e.changed.map(c => c.name));
+    testing.expectEqual(1, names.length);
+    testing.expectEqual('ma-pos', names[0]);
+  });
+</script>
+
 <script id=delete-options type=module>
   const state = await testing.async();
   await cookieStore.set('tmp', 'x');
diff --git a/src/browser/webapi/storage/CookieStore.zig b/src/browser/webapi/storage/CookieStore.zig
index 0de8b3ec3..f2d83f6dd 100644
--- a/src/browser/webapi/storage/CookieStore.zig
+++ b/src/browser/webapi/storage/CookieStore.zig
@@ -68,7 +68,9 @@ fn onCookieChanged(ctx: *anyopaque, data: *const Notification.CookieChanged) !vo
     // same-site treated as first-party against the document URL).
     const doc_url = exec.url.*;
     const target = Cookie.PreparedUri.init(doc_url);
-    if (target.host.len == 0) return;
+    if (target.host.len == 0) {
+        return;
+    }
 
     const probe = Cookie{
         .arena = undefined,
@@ -81,7 +83,16 @@ fn onCookieChanged(ctx: *anyopaque, data: *const Notification.CookieChanged) !vo
         .http_only = data.http_only,
         .same_site = data.same_site,
     };
-    if (!probe.appliesTo(&target, true, true, false)) return;
+    if (!probe.appliesTo(&target, true, true, false)) {
+        return;
+    }
+
+    // Rechecked in ChangeCallback.run, but between now and then, a listener
+    // could be added which should NOT get this event (because it wasn't addded
+    // at this point).
+    if (!exec.hasDirectListeners(self.asEventTarget(), "change", self._on_change)) {
+        return;
+    }
 
     // Per spec, `change` is dispatched as a queued task — never synchronously
     // from the mutation site. We snapshot the notification fields onto a
@@ -205,6 +216,7 @@ const CookieInit = struct {
     name: []const u8,
     value: []const u8,
     expires: ?f64 = null,
+    maxAge: ?f64 = null,
     domain: ?[]const u8 = null,
     path: []const u8 = "/",
     sameSite: SameSite = .strict,
@@ -236,13 +248,18 @@ const SameSite = enum {
     pub const js_enum_from_string = true;
 };
 
-pub fn get(_: *CookieStore, input: GetInput, exec: *const Execution) !js.Promise {
+pub fn get(_: *CookieStore, input: ?GetInput, exec: *const Execution) !js.Promise {
     const local = exec.js.local.?;
 
-    const name: ?[]const u8, const url: ?[]const u8 = switch (input) {
+    const name: ?[]const u8, const url: ?[]const u8 = if (input) |inp| switch (inp) {
         .name => |n| .{ n, null },
         .options => |o| .{ o.name, o.url },
-    };
+    } else .{ null, null };
+
+    if (name == null and url == null) {
+        // Unlike getAll(), get() requires a name or url
+        return local.rejectPromise(.{ .type_error = "get requires a name or url" });
+    }
 
     const items = matchCookies(exec, name, url, true) catch |err| {
         return local.rejectPromise(.{ .type_error = @errorName(err) });
@@ -319,18 +336,27 @@ fn resolveQueryUrl(exec: *const Execution, _override: ?[]const u8) ![:0]const u8
     const current = exec.url.*;
     const override = _override orelse return current;
 
-    const resolved = try URL.resolve(exec.call_arena, exec.base(), override, .{});
-    if (!exec.isSameOrigin(resolved)) return error.SecurityError;
+    const resolved = try URL.resolve(exec.local_arena, exec.base(), override, .{});
+    if (!exec.isSameOrigin(resolved)) {
+        return error.SecurityError;
+    }
 
     switch (exec.js.global) {
         .frame => {
-            if (!std.mem.eql(u8, resolved, current)) return error.InvalidUrl;
+            // URL that differs from document only by #hash is the same document URL
+            if (!std.mem.eql(u8, stripFragment(resolved), stripFragment(current))) {
+                return error.InvalidUrl;
+            }
         },
         .worker => {},
     }
     return resolved;
 }
 
+fn stripFragment(url: []const u8) []const u8 {
+    return url[0 .. std.mem.indexOfScalar(u8, url, '#') orelse url.len];
+}
+
 fn matchCookies(
     exec: *const Execution,
     name: ?[]const u8,
@@ -345,20 +371,30 @@ fn matchCookies(
         .path = URL.getPathname(url_resolved),
         .secure = URL.isSecure(url_resolved),
     };
-    if (target.host.len == 0) return error.SecurityError;
+    if (target.host.len == 0) {
+        return error.SecurityError;
+    }
 
     session.cookie_jar.removeExpired(null);
 
+    // Cookie names are normalized. Apply the same normalization to the input
+    // we're matching
+    const normalized_name: ?[]const u8 = if (name) |n| std.mem.trim(u8, n, " \t") else null;
+
     var items: std.ArrayList(CookieListItem) = .empty;
     for (session.cookie_jar.cookies.items) |*cookie| {
         // CookieStore exposes only cookies that script would see for the
         // current document. HttpOnly cookies stay hidden.
-        if (!cookie.appliesTo(&target, true, true, false)) continue;
-        if (name) |n| {
-            if (!std.mem.eql(u8, cookie.name, n)) continue;
+        if (cookie.appliesTo(&target, true, true, false) == false) {
+            continue;
+        }
+        if (normalized_name) |n| {
+            if (std.mem.eql(u8, cookie.name, n) == false) {
+                continue;
+            }
         }
 
-        try items.append(exec.call_arena, .{
+        try items.append(exec.local_arena, .{
             .name = String.wrap(cookie.name),
             .value = String.wrap(cookie.value),
             .domain = if (cookie.domain.len > 0 and cookie.domain[0] == '.')
@@ -390,6 +426,16 @@ fn storeCookie(exec: *const Execution, init_: CookieInit, is_delete: bool) !void
     init.name = std.mem.trim(u8, init.name, " \t");
     init.value = std.mem.trim(u8, init.value, " \t");
 
+    if (init.maxAge) |max_age| {
+        if (init.expires != null) {
+            // maxAge and expires are mutually exclusive
+            return error.InvalidArgument;
+        }
+        // convert to absolute time, so the rest of the code is shared for
+        // maxAge and expires.
+        init.expires = (@as(f64, @floatFromInt(std.time.timestamp())) + max_age) * 1000.0;
+    }
+
     // delete() may legitimately target a nameless cookie — its value is always empty.
     if (!is_delete and init.name.len == 0) {
         if (init.value.len == 0) {