Clear the circular redirection history of a redirect chain once it is finished

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
 

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
 

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

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<string, ResponseInterface> $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(