Skip to content

Commit d8e6f31

Browse files
authored
prevent duplicate keys in destination's query parameters (#202)
1 parent 738aeb3 commit d8e6f31

File tree

2 files changed

+16
-11
lines changed

2 files changed

+16
-11
lines changed

src/Http/Middleware/HandleNotFound.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,13 @@ public function mergeQuery(Request $request, string $destination): string
154154
if (isset($destination_parsed['query'])) {
155155
parse_str($destination_parsed['query'], $destination_query);
156156
}
157+
// in case of duplicate keys, the destination ones take precedence
158+
$query = array_merge($request->query(), $destination_query);
157159

158-
$query = array_merge($destination_query, $request->query());
160+
ksort($query);
159161

160162
if (count($query)) {
161-
$destination .= '?' . http_build_query($query);
163+
$destination = $destination_parsed['path'] . '?' . http_build_query($query);
162164
}
163165

164166
return $destination;

tests/Feature/Middleware/HandleNotFoundTest.php

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public function it_redirects_and_sets_handled_if_a_redirect_is_found_with_eloque
148148
$this->assertTrue($response->isRedirect(url($result)));
149149
}
150150

151-
public function provideRedirects(): array
151+
public static function provideRedirects(): array
152152
{
153153
return [
154154
['/abc', '/def', '/abc', '/def'],
@@ -211,6 +211,9 @@ public function it_handles_query_parameters()
211211
$this->assertTrue($response->isRedirect(url('/fr')));
212212
}
213213

214+
/**
215+
* @test
216+
*/
214217
public function it_can_preserve_query_strings()
215218
{
216219
config()->set('statamic.redirect.preserve_query_strings', true);
@@ -238,27 +241,27 @@ public function it_can_preserve_query_strings()
238241
$this->assertTrue($response->isRedirect(url('/fr?lang=fr')));
239242
}
240243

244+
/**
245+
* @test
246+
*/
241247
public function it_merges_query_strings()
242248
{
243249
config()->set('statamic.redirect.preserve_query_strings', true);
244250

245-
Redirect::make()
246-
->source('/abc')
247-
->destination('/nl')
248-
->save();
249-
250251
Redirect::make()
251252
->source('/abc?another=value')
252-
->destination('/fr')
253+
->destination('/fr?lang=fr')
253254
->save();
254255

255-
$response = $this->middleware->handle(Request::create('/abc', 'GET', ['lang' => 'nl']), function () {
256+
$response = $this->middleware->handle(Request::create('/abc', 'GET', ['another' => 'value']), function () {
256257
return (new Response('', 404));
257258
});
258259

259-
$this->assertTrue($response->isRedirect(url('/nl?lang=nl&another=value')));
260+
$this->assertTrue($response->isRedirect(url('/fr?another=value&lang=fr')));
260261
}
261262

263+
264+
262265
/**
263266
* @test
264267
*/

0 commit comments

Comments
 (0)