Ensure that redirects are complete

This change ensures that the request is redirected as is. Previously, if a request was to the old domain, it was redirected to the new domain, dropping any path, port, and query strings. This change just updates the request's host from the old to the new domain, keeping the remaining request parameters.
settermjd · Jan 9, 2025 · b1d921a · b1d921a
1 parent 7e42bae
commit b1d921a
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 62 deletions.
diff --git a/src/Middleware/RedirectToNewDomainMiddleware.php b/src/Middleware/RedirectToNewDomainMiddleware.php
@@ -32,12 +32,13 @@ public function __construct(
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         $this->logger
-            ?->debug(sprintf("received request from host: %s", $request->getUri()->getHost()));
+            ?->debug(sprintf("received request from host: %s", (string) $request->getUri()));
 
         if ($request->getUri()->getHost() === $this->oldDomain) {
+            $newRequestUri = $request->getUri()->withHost($this->newDomain);
             $this->logger
-                ?->debug(sprintf("redirecting to: %s", $this->newDomain));
-            return new RedirectResponse($this->newDomain, 301);
+                ?->debug(sprintf("redirecting to: %s", $newRequestUri));
+            return new RedirectResponse($newRequestUri, 301);
         }
 
         return $handler->handle($request);

diff --git a/test/Middleware/RedirectToNewDomainMiddlewareTest.php b/test/Middleware/RedirectToNewDomainMiddlewareTest.php
@@ -6,10 +6,10 @@
 
 use Laminas\Diactoros\Response\EmptyResponse;
 use Laminas\Diactoros\Response\RedirectResponse;
+use Laminas\Diactoros\Uri;
 use PHPUnit\Framework\Attributes\TestWith;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\ServerRequestInterface;
-use Psr\Http\Message\UriInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use Psr\Log\LoggerInterface;
 use Settermjd\Middleware\RedirectToNewDomainMiddleware;
@@ -18,42 +18,63 @@
 
 class RedirectToNewDomainMiddlewareTest extends TestCase
 {
-    #[TestWith(['deploywithdockercompose.com'])]
-    public function testRedirectsToNewDomainIfRequestComesFromOldDomain(string $requestedDomain): void
-    {
+    private string $oldDomain = "deploywithdockercompose.com";
+    private string $newDomain = "deploywithdockercompose.webdevwithmatt.com";
+
+    #[TestWith(
+        [
+            "https://deploywithdockercompose.com", // The original request made
+            "https://deploywithdockercompose.webdevwithmatt.com", // The redirected request
+        ],
+        "Test redirecting from one domain to another"
+    )]
+    #[TestWith(
+        [
+            "https://deploywithdockercompose.com/api/ping",
+            "https://deploywithdockercompose.webdevwithmatt.com/api/ping",
+        ],
+        "Test redirecting from one domain to another with a path"
+    )]
+    #[TestWith(
+        [
+            "https://deploywithdockercompose.com?display=dark",
+            "https://deploywithdockercompose.webdevwithmatt.com?display=dark",
+        ],
+        "Test redirecting from one domain to another with query parameters"
+    )]
+    #[TestWith(
+        [
+            "https://deploywithdockercompose.com/api/ping?display=dark",
+            "https://deploywithdockercompose.webdevwithmatt.com/api/ping?display=dark",
+        ],
+        "Test redirecting from one domain to another with a path and query parameters"
+    )]
+    public function testRedirectsToNewDomainIfRequestComesFromOldDomain(
+        string $originalRequest,
+        string $newRequest
+    ): void {
         $invokedCount = $this->exactly(2);
         $log          = $this->createMock(LoggerInterface::class);
         $log
             ->expects($invokedCount)
             ->method("debug")
             ->willReturnCallback(
-                function ($parameters) use ($invokedCount, $requestedDomain) {
+                function ($parameters) use ($invokedCount, $originalRequest, $newRequest) {
                     if ($invokedCount->numberOfInvocations() === 1) {
-                        $this->assertSame(sprintf("received request from host: %s", $requestedDomain), $parameters);
+                        $this->assertSame(sprintf("received request from host: %s", $originalRequest), $parameters);
                     }
                     if ($invokedCount->numberOfInvocations() === 2) {
                         $this->assertSame(
-                            sprintf(
-                                "redirecting to: %s",
-                                "https://deploywithdockercompose.webdevwithmatt.com/"
-                            ),
+                            sprintf("redirecting to: %s", $newRequest),
                             $parameters
                         );
                     }
                 }
             );
 
-        $middleware = new RedirectToNewDomainMiddleware(
-            "deploywithdockercompose.com",
-            "https://deploywithdockercompose.webdevwithmatt.com/",
-            $log
-        );
+        $middleware = new RedirectToNewDomainMiddleware($this->oldDomain, $this->newDomain, $log);
 
-        $uri = $this->createMock(UriInterface::class);
-        $uri
-            ->expects($this->atMost(2))
-            ->method("getHost")
-            ->willReturn($requestedDomain);
+        $uri      = new Uri($originalRequest);
         $request  = $this->createConfiguredStub(
             ServerRequestInterface::class,
             [
@@ -63,27 +84,20 @@ function ($parameters) use ($invokedCount, $requestedDomain) {
         $response = $middleware->process($request, $this->createMock(RequestHandlerInterface::class));
 
         self::assertInstanceOf(RedirectResponse::class, $response);
-        self::assertSame("https://deploywithdockercompose.webdevwithmatt.com/", $response->getHeaderLine("location"));
+        self::assertSame($newRequest, $response->getHeaderLine("location"));
         self::assertSame(301, $response->getStatusCode());
     }
 
-    #[TestWith(['deploywithdockercompose.webdevwithmatt.com'])]
-    #[TestWith(['webdevwithmatt.com'])]
-    #[TestWith(['example.com'])]
-    #[TestWith(['localhost'])]
-    #[TestWith(['example.org'])]
-    public function testDoesNotRedirectToNewDomainIfRequestDoesNotComeFromOldDomain(string $requestedDomain): void
+    #[TestWith(['https://deploywithdockercompose.webdevwithmatt.com'])]
+    #[TestWith(['https://webdevwithmatt.com'])]
+    #[TestWith(['https://example.com'])]
+    #[TestWith(['https://localhost'])]
+    #[TestWith(['https://example.org'])]
+    public function testDoesNotRedirectToNewDomainIfRequestDoesNotComeFromOldDomain(string $originalRequest): void
     {
-        $middleware = new RedirectToNewDomainMiddleware(
-            "deploywithdockercompose.com",
-            "https://deploywithdockercompose.webdevwithmatt.com/"
-        );
+        $middleware = new RedirectToNewDomainMiddleware($this->oldDomain, $this->newDomain);
 
-        $uri = $this->createMock(UriInterface::class);
-        $uri
-            ->expects($this->atMost(2))
-            ->method("getHost")
-            ->willReturn($requestedDomain);
+        $uri     = new Uri($originalRequest);
         $request = $this->createConfiguredStub(
             ServerRequestInterface::class,
             [
@@ -102,38 +116,27 @@ public function testDoesNotRedirectToNewDomainIfRequestDoesNotComeFromOldDomain(
         self::assertSame(204, $response->getStatusCode());
     }
 
-    #[TestWith(['deploywithdockercompose.webdevwithmatt.com'])]
-    #[TestWith(['webdevwithmatt.com'])]
-    #[TestWith(['example.com'])]
-    #[TestWith(['localhost'])]
-    #[TestWith(['example.org'])]
-    public function testLogsRequestedDomainIfLoggerIsPresent(string $requestedDomain): void
+    #[TestWith(['https://deploywithdockercompose.webdevwithmatt.com'])]
+    #[TestWith(['https://webdevwithmatt.com'])]
+    #[TestWith(['https://example.com'])]
+    #[TestWith(['https://localhost'])]
+    #[TestWith(['https://example.org'])]
+    public function testLogsRequestedDomainIfLoggerIsPresent(string $originalRequest): void
     {
         $log = $this->createMock(LoggerInterface::class);
         $log
             ->expects($this->once())
             ->method("debug")
             ->with(
-                sprintf("received request from host: %s", $requestedDomain)
+                sprintf("received request from host: %s", $originalRequest)
             );
 
-        $middleware = new RedirectToNewDomainMiddleware(
-            "deploywithdockercompose.com",
-            "https://deploywithdockercompose.webdevwithmatt.com/",
-            $log
-        );
+        $middleware = new RedirectToNewDomainMiddleware($this->oldDomain, $this->newDomain, $log);
 
-        $uri = $this->createMock(UriInterface::class);
-        $uri
-            ->expects($this->atMost(2))
-            ->method("getHost")
-            ->willReturn($requestedDomain);
-        $request = $this->createConfiguredStub(
-            ServerRequestInterface::class,
-            [
-                "getUri" => $uri,
-            ]
-        );
+        $uri     = new Uri($originalRequest);
+        $request = $this->createConfiguredStub(ServerRequestInterface::class, [
+            "getUri" => $uri,
+        ]);
         $handler = $this->createMock(RequestHandlerInterface::class);
         $handler
             ->expects($this->once())