redirection to different domain must not keep previous port

Author: dbu

CHANGELOG.md

@@ -4,7 +4,8 @@
 
 ### Fixed
 
-- Fixes false positive circular detection in RedirectPlugin in cases when target location does not contain path
+- [RedirectPlugin] Fixed handling of redirection to different domain with default port
+- [RedirectPlugin] Fixed false positive circular detection in RedirectPlugin in cases when target location does not contain path
 
 ## 2.5.0 - 2021-11-26
 

src/Plugin/RedirectPlugin.php

@@ -231,8 +231,29 @@ private function createUri(ResponseInterface $redirectResponse, RequestInterface
         }
 
         $uri = $originalRequest->getUri();
+
+        // Redirections can either use an absolute uri or a relative reference https://www.rfc-editor.org/rfc/rfc3986#section-4.2
+        // If relative, we need to check if we have an absolute path or not
+
+        $path = array_key_exists('path', $parsedLocation) ? $parsedLocation['path'] : '';
+        if (!array_key_exists('host', $parsedLocation) && '/' !== $location[0]) {
+            // this is a relative-path reference
+            $originalPath = $uri->getPath();
+            if ('' === $path) {
+                $path = $originalPath;
+            } elseif (($pos = strrpos($originalPath, '/')) !== false) {
+                $path = substr($originalPath, 0, $pos+1).$path;
+            } else {
+                $path = '/'.$path;
+            }
+            /* replace '/./' or '/foo/../' with '/' */
+            $re = ['#(/\./)#', '#/(?!\.\.)[^/]+/\.\./#'];
+            for( $n=1; $n>0; $path=preg_replace( $re, '/', $path, -1, $n ) ) {
+                // keep looping until all is resolved
+            }
+        }
         $uri = $uri
-            ->withPath(array_key_exists('path', $parsedLocation) ? $parsedLocation['path'] : '')
+            ->withPath($path)
             ->withQuery(array_key_exists('query', $parsedLocation) ? $parsedLocation['query'] : '')
             ->withFragment(array_key_exists('fragment', $parsedLocation) ? $parsedLocation['fragment'] : '')
         ;
@@ -247,6 +268,8 @@ private function createUri(ResponseInterface $redirectResponse, RequestInterface
 
         if (array_key_exists('port', $parsedLocation)) {
             $uri = $uri->withPort($parsedLocation['port']);
+        } elseif (array_key_exists('host', $parsedLocation)) {
+            $uri = $uri->withPort(null);
         }
 
         return $uri;

tests/Plugin/RedirectPluginTest.php

@@ -26,10 +26,26 @@ function () {}
         )->wait();
     }
 
+    public function provideRedirections(): array
+    {
+        return [
+            'no path on target' => ["https://example.com/path?query=value", "https://example.com?query=value", "https://example.com?query=value"],
+            'root path on target' => ["https://example.com/path?query=value", "https://example.com/?query=value", "https://example.com/?query=value"],
+            'redirect to query' => ["https://example.com", "https://example.com?query=value", "https://example.com?query=value"],
+            'redirect to different domain without port' => ["https://example.com:8000", "https://foo.com?query=value", "https://foo.com?query=value"],
+            'network-path redirect, preserve scheme' => ["https://example.com:8000", "//foo.com/path?query=value", "https://foo.com/path?query=value"],
+            'absolute-path redirect, preserve host' => ["https://example.com:8000", "/path?query=value", "https://example.com:8000/path?query=value"],
+            'relative-path redirect, append' => ["https://example.com:8000/path/", "sub/path?query=value", "https://example.com:8000/path/sub/path?query=value"],
+            'relative-path on non-folder' => ["https://example.com:8000/path/foo", "sub/path?query=value", "https://example.com:8000/path/sub/path?query=value"],
+            'relative-path moving up' => ["https://example.com:8000/path/", "../other?query=value", "https://example.com:8000/other?query=value"],
+            'relative-path with ./' => ["https://example.com:8000/path/", "./other?query=value", "https://example.com:8000/path/other?query=value"],
+            'relative-path with //' => ["https://example.com:8000/path/", "other//sub?query=value", "https://example.com:8000/path/other//sub?query=value"],
+            'relative-path redirect with only query' => ["https://example.com:8000/path", "?query=value", "https://example.com:8000/path?query=value"],
+       ];
+    }
+
     /**
-     * @testWith ["https://example.com/path?query=value", "https://example.com?query=value", "https://example.com?query=value"]
-     *           ["https://example.com/path?query=value", "https://example.com/?query=value", "https://example.com/?query=value"]
-     *           ["https://example.com", "https://example.com?query=value", "https://example.com?query=value"]
+     * @dataProvider provideRedirections
      */
     public function testTargetUriMappingFromLocationHeader(string $originalUri, string $locationUri, string $targetUri): void
     {