From c0e6bf6f360fddc963786d3d61c48fd034678816 Mon Sep 17 00:00:00 2001 From: James Halsall Date: Tue, 16 Aug 2016 15:45:37 +0100 Subject: [PATCH 1/3] Update UrlGenerator::generate() so it encodes in compliance with PHP_QUERY_RFC3986 --- .../Component/Routing/Generator/UrlGenerator.php | 12 +++++++++++- .../Routing/Tests/Generator/UrlGeneratorTest.php | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index e72b6cf974c74..884ac05bb7566 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -268,7 +268,17 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa return $a == $b ? 0 : 1; }); - if ($extra && $query = http_build_query($extra, '', '&')) { + // extract fragment + $fragment = isset($extra['_fragment']) ? $extra['_fragment'] : ''; + unset($extra['_fragment']); + + if (defined('PHP_QUERY_RFC3986')) { + $query = http_build_query($extra, '', '&', PHP_QUERY_RFC3986); + } else { + $query = http_build_query($extra, '', '&'); + } + + if ($extra && $query) { // "/" and "?" can be left decoded for better user experience, see // http://tools.ietf.org/html/rfc3986#section-3.4 $url .= '?'.strtr($query, array('%2F' => '/')); diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index b2a4ecd72db73..532143b5a0091 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -331,7 +331,7 @@ public function testUrlEncoding() $routes = $this->getRoutes('test', new Route("/$chars/{varpath}", array(), array('varpath' => '.+'))); $this->assertSame('/app.php/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id' .'/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id' - .'?query=%40%3A%5B%5D/%28%29%2A%27%22+%2B%2C%3B-._%7E%26%24%3C%3E%7C%7B%7D%25%5C%5E%60%21%3Ffoo%3Dbar%23id', + .'?query=%40%3A%5B%5D/%28%29%2A%27%22%20%2B%2C%3B-._~%26%24%3C%3E%7C%7B%7D%25%5C%5E%60%21%3Ffoo%3Dbar%23id', $this->getGenerator($routes)->generate('test', array( 'varpath' => $chars, 'query' => $chars, From 4c9eb0070917fbcc19519bd6ead605b050d17261 Mon Sep 17 00:00:00 2001 From: James Halsall Date: Tue, 16 Aug 2016 16:51:26 +0100 Subject: [PATCH 2/3] Fix breaking test under 5.3.x --- .../Routing/Tests/Generator/UrlGeneratorTest.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 532143b5a0091..03cdfc171586b 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -325,6 +325,16 @@ public function testGenerateWithSpecialRouteName() public function testUrlEncoding() { + if (defined('PHP_QUERY_RFC3986')) { + $expectedPath = '/app.php/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id' + .'/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id' + .'?query=%40%3A%5B%5D/%28%29%2A%27%22%20%2B%2C%3B-._~%26%24%3C%3E%7C%7B%7D%25%5C%5E%60%21%3Ffoo%3Dbar%23id'; + } else { + $expectedPath = '/app.php/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id' + .'/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id' + .'?query=%40%3A%5B%5D/%28%29%2A%27%22+%2B%2C%3B-._%7E%26%24%3C%3E%7C%7B%7D%25%5C%5E%60%21%3Ffoo%3Dbar%23id'; + } + // This tests the encoding of reserved characters that are used for delimiting of URI components (defined in RFC 3986) // and other special ASCII chars. These chars are tested as static text path, variable path and query param. $chars = '@:[]/()*\'" +,;-._~&$<>|{}%\\^`!?foo=bar#id'; From 389e446de96412bdacd2940f8dde3518f7bc8255 Mon Sep 17 00:00:00 2001 From: James Halsall Date: Tue, 16 Aug 2016 21:31:31 +0100 Subject: [PATCH 3/3] Remove redundant code, fix UrlGenerator::testUrlEncoding() --- .../Component/Routing/Generator/UrlGenerator.php | 4 ---- .../Routing/Tests/Generator/UrlGeneratorTest.php | 12 ++++-------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php index 884ac05bb7566..05be2439d4a84 100644 --- a/src/Symfony/Component/Routing/Generator/UrlGenerator.php +++ b/src/Symfony/Component/Routing/Generator/UrlGenerator.php @@ -268,10 +268,6 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa return $a == $b ? 0 : 1; }); - // extract fragment - $fragment = isset($extra['_fragment']) ? $extra['_fragment'] : ''; - unset($extra['_fragment']); - if (defined('PHP_QUERY_RFC3986')) { $query = http_build_query($extra, '', '&', PHP_QUERY_RFC3986); } else { diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php index 03cdfc171586b..da98bad0b2614 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php @@ -339,14 +339,10 @@ public function testUrlEncoding() // and other special ASCII chars. These chars are tested as static text path, variable path and query param. $chars = '@:[]/()*\'" +,;-._~&$<>|{}%\\^`!?foo=bar#id'; $routes = $this->getRoutes('test', new Route("/$chars/{varpath}", array(), array('varpath' => '.+'))); - $this->assertSame('/app.php/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id' - .'/@:%5B%5D/%28%29*%27%22%20+,;-._~%26%24%3C%3E|%7B%7D%25%5C%5E%60!%3Ffoo=bar%23id' - .'?query=%40%3A%5B%5D/%28%29%2A%27%22%20%2B%2C%3B-._~%26%24%3C%3E%7C%7B%7D%25%5C%5E%60%21%3Ffoo%3Dbar%23id', - $this->getGenerator($routes)->generate('test', array( - 'varpath' => $chars, - 'query' => $chars, - )) - ); + $this->assertSame($expectedPath, $this->getGenerator($routes)->generate('test', array( + 'varpath' => $chars, + 'query' => $chars, + ))); } public function testEncodingOfRelativePathSegments()