Avoiding possible key collisions in the Paginator helper

It was possiblle before to pass keys in the query string that collided
with parts of the url params in the router

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);

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['#']);

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])
         );
     }
 

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.
      *

tests/TestCase/View/Helper/PaginatorHelperTest.php

@@ -814,19 +814,19 @@ public function testPassedArgsMergingWithUrlOptions()
         $result = $this->Paginator->numbers();
         $expected = [
             ['li' => ['class' => 'active']], '<a href=""', '1', '/a', '/li',
-            ['li' => []], ['a' => ['href' => '/articles/index/2?page=2&amp;foo=bar&amp;x=y&amp;num=0']], '2', '/a', '/li',
-            ['li' => []], ['a' => ['href' => '/articles/index/2?page=3&amp;foo=bar&amp;x=y&amp;num=0']], '3', '/a', '/li',
-            ['li' => []], ['a' => ['href' => '/articles/index/2?page=4&amp;foo=bar&amp;x=y&amp;num=0']], '4', '/a', '/li',
-            ['li' => []], ['a' => ['href' => '/articles/index/2?page=5&amp;foo=bar&amp;x=y&amp;num=0']], '5', '/a', '/li',
-            ['li' => []], ['a' => ['href' => '/articles/index/2?page=6&amp;foo=bar&amp;x=y&amp;num=0']], '6', '/a', '/li',
-            ['li' => []], ['a' => ['href' => '/articles/index/2?page=7&amp;foo=bar&amp;x=y&amp;num=0']], '7', '/a', '/li',
+            ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&amp;x=y&amp;num=0&amp;page=2']], '2', '/a', '/li',
+            ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&amp;x=y&amp;num=0&amp;page=3']], '3', '/a', '/li',
+            ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&amp;x=y&amp;num=0&amp;page=4']], '4', '/a', '/li',
+            ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&amp;x=y&amp;num=0&amp;page=5']], '5', '/a', '/li',
+            ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&amp;x=y&amp;num=0&amp;page=6']], '6', '/a', '/li',
+            ['li' => []], ['a' => ['href' => '/articles/index/2?foo=bar&amp;x=y&amp;num=0&amp;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&amp;foo=bar&amp;x=y&amp;num=0', 'rel' => 'next'],
+            'a' => ['href' => '/articles/index/2?foo=bar&amp;x=y&amp;num=0&amp;page=2', 'rel' => 'next'],
             'Next',
             '/a',
             '/li'

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);
     }
 
     /**