CORS Implementation by mookums · Pull Request #3002 · lightpanda-io/browser

mookums · 2026-07-20T13:45:11Z

This adds a CORS implementation as experimental feature for now.
Enable it with --experimental-feature cors

fix #2015

mookums · 2026-07-24T14:02:34Z

Not ready yet, just want CI to start running on this branch.

karlseguin

I ran the demo tests, and there wasn't 1 OPTIONS request.
The singleflight is nice, but it might have been a mistake to land it with this PR. It introduces bugs not related to CORS. I'd consider landing Singlelight first.
Without caching CORS responses, this seems like it could hurt performance more than initially anticipated.

karlseguin · 2026-07-29T01:17:43Z

    return raw[0..authority_end];
 }

+pub fn isSameOrigin(url: [:0]const u8, origin: [:0]const u8) bool {


Frame.isSameOrigin should use this (there's a comment there that assumes Protocols are equal, but I'm not sure why that's true).

karlseguin · 2026-07-29T01:27:13Z

            owned.credentials = try arena.dupeZ(u8, c);
        }

+        if (req.authored_headers.len > 0) {


The callsite dupes these in call_arena, and then we dupe it again in transfer.arena. We should rework headers before landing this.Request.headers is a micro-optimization which put the headers in a curl-friendly list from the get go. It was always questionable, and if we're going to iterate + dupe + iterate + dupe, I think it would both be cleaner and more efficient to:
1 - A single header list
2 - The header has a authored: bool flag
3 - The caller sets the headers after newRequest is called so that transfer.arena is available

Done in #3118

I want to work on referrer policy, and I'll have the same issue.

karlseguin · 2026-07-29T01:33:11Z

+        .GET, .HEAD => false,
+        else => true,
+    };
+    if (!is_cross_origin and !is_unsafe_method) {


There's two different types of double negative going on here. They can all be eliminated.

const is_same_origin = URL.isSameOrigin(req.cookie_origin, req.url); const is_safe_method = switch (req.method) { .GET, .HEAD => true, else => false, }; if (is_same_origin and is_safe_method) { return; }

karlseguin · 2026-07-29T01:37:33Z

+    switch (res) {
+        .queued => {
+            // joined inflight fetch so release it.
+            client.arena_pool.release(arena);


I think there's a UAF bug in SingleFlight (1) related to this, BUT, as-is there's no reason to even acquire the arena in this case.

(1) Commented in SingleFlight.enter

karlseguin · 2026-07-29T02:24:45Z

+const lp = @import("lightpanda");
+
+const http = @import("http.zig");
+const Request = @import("../browser/webapi/net/Request.zig");


Adding headers to an HTTP request was a bit awkward due to my desire to avoid having an intermediate representation (e.g. an ArrayList(Header)). Going straight to a curl slist avoids double-copying the headers (first to Zig, then to curl). But the CORS work (lightpanda-io#3002) showcases that this micro-optimization simply isn't worth it, since it needs that intermediate representation anyways. And, this change isn't just for CORS. Headers have been a silly pain in the past like unclear ownership, and messy APIs used in _a lot_ of places (WebBotAuth, WebSocket, Fetch, ...) This new approach stores headers on the transfer in an ArrayList. The API is: ``` const transfer = try client.newRequest(.{...}, owner); { errdefer transfer.deinit(); try transfer.addHeader("Over", "9000", .{}); } try transfer.submit(); ``` This: 1 - Eliminates ambiguity about errdefer cleanup responsibility 2 - Eliminates a bunch of stringZ concat that Frame, Config, CDP were doing 3 - Transfer.arena is now available for headers

mookums · 2026-08-18T21:47:53Z

Redid a lot of the implementation of the CorsGate. I'm happier with this one. I purposefully omitted the CorsStore that I wrote, from my testing (mostly just running the WPT tests or the integration tests against it) i rarely if ever got a proper CORS cache hit with the store.

It is currently disabled by default behind an --obey-cors flag.

krichprollsch · 2026-08-19T16:08:00Z

As far as I can see, chrome sends the pre-flight request each time... It doesn't use any cache.

karlseguin · 2026-08-19T23:28:42Z

As far as I can see, chrome sends the pre-flight request each time... It doesn't use any cache.

It does if the response includes a access-control-max-age header

karlseguin · 2026-08-21T00:40:57Z

    \\  --obey-robots
    \\          Fetches and obeys robots.txt of the target page.
    \\          Defaults to false.
+    \\  --obey-cors


Should be above --obey-robots

karlseguin · 2026-08-21T01:14:58Z

i started to review / test this, but I ran into a couple show-stoppers.

It panics on a preflight deny (trivial to fix, but makes it impossible to test)
It seems to block things it shouldn't? I expect to see 0 on virtually every website.

zig build run -- fetch --obey-cors "https://www.theverge.com/"  2>&1 | grep "CorsBlocked" | wc -l
24

Doesn't seem right.

mookums · 2026-08-21T05:25:52Z

2 is fixed. It was blocking some scripts that are meant to be processed as no-cors by default, mostly analytics and tracking stuff.

w.r.t 1, I'm not sure how it would panic on a preflight deny? It just sets allowed as false and will resolve following the same path as preflight accepts and just resolve the requests as blocked.

mookums · 2026-08-21T05:29:58Z

as a follow up, I ran it against the /cors/preflight-failure.htm WPT test that does preflight failures and I got no panics

karlseguin · 2026-08-21T05:38:52Z

use the wptrunner in the demo project to run /cors/. It'll crash.

mookums · 2026-08-21T15:14:51Z

I always run it with --log-filter-scopes -all,+cors so never came up for me.

karlseguin

A redirect to a different origin doesn't trigger a CORS check. It might be worth looking at how redirects are handled holistically, or maybe there's a simple / clean fix for CORS.

But, for now, I'm less worried about places where CORS doesn't block where it should, but rather places where CORS blocks when it shouldn't (since that can break existing use-cases). There's ~4 different ways that can happen, all commented inline.

karlseguin · 2026-08-24T08:47:29Z

                .url = "http://example.com/",
                .cookie_jar = null,
                .cookie_origin = "",
+                .origin = "",


A bit more correct to pass null for all these tests. I don't think you'll ever get a "" origin in the code.

karlseguin · 2026-08-24T09:07:36Z

    try transfer.materializeResponse(msg.conn, .{});

+    // Validate the headers for the response with CORS.
+    if (transfer._cors_cross_origin) {


There are some references online to how this behaves with a 304, but I couldn't find anything official in the spec. There's at least a few mentions that a 304 doesn't have to (or maybe shouldn't) include CORS headers and they should come from the cached value. /cors/304.htm is currently 0/4 which tells me we're doing it wrong.

Yeah, the WPT tests also seem to want it to resolve the CORS status from the HTTP cache.

karlseguin · 2026-08-24T09:30:54Z

No caching

krichprollsch · 2026-08-28T08:58:38Z

Shouldn't HttpClient.Rquest.getCookieString check credentials_mode before writing cookies?

EDIT: it seems we also should check credentials_mode when we handle Set-Cookie from response (into materializeResponse)

mookums force-pushed the cors-impl branch 2 times, most recently from ab8be3b to 235f9ea Compare July 24, 2026 14:01

mookums marked this pull request as ready for review July 24, 2026 14:02

karlseguin reviewed Jul 29, 2026

View reviewed changes

karlseguin mentioned this pull request Aug 3, 2026

refactor: rework how httpclient headers work #3118

Merged

mookums mentioned this pull request Aug 13, 2026

URL.isSameOrigin #3191

Merged

mookums force-pushed the cors-impl branch 2 times, most recently from 5e1c6dd to 4eb88c0 Compare August 18, 2026 20:02

mookums mentioned this pull request Aug 20, 2026

XHR Sync Request #3240

Merged

karlseguin reviewed Aug 21, 2026

View reviewed changes

mookums force-pushed the cors-impl branch from 2555ad0 to 97cba0e Compare August 21, 2026 15:35

karlseguin reviewed Aug 24, 2026

View reviewed changes

mookums force-pushed the cors-impl branch from 97cba0e to 22e9c1f Compare August 25, 2026 21:42

krichprollsch reviewed Aug 28, 2026

View reviewed changes

Comment thread src/help.zon Outdated

mookums force-pushed the cors-impl branch from 76f33ec to ee6a1fa Compare August 28, 2026 15:04

karlseguin reviewed Aug 29, 2026

View reviewed changes

Comment thread src/browser/webapi/net/Fetch.zig Outdated

Comment thread src/browser/webapi/WorkerGlobalScope.zig Outdated

mookums force-pushed the cors-impl branch 2 times, most recently from 3275163 to 7dc3257 Compare August 31, 2026 14:15

mookums added 25 commits September 4, 2026 06:59

safelisted methods always pass in CORS

89df63e

cors check before cache check

feebb88

proper safelist checking for CORS headers

8ee7144

authorization header doesnt accept wildcard

2b06583

more comprehensive cors singleflight key

b0fffe6

non-default credentials and request mode

d1f4605

better no cors opaque behavior

12ed38d

add modes to the tests

1fe8456

use experimental features flag instead of obey cors

15d2790

add CORS metrics

addba12

include modes on EchoDriver

c3a71ef

use same cors guard for fetch body as headers

e195886

use no_cors for importScript

6002dfe

fix test running

8a8bb38

only check cross origin mode on obey cors

827d0f0

fetch can't be navigate mode

db9ccfb

add origin conditionally on no_cors

868882b

add credentials to preflight key for cors

1f9342e

properly use include on default corsSettings and image load

23c1c10

use credentials_mode instead of cookie bool

0aeba82

collapse isCrossOriginModeAllowed check in pipeline

0610d5e

enforce cors response on redirects as well

5451838

origin is tainted on cross origin redirects

34b743f

make test client buf larger to accomodate larger metrics

6bf77ad

avoid total capacity reservation on opaque fetch

620df37

mookums force-pushed the cors-impl branch from 47adcda to 620df37 Compare September 4, 2026 14:06

krichprollsch approved these changes Sep 4, 2026

View reviewed changes

krichprollsch merged commit 2f76e19 into main Sep 4, 2026
26 checks passed

krichprollsch deleted the cors-impl branch September 4, 2026 16:26

github-actions Bot locked and limited conversation to collaborators Sep 4, 2026

Uh oh!

Conversation

mookums commented Jul 20, 2026 • edited by krichprollsch Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mookums commented Jul 24, 2026

Uh oh!

karlseguin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mookums commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krichprollsch commented Aug 19, 2026

Uh oh!

karlseguin commented Aug 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karlseguin commented Aug 21, 2026

Uh oh!

mookums commented Aug 21, 2026

Uh oh!

mookums commented Aug 21, 2026

Uh oh!

karlseguin commented Aug 21, 2026

Uh oh!

mookums commented Aug 21, 2026

Uh oh!

karlseguin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

karlseguin commented Aug 24, 2026

Uh oh!

Uh oh!

krichprollsch commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mookums commented Jul 20, 2026 •

edited by krichprollsch

Loading

mookums commented Aug 18, 2026 •

edited

Loading

krichprollsch commented Aug 28, 2026 •

edited

Loading