From 482a1e93e305ad04cc8207b4df9c2c1be45c18e9 Mon Sep 17 00:00:00 2001 From: Pascal CESCON - Amoifr Date: Thu, 8 Oct 2026 10:00:34 +0200 Subject: [PATCH 1/2] Cover redirect chains that reuse the identifier of a finished chain --- tests/Plugin/RedirectPluginTest.php | 67 +++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/Plugin/RedirectPluginTest.php b/tests/Plugin/RedirectPluginTest.php index 92072ad..23d57b6 100644 --- a/tests/Plugin/RedirectPluginTest.php +++ b/tests/Plugin/RedirectPluginTest.php @@ -28,6 +28,73 @@ function () {} )->wait(); } + public function testCircularDetectionAcrossSeveralRedirections(): void + { + $responses = [ + 'https://example.com/a' => new Response(302, ['Location' => 'https://example.com/b']), + 'https://example.com/b' => new Response(302, ['Location' => 'https://example.com/a']), + ]; + $first = $this->createFirst(new RedirectPlugin(), $responses); + + $this->expectException(CircularRedirectionException::class); + $first(new Request('GET', 'https://example.com/a'))->wait(); + } + + public function testFinishedChainDoesNotLeakIntoTheNextOneWithTheSameFirstCallable(): void + { + $responses = [ + 'https://example.com/a' => new Response(302, ['Location' => 'https://example.com/b']), + 'https://example.com/b' => new Response(200), + 'https://example.com/c' => new Response(302, ['Location' => 'https://example.com/a']), + ]; + // the chain identifier is derived from $first, so reusing the instance simulates a new + // chain that gets the identifier of a finished one + $first = $this->createFirst(new RedirectPlugin(), $responses); + + $this->assertSame(200, $first(new Request('GET', 'https://example.com/a'))->wait()->getStatusCode()); + $this->assertSame(200, $first(new Request('GET', 'https://example.com/c'))->wait()->getStatusCode()); + } + + public function testChainStoppedByACircularRedirectionDoesNotLeakIntoTheNextOne(): void + { + $responses = [ + 'https://example.com/a' => new Response(302, ['Location' => 'https://example.com/b']), + 'https://example.com/b' => new Response(302, ['Location' => 'https://example.com/a']), + ]; + $first = $this->createFirst(new RedirectPlugin(), $responses); + + try { + $first(new Request('GET', 'https://example.com/a'))->wait(); + $this->fail('The first chain should be detected as circular.'); + } catch (CircularRedirectionException $e) { + } + + $responses = [ + 'https://example.com/c' => new Response(302, ['Location' => 'https://example.com/a']), + 'https://example.com/a' => new Response(200), + ]; + + $this->assertSame(200, $first(new Request('GET', 'https://example.com/c'))->wait()->getStatusCode()); + } + + /** + * Builds the $first callable of a plugin chain made of the redirect plugin only, like PluginClient does. + * + * @param array $responses Response for each requested URI + */ + private function createFirst(RedirectPlugin $plugin, array &$responses): callable + { + $next = function (RequestInterface $request) use (&$responses) { + return new FulfilledPromise($responses[(string) $request->getUri()]); + }; + + $first = function (RequestInterface $request) use ($plugin, $next, &$first) { + return $plugin->handleRequest($request, $next, $first); + }; + + return $first; + } + public function testPostGetDropRequestBody(): void { $response = (new RedirectPlugin())->handleRequest( From e5428a7e6e972929b0da213807aa76054161bc52 Mon Sep 17 00:00:00 2001 From: Pascal CESCON - Amoifr Date: Thu, 8 Oct 2026 10:00:34 +0200 Subject: [PATCH 2/2] Clear the circular redirection history of a redirect chain once it is finished --- CHANGELOG.md | 2 ++ phpstan.neon.dist | 2 +- src/Plugin/RedirectPlugin.php | 34 +++++++++++++++++++++------------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce273e1..a990ba1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## 2.7.4 - unreleased - Fix `spl_object_hash` deprecation on PHP 8.6. +- Fix `RedirectPlugin` reporting a circular redirection when a new request reuses the identifier of a finished one, + and stop keeping the URLs of finished redirect chains in memory. ## 2.7.3 - 2025-11-29 diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 265743b..48455be 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -65,7 +65,7 @@ parameters: path: src/Plugin/RedirectPlugin.php - - message: "#^Method Psr\\\\Http\\\\Message\\\\StreamFactoryInterface@anonymous\/Plugin\/RedirectPlugin.php:221\\:\\:createStream\\(\\) should return Psr\\\\Http\\\\Message\\\\StreamInterface but returns mixed\\.$#" + message: "#^Method Psr\\\\Http\\\\Message\\\\StreamFactoryInterface@anonymous\/Plugin\/RedirectPlugin.php:229\\:\\:createStream\\(\\) should return Psr\\\\Http\\\\Message\\\\StreamInterface but returns mixed\\.$#" count: 1 path: src/Plugin/RedirectPlugin.php diff --git a/src/Plugin/RedirectPlugin.php b/src/Plugin/RedirectPlugin.php index b2c1d15..b33cf6e 100644 --- a/src/Plugin/RedirectPlugin.php +++ b/src/Plugin/RedirectPlugin.php @@ -180,25 +180,33 @@ public function handleRequest(RequestInterface $request, callable $next, callabl $redirectRequest = $this->buildRedirectRequest($request, $uri, $statusCode); $chainIdentifier = \PHP_VERSION_ID < 70200 ? spl_object_hash((object) $first) : (string) spl_object_id((object) $first); - if (!array_key_exists($chainIdentifier, $this->circularDetection)) { + // the identifier can be reused once the chain is finished, so the chain that starts it also clears it + $startsChain = !array_key_exists($chainIdentifier, $this->circularDetection); + if ($startsChain) { $this->circularDetection[$chainIdentifier] = []; } - $this->circularDetection[$chainIdentifier][] = (string) $request->getUri(); + try { + $this->circularDetection[$chainIdentifier][] = (string) $request->getUri(); - if (in_array((string) $redirectRequest->getUri(), $this->circularDetection[$chainIdentifier], true)) { - throw new CircularRedirectionException('Circular redirection detected', $request, $response); - } + if (in_array((string) $redirectRequest->getUri(), $this->circularDetection[$chainIdentifier], true)) { + throw new CircularRedirectionException('Circular redirection detected', $request, $response); + } - if ($this->redirectCodes[$statusCode]['permanent']) { - $this->redirectStorage[(string) $request->getUri()] = [ - 'uri' => $uri, - 'status' => $statusCode, - ]; - } + if ($this->redirectCodes[$statusCode]['permanent']) { + $this->redirectStorage[(string) $request->getUri()] = [ + 'uri' => $uri, + 'status' => $statusCode, + ]; + } - // Call redirect request synchronously - return $first($redirectRequest)->wait(); + // Call redirect request synchronously + return $first($redirectRequest)->wait(); + } finally { + if ($startsChain) { + unset($this->circularDetection[$chainIdentifier]); + } + } }); }