symfony · likeuntomurphy · Jul 16, 2018
diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+4.3.0
+-----
+
+ * Allowed configuring query encoding type passed to `http_build_query` in `Symfony\Component\Routing\Generator\UrlGenerator` via a new `framework.router.query_encoding_type` option
+
 4.2.0
 -----
 

diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
@@ -468,6 +468,21 @@ private function addRouterSection(ArrayNodeDefinition $rootNode)
                             ->defaultTrue()
                         ->end()
                         ->booleanNode('utf8')->defaultFalse()->end()
+                        ->scalarNode('query_encoding_type')
+                            ->validate()
+                                ->ifNotInArray(array(PHP_QUERY_RFC1738, PHP_QUERY_RFC3986))
+                                ->thenInvalid(
+                                    'The value %s is not allowed for path "framework.router.query_encoding_type". '.
+                                    "Permissible values are PHP_QUERY_RFC1738 and PHP_QUERY_RFC3986, entered as a constant: '!php/const PHP_QUERY_RFC1738'"
+                                )
+                            ->end()
+                            ->info(
+                                "This value defaults to PHP_QUERY_RFC3986, which makes the UrlGenerator percent-encode spaces (%20) when building query strings.\n".
+                                "It can be set to PHP_QUERY_RFC1738 to make the UrlGenerator encode spaces as plus signs (+) instead.\n".
+                                "It should be declared as a constant, like '!php/const PHP_QUERY_RFC1738'"
+                            )
+                            ->defaultValue(PHP_QUERY_RFC3986)
+                        ->end()
                     ->end()
                 ->end()
             ->end()

diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -743,6 +743,9 @@ private function registerRouterConfiguration(array $config, ContainerBuilder $co
         if (isset($config['type'])) {
             $argument['resource_type'] = $config['type'];
         }
+        if (isset($config['query_encoding_type'])) {
+            $argument['query_encoding_type'] = $config['query_encoding_type'];
+        }
         $router->replaceArgument(2, $argument);
 
         $container->setParameter('request_listener.http_port', $config['http_port']);

diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
@@ -158,6 +158,58 @@ public function provideInvalidAssetConfigurationTests()
         yield array($createPackageConfig($config), 'You cannot use both "version" and "json_manifest_path" at the same time under "assets" packages.');
     }
 
