From 9c4a8e1018a0e36278afec098d11fabf04c0bccd Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 21:43:58 +0000 Subject: [PATCH 01/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Routing/Generator/UrlGenerator.php | 8 +++++++ .../Tests/Generator/UrlGeneratorTest.php | 23 ++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index dc46c60fb98c7..e9296aa125581 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -258,6 +258,14 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa // add a query string if needed $extra = array_diff_key($parameters, $variables, $defaults); + foreach($parameters as $key => $value) { + if (isset($defaults[$key]) && + !array_key_exists($key, $variables) && + $defaults[$key] !== $value + ) { + $extra[$key] = $value; + } + } if ($extra && $query = http_build_query($extra, '', '&')) { $url .= '?'.$query; } diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index d7b74f01e1bb3..5b31731a182c8 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -117,6 +117,27 @@ public function testAbsoluteUrlWithExtraParameters() $this->assertEquals('http://localhost/app.php/testing?foo=bar', $url); } + public function testAbsoluteUrlWithExtraParametersAndDefault() + { + $routes = $this->getRoutes('test', new Route('/testing', array('foo' => 'bell'))); + $url = $this->getGenerator($routes)->generate('test', array('foo' => 'bar'), UrlGeneratorInterface::ABSOLUTE_URL); + $this->assertEquals('http://localhost/app.php/testing?foo=bar', $url); + } + + public function testAbsoluteUrlWithExtraParametersAndArrayDefault() + { + $routes = $this->getRoutes('test', new Route('/testing', array('foo' => array('bell')))); + $url = $this->getGenerator($routes)->generate('test', array('foo' => array('bar')), UrlGeneratorInterface::ABSOLUTE_URL); + $this->assertEquals('http://localhost/app.php/testing?foo%5B0%5D=bar', $url); + } + + public function testAbsoluteUrlWithoutExtraParametersAndArrayDefault() + { + $routes = $this->getRoutes('test', new Route('/testing', array('foo' => array('bell')))); + $url = $this->getGenerator($routes)->generate('test', array('foo' => array('bell')), UrlGeneratorInterface::ABSOLUTE_URL); + $this->assertEquals('http://localhost/app.php/testing', $url); + } + public function testUrlWithNullExtraParameters() { $routes = $this->getRoutes('test', new Route('/testing')); @@ -293,7 +314,7 @@ public function testQueryParamSameAsDefault() { $routes = $this->getRoutes('test', new Route('/test', array('default' => 'value'))); - $this->assertSame('/app.php/test', $this->getGenerator($routes)->generate('test', array('default' => 'foo'))); + $this->assertSame('/app.php/test?default=foo', $this->getGenerator($routes)->generate('test', array('default' => 'foo'))); $this->assertSame('/app.php/test', $this->getGenerator($routes)->generate('test', array('default' => 'value'))); $this->assertSame('/app.php/test', $this->getGenerator($routes)->generate('test')); } From 08c7517049e03796c624464ae0770dfed914c142 Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 21:54:34 +0000 Subject: [PATCH 02/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Component/Routing/Tests/Generator/UrlGeneratorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 5b31731a182c8..e0845a915509f 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -277,6 +277,7 @@ public function testSchemeRequirementForcesAbsoluteUrl() $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test')); + $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test')); } From 14ec0da9776070f3d2ba59f69a9da70a3b6130c5 Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 21:54:45 +0000 Subject: [PATCH 03/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Component/Routing/Tests/Generator/UrlGeneratorTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index e0845a915509f..5b31731a182c8 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -277,7 +277,6 @@ public function testSchemeRequirementForcesAbsoluteUrl() $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test')); - $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test')); } From 24e37e24c58ae1480bd809d88f0265204f206491 Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 22:00:22 +0000 Subject: [PATCH 04/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Component/Routing/Tests/Generator/UrlGeneratorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 5b31731a182c8..e0845a915509f 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -277,6 +277,7 @@ public function testSchemeRequirementForcesAbsoluteUrl() $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test')); + $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test')); } From 6c9c008d4452425fd8d6c2a2e99f0db766924d66 Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 22:00:30 +0000 Subject: [PATCH 05/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Component/Routing/Tests/Generator/UrlGeneratorTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index e0845a915509f..5b31731a182c8 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -277,7 +277,6 @@ public function testSchemeRequirementForcesAbsoluteUrl() $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test')); - $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test')); } From 20dfd2e317e5a30851245070e5782b680b3f18d3 Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 21:43:58 +0000 Subject: [PATCH 06/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Routing/Generator/UrlGenerator.php | 8 +++++++ .../Tests/Generator/UrlGeneratorTest.php | 23 ++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index dc46c60fb98c7..e9296aa125581 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -258,6 +258,14 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa // add a query string if needed $extra = array_diff_key($parameters, $variables, $defaults); + foreach($parameters as $key => $value) { + if (isset($defaults[$key]) && + !array_key_exists($key, $variables) && + $defaults[$key] !== $value + ) { + $extra[$key] = $value; + } + } if ($extra && $query = http_build_query($extra, '', '&')) { $url .= '?'.$query; } diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index d7b74f01e1bb3..5b31731a182c8 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -117,6 +117,27 @@ public function testAbsoluteUrlWithExtraParameters() $this->assertEquals('http://localhost/app.php/testing?foo=bar', $url); } + public function testAbsoluteUrlWithExtraParametersAndDefault() + { + $routes = $this->getRoutes('test', new Route('/testing', array('foo' => 'bell'))); + $url = $this->getGenerator($routes)->generate('test', array('foo' => 'bar'), UrlGeneratorInterface::ABSOLUTE_URL); + $this->assertEquals('http://localhost/app.php/testing?foo=bar', $url); + } + + public function testAbsoluteUrlWithExtraParametersAndArrayDefault() + { + $routes = $this->getRoutes('test', new Route('/testing', array('foo' => array('bell')))); + $url = $this->getGenerator($routes)->generate('test', array('foo' => array('bar')), UrlGeneratorInterface::ABSOLUTE_URL); + $this->assertEquals('http://localhost/app.php/testing?foo%5B0%5D=bar', $url); + } + + public function testAbsoluteUrlWithoutExtraParametersAndArrayDefault() + { + $routes = $this->getRoutes('test', new Route('/testing', array('foo' => array('bell')))); + $url = $this->getGenerator($routes)->generate('test', array('foo' => array('bell')), UrlGeneratorInterface::ABSOLUTE_URL); + $this->assertEquals('http://localhost/app.php/testing', $url); + } + public function testUrlWithNullExtraParameters() { $routes = $this->getRoutes('test', new Route('/testing')); @@ -293,7 +314,7 @@ public function testQueryParamSameAsDefault() { $routes = $this->getRoutes('test', new Route('/test', array('default' => 'value'))); - $this->assertSame('/app.php/test', $this->getGenerator($routes)->generate('test', array('default' => 'foo'))); + $this->assertSame('/app.php/test?default=foo', $this->getGenerator($routes)->generate('test', array('default' => 'foo'))); $this->assertSame('/app.php/test', $this->getGenerator($routes)->generate('test', array('default' => 'value'))); $this->assertSame('/app.php/test', $this->getGenerator($routes)->generate('test')); } From 41439176859201c853e7b339946f96d61c9c8563 Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 21:54:34 +0000 Subject: [PATCH 07/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Component/Routing/Tests/Generator/UrlGeneratorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 5b31731a182c8..e0845a915509f 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -277,6 +277,7 @@ public function testSchemeRequirementForcesAbsoluteUrl() $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test')); + $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test')); } From 39f20a673d6426a077b75d6b78038041389f3f9e Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 21:54:45 +0000 Subject: [PATCH 08/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Component/Routing/Tests/Generator/UrlGeneratorTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index e0845a915509f..5b31731a182c8 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -277,7 +277,6 @@ public function testSchemeRequirementForcesAbsoluteUrl() $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test')); - $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test')); } From 6a787816942ddd0e78d050a433942732c2f1a465 Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 22:00:22 +0000 Subject: [PATCH 09/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Component/Routing/Tests/Generator/UrlGeneratorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 5b31731a182c8..e0845a915509f 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -277,6 +277,7 @@ public function testSchemeRequirementForcesAbsoluteUrl() $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test')); + $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test')); } From 6a464f9f1632821fdf45d9240026598bd7b02555 Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Fri, 11 Mar 2016 22:00:30 +0000 Subject: [PATCH 10/11] #18035 Fix for Router doesn't take additional get parameter if default is provided --- .../Component/Routing/Tests/Generator/UrlGeneratorTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index e0845a915509f..5b31731a182c8 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -277,7 +277,6 @@ public function testSchemeRequirementForcesAbsoluteUrl() $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test')); - $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test')); } From 96e81d45b15ee3d9a5cdfedc973fda5eeefe38a0 Mon Sep 17 00:00:00 2001 From: Simon Hayre Date: Tue, 22 Mar 2016 19:03:19 +0000 Subject: [PATCH 11/11] 18035 Changes from jakzal's review --- src/Symfony/Component/Routing/Generator/UrlGenerator.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index e9296aa125581..355f31a8c5a71 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -259,10 +259,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa // add a query string if needed $extra = array_diff_key($parameters, $variables, $defaults); foreach($parameters as $key => $value) { - if (isset($defaults[$key]) && - !array_key_exists($key, $variables) && - $defaults[$key] !== $value - ) { + if (isset($defaults[$key]) && !array_key_exists($key, $variables) && $defaults[$key] !== $value) { $extra[$key] = $value; } }