Skip to content

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

@nikophil

Description

@nikophil

There is a bug here because HttpOptions is not immutable, and HttpOptions::merge() mutates the default options, so this code will fail:

$browser = $this->browser()->setDefaultHttpOptions(['HTTP_SOME_HEADER' => 'xyz']);

$browser->get('/', ['HTTP_SOME_HEADER' => 'abc']);

$browser->get('/'); // 💥 uses "HTTP_SOME_HEADER' => 'abc'"

I propose that in 1.x we could clone the option:

    final public function request(string $method, string $url, $options = []): self
    {
        if ($this->defaultHttpOptions) {
-            $options = $this->defaultHttpOptions->merge($options);
+            $options = (clone $this->defaultHttpOptions)->merge($options);
        }

and I'd be in favor to make HttpOptions immutable in 2.x. This is a BC break, so this would be nice to do it now.

Also, we could make the BC break more explicit by adding the #[NoDiscard] attribute to all methods in HttpOptions, so at least in PHP 8.5 it would be very clear that this object is immutable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions