Skip to content

fix: don't mutate the default http options when merging request options - #210

Merged
kbond merged 3 commits into
zenstruck:1.xfrom
Amoifr:fix-191-immutable-default-http-options
Aug 29, 2026
Merged

fix: don't mutate the default http options when merging request options#210
kbond merged 3 commits into
zenstruck:1.xfrom
Amoifr:fix-191-immutable-default-http-options

Conversation

@Amoifr

@Amoifr Amoifr commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #191.

HttpOptions::merge() mutates $this, so merging the per-request options into $this->defaultHttpOptions permanently rewrote the defaults:

$browser = $this->browser()->setDefaultHttpOptions(['headers' => ['x-foo' => 'bar']]);

$browser->get('/', ['headers' => ['x-foo' => 'baz']]);

$browser->get('/'); // still sends "x-foo: baz"

Cloning before merging, as suggested in the issue, keeps the defaults intact. HttpOptions only holds a single array, so a shallow clone fully isolates the state, and clone preserves the concrete class for HttpOptions subclasses.

The regression test lives in KernelBrowserTests, so it covers both the kernel and the HTTP browser; it fails on 1.x without the fix.

I left BrowserKitDriver::request() alone even though it merges into the caller's instance too: serverParameters is only ever populated through the Mink driver methods, never through the Browser API, so the merge is a no-op there. Happy to include it if you would rather have it defensive.

Making HttpOptions immutable (plus the #[NoDiscard] idea) is the 2.x half of the issue, so I kept it out of this one.

@kbond kbond left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I tacked on a couple of commits to cover the other options merge() leaks (query, json, ajax) and to give BrowserKitDriver the same treatment, thanks! 🎉

@kbond
kbond merged commit ba0876d into zenstruck:1.x Aug 29, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Passing options to the request mutates "default options"

2 participants