diff --git a/src/Routing/Route/Route.php b/src/Routing/Route/Route.php index 468b8c4afde..0af17194ee5 100644 --- a/src/Routing/Route/Route.php +++ b/src/Routing/Route/Route.php @@ -441,7 +441,9 @@ public function match(array $url, array $context = []) if (!isset($hostOptions['_base']) && isset($context['_base'])) { $hostOptions['_base'] = $context['_base']; } - unset($url['_host'], $url['_scheme'], $url['_port'], $url['_base']); + + $query = !empty($url['?']) ? array_filter((array)$url['?'], 'strlen') : []; + unset($url['_host'], $url['_scheme'], $url['_port'], $url['_base'], $url['?']); // Move extension into the hostOptions so its not part of // reverse matches. @@ -484,8 +486,6 @@ public function match(array $url, array $context = []) } $pass = []; - $query = []; - foreach ($url as $key => $value) { // keys that exist in the defaults and have different values is a match failure. $defaultExists = array_key_exists($key, $defaults); diff --git a/src/Routing/Router.php b/src/Routing/Router.php index 76b2b5f85ee..af8a84978e2 100644 --- a/src/Routing/Router.php +++ b/src/Routing/Router.php @@ -577,12 +577,6 @@ public static function url($url = null, $full = false) $full = true; unset($url['_full']); } - // Compatibility for older versions. - if (isset($url['?'])) { - $q = $url['?']; - unset($url['?']); - $url = array_merge($url, $q); - } if (isset($url['#'])) { $frag = '#' . $url['#']; unset($url['#']); diff --git a/src/View/Helper/PaginatorHelper.php b/src/View/Helper/PaginatorHelper.php index fc14c025cbc..de274f1ea6f 100644 --- a/src/View/Helper/PaginatorHelper.php +++ b/src/View/Helper/PaginatorHelper.php @@ -98,9 +98,14 @@ public function __construct(View $View, array $config = []) { parent::__construct($View, $config); + $query = $this->request->query; + if (isset($query['page']) && $query['page'] == 1) { + unset($query['page']); + } + $this->config( 'options.url', - array_merge($this->request->params['pass'], $this->request->query) + array_merge($this->request->params['pass'], ['?' => $query]) ); } diff --git a/tests/TestCase/Routing/RouterTest.php b/tests/TestCase/Routing/RouterTest.php index 1d1ddeea52b..5d524ec0ccc 100644 --- a/tests/TestCase/Routing/RouterTest.php +++ b/tests/TestCase/Routing/RouterTest.php @@ -2971,6 +2971,21 @@ public function testDefaultRouteClass() $this->assertEquals('/foo-bar', $result); } + /** + * Test generation of routes with collisions between the query string + * and other url params + * + * @return void + */ + public function testUrlWithCollidingQueryString() + { + Router::connect('/:controller/:action/:id'); + + $query = ['controller' => 'Foo', 'action' => 'bar', 'id' => 100]; + $result = Router::url(['controller' => 'posts', 'action' => 'view', 'id' => 1, '?' => $query]); + $this->assertEquals('/posts/view/1?controller=Foo&action=bar&id=100', $result); + } + /** * Connect some fallback routes for testing router behavior. * diff --git a/tests/TestCase/View/Helper/PaginatorHelperTest.php b/tests/TestCase/View/Helper/PaginatorHelperTest.php index 666b08b7dcf..e5dfd4bacdb 100644 --- a/tests/TestCase/View/Helper/PaginatorHelperTest.php +++ b/tests/TestCase/View/Helper/PaginatorHelperTest.php @@ -814,19 +814,19 @@ public function testPassedArgsMergingWithUrlOptions() $result = $this->Paginator->numbers(); $expected = [ ['li' => ['class' => 'active']], ' []], ['a' => ['href' => '/articles/index/2?page=2&foo=bar&x=y&num=0']], '2', '/a', '/li', - ['li' => []], ['a' => ['href' => '/articles/index/2?page=3&foo=bar&x=y&num=0']], '3', '/a', '/li', - ['li' => []], ['a' => ['href' => '/articles/index/2?page=4&foo=bar&x=y&num=0']], '4', '/a', '/li', - ['li' => []], ['a' => ['href' => '/articles/index/2?page=5&foo=bar&x=y&num=0']], '5', '/a', '/li', - ['li' => []], ['a' => ['href' => '/articles/index/2?page=6&foo=bar&x=y&num=0']], '6', '/a', '/li', - ['li' => []], ['a' => ['href' => '/articles/index/2?page=7&foo=bar&x=y&num=0']], '7', '/a', '/li', + ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&x=y&num=0&page=2']], '2', '/a', '/li', + ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&x=y&num=0&page=3']], '3', '/a', '/li', + ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&x=y&num=0&page=4']], '4', '/a', '/li', + ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&x=y&num=0&page=5']], '5', '/a', '/li', + ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&x=y&num=0&page=6']], '6', '/a', '/li', + ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&x=y&num=0&page=7']], '7', '/a', '/li', ]; $this->assertHtml($expected, $result); $result = $this->Paginator->next('Next'); $expected = [ 'li' => ['class' => 'next'], - 'a' => ['href' => '/articles/index/2?page=2&foo=bar&x=y&num=0', 'rel' => 'next'], + 'a' => ['href' => '/articles/index/2?foo=bar&x=y&num=0&page=2', 'rel' => 'next'], 'Next', '/a', '/li' diff --git a/tests/TestCase/View/Helper/UrlHelperTest.php b/tests/TestCase/View/Helper/UrlHelperTest.php index f4de99bb50f..b9182a81afd 100644 --- a/tests/TestCase/View/Helper/UrlHelperTest.php +++ b/tests/TestCase/View/Helper/UrlHelperTest.php @@ -97,7 +97,7 @@ public function testUrlConversion() 'controller' => 'posts', 'action' => 'index', 'page' => '1', '?' => ['one' => 'value', 'two' => 'value', 'three' => 'purple'] ]); - $this->assertEquals("/posts/index?page=1&one=value&two=value&three=purple", $result); + $this->assertEquals("/posts/index?one=value&two=value&three=purple&page=1", $result); } /**