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]); + } + } }); } 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(