fix query parameter merging for urls without path part (respecting fragment part) (#203)

Author: faltjo

src/Http/Middleware/HandleNotFound.php

@@ -160,7 +160,16 @@ public function mergeQuery(Request $request, string $destination): string
         ksort($query);
 
         if (count($query)) {
-            $destination = $destination_parsed['path'] . '?' . http_build_query($query);
+            // make sure to preserve fragment if specified in destination
+            $fragment = isset($destination_parsed['fragment']) ? '#' . $destination_parsed['fragment'] : '';
+            if (($urlBeforeFragment = strstr($destination, '#', true)) === false) {
+                $urlBeforeFragment = $destination;
+            }
+            if (($urlBeforeQuery = strstr($urlBeforeFragment, '?', true)) === false) {
+                $urlBeforeQuery = $destination;
+            }
+
+            $destination = $urlBeforeQuery . '?' . http_build_query($query) . $fragment;
         }
 
         return $destination;

tests/Feature/Middleware/HandleNotFoundTest.php

@@ -260,6 +260,62 @@ public function it_merges_query_strings()
         $this->assertTrue($response->isRedirect(url('/fr?another=value&lang=fr')));
     }
 
+    /**
+     * @test
+     */
+    public function it_merges_query_strings_on_urls_without_path()
+    {
+        config()->set('statamic.redirect.preserve_query_strings', true);
+
+        Redirect::make()
+            ->source('/abc?another=value')
+            ->destination('?lang=fr')
+            ->save();
+
+        $response = $this->middleware->handle(Request::create('/abc', 'GET', ['another' => 'value']), function () {
+            return (new Response('', 404));
+        });
+
+        $this->assertTrue($response->isRedirect(url('?another=value&lang=fr')));
+    }
+
+    /**
+     * @test
+     */
+    public function it_merges_query_strings_on_urls_with_fragment()
+    {
+        config()->set('statamic.redirect.preserve_query_strings', true);
+
+        Redirect::make()
+            ->source('/abc?another=value')
+            ->destination('/abc?lang=fr#some-fragment?with=fragment_param')
+            ->save();
+
+        $response = $this->middleware->handle(Request::create('/abc', 'GET', ['another' => 'value']), function () {
+            return (new Response('', 404));
+        });
+
+        $this->assertTrue($response->isRedirect(url('/abc?another=value&lang=fr#some-fragment?with=fragment_param')));
+    }
+
+    /**
+     * @test
+     */
+    public function it_merges_query_strings_on_urls_without_path_with_fragment()
+    {
+        config()->set('statamic.redirect.preserve_query_strings', true);
+
+        Redirect::make()
+            ->source('/abc?another=value')
+            ->destination('?lang=fr#some-fragment?with=fragment_param')
+            ->save();
+
+        $response = $this->middleware->handle(Request::create('/abc', 'GET', ['another' => 'value']), function () {
+            return (new Response('', 404));
+        });
+
+        $this->assertTrue($response->isRedirect(url('?another=value&lang=fr#some-fragment?with=fragment_param')));
+    }
 
 
     /**