+    public function testQueryEncodingTypeDefaultValue()
+    {
+        $processor = new Processor();
+        $config = $processor->processConfiguration(new Configuration(true), array(array()));
+
+        $this->assertEquals(PHP_QUERY_RFC3986, $config['router']['query_encoding_type']);
+    }
+
+    public function getTestValidQueryEncodingTypes()
+    {
+        return array(
+            'PHP_QUERY_RFC1738' => array(PHP_QUERY_RFC1738),
+            'PHP_QUERY_RFC3986' => array(PHP_QUERY_RFC3986),
+        );
+    }
+
+    /**
+     * @dataProvider getTestValidQueryEncodingTypes
+     */
+    public function testValidQueryEncodingType($encodingType)
+    {
+        $processor = new Processor();
+
+        $config = $processor->processConfiguration(
+            new Configuration(true),
+            array(array('router' => array('query_encoding_type' => $encodingType, 'resource' => '.')))
+        );
+
+        $this->assertEquals($encodingType, $config['router']['query_encoding_type']);
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     */
+    public function testInvalidQueryEncodingType()
+    {
+        $expectedMessage = 'Invalid configuration for path "framework.router.query_encoding_type": The value '.PHP_INT_MAX." is not allowed for path \"framework.router.query_encoding_type\". Permissible values are PHP_QUERY_RFC1738 and PHP_QUERY_RFC3986, entered as a constant: '!php/const PHP_QUERY_RFC1738'";
+
+        if (method_exists($this, 'expectException')) {
+            $this->expectException(InvalidConfigurationException::class);
+            $this->expectExceptionMessage($expectedMessage);
+        } else {
+            $this->setExpectedException(InvalidConfigurationException::class, $expectedMessage);
+        }
+
+        $processor = new Processor();
+        $processor->processConfiguration(
+            new Configuration(true),
+            array(array('router' => array('query_encoding_type' => PHP_INT_MAX, 'resource' => '.')))
+        );
+    }
+
     protected static function getBundleDefaultConfig()
     {
         return array(
@@ -228,6 +280,7 @@ protected static function getBundleDefaultConfig()
                 'https_port' => 443,
                 'strict_requirements' => true,
                 'utf8' => false,
+                'query_encoding_type' => PHP_QUERY_RFC3986,
             ),
             'session' => array(
                 'enabled' => false,

diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json
@@ -28,7 +28,7 @@
         "symfony/polyfill-mbstring": "~1.0",
         "symfony/filesystem": "~3.4|~4.0",
         "symfony/finder": "~3.4|~4.0",
-        "symfony/routing": "^4.1"
+        "symfony/routing": "^4.3"
     },
     "require-dev": {
         "doctrine/cache": "~1.0",

diff --git a/src/Symfony/Component/Routing/CHANGELOG.md b/src/Symfony/Component/Routing/CHANGELOG.md
@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+4.3.0
+-----
+
+ * Added a `query_encoding_type` configuration option for `UrlGenerator` to allow encoding query strings according to RFC 1738. 
+
 4.2.0
 -----
 

diff --git a/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php b/src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php
@@ -55,12 +55,14 @@ class {$options['class']} extends {$options['base_class']}
 {
     private static \$declaredRoutes;
     private \$defaultLocale;
-
-    public function __construct(RequestContext \$context, LoggerInterface \$logger = null, string \$defaultLocale = null)
+    protected \$queryEncodingType;
+
+    public function __construct(RequestContext \$context, LoggerInterface \$logger = null, string \$defaultLocale = null, int \$queryEncodingType = PHP_QUERY_RFC3986)
     {
         \$this->context = \$context;
         \$this->logger = \$logger;
         \$this->defaultLocale = \$defaultLocale;
+        \$this->queryEncodingType = \$queryEncodingType;
         if (null === self::\$declaredRoutes) {
             self::\$declaredRoutes = {$this->generateDeclaredRoutes()};
         }

diff --git a/src/Symfony/Component/Routing/Generator/UrlGenerator.php b/src/Symfony/Component/Routing/Generator/UrlGenerator.php
@@ -35,6 +35,15 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt
      */
     protected $strictRequirements = true;
 
+    /**
+     * By default, http_build_query() uses PHP_QUERY_RFC1738 as its fourth
+     * parameter. This implementation passes PHP_QUERY_RFC3986 as its preferred
+     * encoding type, but it can be configured to use PHP_QUERY_RFC1738.
+     *
+     * @var int
+     */
+    protected $queryEncodingType = PHP_QUERY_RFC3986;
+
     protected $logger;
 
     private $defaultLocale;
@@ -67,12 +76,13 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt
         '%7C' => '|',
     );
 
-    public function __construct(RouteCollection $routes, RequestContext $context, LoggerInterface $logger = null, string $defaultLocale = null)
+    public function __construct(RouteCollection $routes, RequestContext $context, LoggerInterface $logger = null, string $defaultLocale = null, int $queryEncodingType = PHP_QUERY_RFC3986)
     {
         $this->routes = $routes;
         $this->context = $context;
         $this->logger = $logger;
         $this->defaultLocale = $defaultLocale;
+        $this->queryEncodingType = $queryEncodingType;
     }
 
     /**
@@ -269,7 +279,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
             unset($extra['_fragment']);
         }
 
-        if ($extra && $query = http_build_query($extra, '', '&', PHP_QUERY_RFC3986)) {
+        if ($extra && $query = http_build_query($extra, '', '&', $this->queryEncodingType)) {
             // "/" 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/Router.php b/src/Symfony/Component/Routing/Router.php
@@ -117,6 +117,7 @@ public function __construct(LoaderInterface $loader, $resource, array $options =
      *   * resource_type:          Type hint for the main resource (optional)
      *   * strict_requirements:    Configure strict requirement checking for generators
      *                             implementing ConfigurableRequirementsInterface (default is true)
+     *   * query_encoding_type:    Configure the encoding type passed by generators to http_build_query
      *
      * @param array $options An array of options
      *
@@ -137,6 +138,7 @@ public function setOptions(array $options)
             'matcher_cache_class' => 'ProjectUrlMatcher',
             'resource_type' => null,
             'strict_requirements' => true,
+            'query_encoding_type' => PHP_QUERY_RFC3986,
         );
 
         // check option names and live merge, if errors are encountered Exception will be thrown
@@ -321,7 +323,7 @@ public function getGenerator()
         }
 
         if (null === $this->options['cache_dir'] || null === $this->options['generator_cache_class']) {
-            $this->generator = new $this->options['generator_class']($this->getRouteCollection(), $this->context, $this->logger);
+            $this->generator = new $this->options['generator_class']($this->getRouteCollection(), $this->context, $this->logger, null, $this->options['query_encoding_type']);
         } else {
             $cache = $this->getConfigCacheFactory()->cache($this->options['cache_dir'].'/'.$this->options['generator_cache_class'].'.php',
                 function (ConfigCacheInterface $cache) {
@@ -340,7 +342,7 @@ function (ConfigCacheInterface $cache) {
                 require_once $cache->getPath();
             }
 
-            $this->generator = new $this->options['generator_cache_class']($this->context, $this->logger);
+            $this->generator = new $this->options['generator_cache_class']($this->context, $this->logger, null, $this->options['query_encoding_type']);
         }
 
         if ($this->generator instanceof ConfigurableRequirementsInterface) {

diff --git a/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php b/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php
@@ -253,4 +253,32 @@ public function testDumpWithSchemeRequirement()
         $this->assertEquals('https://localhost/app.php/testing', $absoluteUrl);
         $this->assertEquals('/app.php/testing', $relativeUrl);
     }
+
+    public function testDumpWithDefaultQueryEncoding()
+    {
+        $this->routeCollection->add('Test1', new Route('/with space'));
+
+        file_put_contents($this->testTmpFilepath, $this->generatorDumper->dump(array('class' => 'Rfc3986UrlGenerator')));
+        include $this->testTmpFilepath;
+
+        $rfc3986UrlGenerator = new \Rfc3986UrlGenerator(new RequestContext());
+
+        $url = $rfc3986UrlGenerator->generate('Test1', array('query' => 'with space', '_fragment' => 'with space'));
+
+        $this->assertEquals('/with%20space?query=with%20space#with%20space', $url);
+    }
+
+    public function testDumpWithRfc1738QueryEncoding()
+    {
+        $this->routeCollection->add('Test1', new Route('/with space'));
+
+        file_put_contents($this->testTmpFilepath, $this->generatorDumper->dump(array('class' => 'Rfc1738UrlGenerator')));
+        include $this->testTmpFilepath;
+
+        $rfc1738UrlGenerator = new \Rfc1738UrlGenerator(new RequestContext(), null, null, PHP_QUERY_RFC1738);
+
+        $url = $rfc1738UrlGenerator->generate('Test1', array('query' => 'with space', '_fragment' => 'with space'));
+
+        $this->assertEquals('/with%20space?query=with+space#with%20space', $url);
+    }
 }
diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
@@ -349,6 +349,25 @@ public function testUrlEncoding()
         )));
     }
 
+    public function testRfc1738UrlEncoding()
+    {
+        // The difference between this and the default encoding will be that spaces in the query string are encoded as
+        // plus signs (+) and tildes (~) are percent-encoded
+        $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';
+
+        $chars = '@:[]/()*\'" +,;-._~&$<>|{}%\\^`!?foo=bar#id';
+        $routes = $this->getRoutes('test', new Route("/$chars/{varpath}", array(), array('varpath' => '.+')));
+
+        $generator = $this->getGenerator($routes, array(), null, PHP_QUERY_RFC1738);
+
+        $this->assertSame($expectedPath, $generator->generate('test', array(
+            'varpath' => $chars,
+            'query' => $chars,
+        )));
+    }
+
     public function testEncodingOfRelativePathSegments()
     {
         $routes = $this->getRoutes('test', new Route('/dir/../dir/..'));
@@ -703,15 +722,15 @@ public function testFragmentsCanBeDefinedAsDefaults()
         $this->assertEquals('/app.php/testing#fragment', $url);
     }
 
-    protected function getGenerator(RouteCollection $routes, array $parameters = array(), $logger = null)
+    protected function getGenerator(RouteCollection $routes, array $parameters = array(), $logger = null, $queryEncodingType = PHP_QUERY_RFC3986)
     {
         $context = new RequestContext('/app.php');
         foreach ($parameters as $key => $value) {
             $method = 'set'.$key;
             $context->$method($value);
         }
 
-        return new UrlGenerator($routes, $context, $logger);
+        return new UrlGenerator($routes, $context, $logger, null, $queryEncodingType);
     }
 
     protected function getRoutes($name, Route $route)