From 293c540e1ddc326d36cd134cc0e577360722953b Mon Sep 17 00:00:00 2001 From: JonPurvis Date: Sat, 25 Apr 2026 17:43:56 +0100 Subject: [PATCH 1/3] fix shallow clone request stores --- src/Helpers/MiddlewarePipeline.php | 10 ++++ src/Http/Request.php | 30 ++++++++++ tests/Unit/RequestCloneTest.php | 92 ++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+) create mode 100644 tests/Unit/RequestCloneTest.php diff --git a/src/Helpers/MiddlewarePipeline.php b/src/Helpers/MiddlewarePipeline.php index 06fe94f5..df56675f 100644 --- a/src/Helpers/MiddlewarePipeline.php +++ b/src/Helpers/MiddlewarePipeline.php @@ -37,6 +37,16 @@ public function __construct() $this->fatalPipeline = new Pipeline; } + /** + * Deep-clone internal pipelines so cloned requests do not share pipe state. + */ + public function __clone(): void + { + $this->requestPipeline = clone $this->requestPipeline; + $this->responsePipeline = clone $this->responsePipeline; + $this->fatalPipeline = clone $this->fatalPipeline; + } + /** * Add a middleware before the request is sent * diff --git a/src/Http/Request.php b/src/Http/Request.php index 6f145133..adadf989 100644 --- a/src/Http/Request.php +++ b/src/Http/Request.php @@ -59,6 +59,36 @@ public function getMethod(): Method return $this->method; } + /** + * Ensure cloned requests do not share mutable object state (query, headers, etc.). + * + * Without this, a shallow clone reuses the same {@see \Saloon\Repositories\ArrayStore} + * instances when they were initialized before cloning, which breaks concurrent pools + * (e.g. paginated requests mutating the same query bag). + */ + public function __clone(): void + { + if (isset($this->query)) { + $this->query = clone $this->query; + } + + if (isset($this->headers)) { + $this->headers = clone $this->headers; + } + + if (isset($this->config)) { + $this->config = clone $this->config; + } + + if (isset($this->delay)) { + $this->delay = clone $this->delay; + } + + if (isset($this->middlewarePipeline)) { + $this->middlewarePipeline = clone $this->middlewarePipeline; + } + } + /** * Define the endpoint for the request. */ diff --git a/tests/Unit/RequestCloneTest.php b/tests/Unit/RequestCloneTest.php new file mode 100644 index 00000000..545c893b --- /dev/null +++ b/tests/Unit/RequestCloneTest.php @@ -0,0 +1,92 @@ +query()->add('key', 'value'); + + $a = clone $original; + $b = clone $original; + + $a->query()->add('page', 1); + $b->query()->add('page', 2); + + expect($a->query()->get('page'))->toBe(1); + expect($b->query()->get('page'))->toBe(2); + expect($original->query()->get('page'))->toBeNull(); + expect($original->query()->get('key'))->toBe('value'); +}); + +test('cloning a request after headers config and delay are initialized gives independent stores', function () { + $original = new UserRequest; + $original->headers()->add('X-Test', 'one'); + $original->config()->add('timeout', 10); + $original->delay()->set(5); + + $clone = clone $original; + + $clone->headers()->add('X-Test', 'two'); + $clone->config()->add('timeout', 20); + $clone->delay()->set(15); + + expect($original->headers()->get('X-Test'))->toBe('one'); + expect($clone->headers()->get('X-Test'))->toBe('two'); + expect($original->config()->get('timeout'))->toBe(10); + expect($clone->config()->get('timeout'))->toBe(20); + expect($original->delay()->get())->toBe(5); + expect($clone->delay()->get())->toBe(15); +}); + +test('cloning a request after middleware is initialized gives independent pipelines', function () { + $original = new UserRequest; + $original->middleware()->onResponse(static fn (Response $response): Response => $response); + + $clone = clone $original; + + expect($original->middleware())->not->toBe($clone->middleware()); +}); + +test('concurrent pool sends with cloned requests do not share query mutation', function () { + $sequence = []; + for ($i = 0; $i < 10; $i++) { + $sequence[] = MockResponse::make(['ok' => true]); + } + + $connector = new TestConnector; + $connector->withMockClient(new MockClient($sequence)); + + $base = new UserRequest; + $base->query()->add('key', 'value'); + + $requests = []; + for ($i = 1; $i <= 10; $i++) { + $r = clone $base; + $r->query()->add('page', $i); + $requests[] = $r; + } + + $pagesSeen = []; + + $pool = $connector->pool($requests, 5); + $pool->withResponseHandler(function (Response $response) use (&$pagesSeen): void { + $pagesSeen[] = (int) $response->getRequest()->query()->get('page'); + }); + + $pool->send()->wait(); + + expect($pagesSeen)->toHaveCount(10); + expect(array_unique($pagesSeen))->toHaveCount(10); +}); From fe0be491049a8c3dbb5039a09364bf2ed642cfb1 Mon Sep 17 00:00:00 2001 From: JonPurvis Date: Sat, 25 Apr 2026 17:46:03 +0100 Subject: [PATCH 2/3] clean up test file --- tests/Unit/RequestCloneTest.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/Unit/RequestCloneTest.php b/tests/Unit/RequestCloneTest.php index 545c893b..bb98cc9b 100644 --- a/tests/Unit/RequestCloneTest.php +++ b/tests/Unit/RequestCloneTest.php @@ -8,12 +8,6 @@ use Saloon\Tests\Fixtures\Connectors\TestConnector; use Saloon\Tests\Fixtures\Requests\UserRequest; -/** - * Regression for https://github.com/saloonphp/saloon/issues/524 - * - * Shallow-cloned requests must not share ArrayStore / pipeline instances when those - * were initialized before cloning (e.g. async paginated pools). - */ test('cloning a request after query() is initialized gives independent query bags', function () { $original = new UserRequest; $original->query()->add('key', 'value'); From 90da5dbb3067db64c67d4e4955163c01920c2caa Mon Sep 17 00:00:00 2001 From: JonPurvis Date: Sat, 25 Apr 2026 18:09:56 +0100 Subject: [PATCH 3/3] run cs fixer --- tests/Unit/RequestCloneTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Unit/RequestCloneTest.php b/tests/Unit/RequestCloneTest.php index bb98cc9b..8c6a9648 100644 --- a/tests/Unit/RequestCloneTest.php +++ b/tests/Unit/RequestCloneTest.php @@ -2,11 +2,11 @@ declare(strict_types=1); +use Saloon\Http\Response; use Saloon\Http\Faking\MockClient; use Saloon\Http\Faking\MockResponse; -use Saloon\Http\Response; -use Saloon\Tests\Fixtures\Connectors\TestConnector; use Saloon\Tests\Fixtures\Requests\UserRequest; +use Saloon\Tests\Fixtures\Connectors\TestConnector; test('cloning a request after query() is initialized gives independent query bags', function () { $original = new UserRequest;