Skip to content

Commit af038a8

Browse files
likeuntomurphylikeuntomurphy
committed
[Routing][FrameworkBundle] Allow configurable query encoding type in UrlGenerator
1 parent 8d277ce commit af038a8

File tree

11 files changed

+143
-9
lines changed

11 files changed

+143
-9
lines changed

‎src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ CHANGELOG
2626
* Deprecated `CachePoolPrunerPass`. Use `Symfony\Component\Cache\DependencyInjection\CachePoolPrunerPass` instead.
2727
* Deprecated support for legacy translations directories `src/Resources/translations/` and `src/Resources/<BundleName>/translations/`, use `translations/` instead.
2828
* Deprecated support for the legacy directory structure in `translation:update` and `debug:translation` commands.
29+
* 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
2930

3031
4.1.0
3132
-----

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,21 @@ private function addRouterSection(ArrayNodeDefinition $rootNode)
454454
->defaultTrue()
455455
->end()
456456
->booleanNode('utf8')->defaultFalse()->end()
457+
->scalarNode('query_encoding_type')
458+
->validate()
459+
->ifNotInArray(array(PHP_QUERY_RFC1738, PHP_QUERY_RFC3986))
460+
->thenInvalid(
461+
'The value %s is not allowed for path "framework.router.query_encoding_type". '.
462+
"Permissible values are PHP_QUERY_RFC1738 and PHP_QUERY_RFC3986, entered as a constant: '!php/const PHP_QUERY_RFC1738'"
463+
)
464+
->end()
465+
->info(
466+
"This value defaults to PHP_QUERY_RFC3986, which makes the UrlGenerator percent-encode spaces (%20) when building query strings.\n".
467+
"It can be set to PHP_QUERY_RFC1738 to make the UrlGenerator encode spaces as plus signs (+) instead.\n".
468+
"It should be declared as a constant, like '!php/const PHP_QUERY_RFC1738'"
469+
)
470+
->defaultValue(PHP_QUERY_RFC3986)
471+
->end()
457472
->end()
458473
->end()
459474
->end()

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,9 @@ private function registerRouterConfiguration(array $config, ContainerBuilder $co
726726
if (isset($config['type'])) {
727727
$argument['resource_type'] = $config['type'];
728728
}
729+
if (isset($config['query_encoding_type'])) {
730+
$argument['query_encoding_type'] = $config['query_encoding_type'];
731+
}
729732
$router->replaceArgument(2, $argument);
730733

731734
$container->setParameter('request_listener.http_port', $config['http_port']);

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,58 @@ public function provideInvalidAssetConfigurationTests()
156156
yield array($createPackageConfig($config), 'You cannot use both "version" and "json_manifest_path" at the same time under "assets" packages.');
157157
}
158158

159+
public function testQueryEncodingTypeDefaultValue()
160+
{
161+
$processor = new Processor();
162+
$config = $processor->processConfiguration(new Configuration(true), array(array()));
163+
164+
$this->assertEquals(PHP_QUERY_RFC3986, $config['router']['query_encoding_type']);
165+
}
166+
167+
public function getTestValidQueryEncodingTypes()
168+
{
169+
return array(
170+
'PHP_QUERY_RFC1738' => array(PHP_QUERY_RFC1738),
171+
'PHP_QUERY_RFC3986' => array(PHP_QUERY_RFC3986),
172+
);
173+
}
174+
175+
/**
176+
* @dataProvider getTestValidQueryEncodingTypes
177+
*/
178+
public function testValidQueryEncodingType($encodingType)
179+
{
180+
$processor = new Processor();
181+
182+
$config = $processor->processConfiguration(
183+
new Configuration(true),
184+
array(array('router' => array('query_encoding_type' => $encodingType, 'resource' => '.')))
185+
);
186+
187+
$this->assertEquals($encodingType, $config['router']['query_encoding_type']);
188+
}
189+
190+
/**
191+
* @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
192+
*/
193+
public function testInvalidQueryEncodingType()
194+
{
195+
$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'";
196+
197+
if (method_exists($this, 'expectException')) {
198+
$this->expectException(InvalidConfigurationException::class);
199+
$this->expectExceptionMessage($expectedMessage);
200+
} else {
201+
$this->setExpectedException(InvalidConfigurationException::class, $expectedMessage);
202+
}
203+
204+
$processor = new Processor();
205+
$processor->processConfiguration(
206+
new Configuration(true),
207+
array(array('router' => array('query_encoding_type' => PHP_INT_MAX, 'resource' => '.')))
208+
);
209+
}
210+
159211
protected static function getBundleDefaultConfig()
160212
{
161213
return array(
@@ -226,6 +278,7 @@ protected static function getBundleDefaultConfig()
226278
'https_port' => 443,
227279
'strict_requirements' => true,
228280
'utf8' => false,
281+
'query_encoding_type' => PHP_QUERY_RFC3986,
229282
),
230283
'session' => array(
231284
'enabled' => false,

‎src/Symfony/Bundle/FrameworkBundle/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"symfony/polyfill-mbstring": "~1.0",
2828
"symfony/filesystem": "~3.4|~4.0",
2929
"symfony/finder": "~3.4|~4.0",
30-
"symfony/routing": "^4.1"
30+
"symfony/routing": "^4.2"
3131
},
3232
"require-dev": {
3333
"doctrine/cache": "~1.0",

‎src/Symfony/Component/Routing/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ CHANGELOG
55
-----
66

77
* added fallback to cultureless locale for internationalized routes
8+
* Added a `query_encoding_type` configuration option for `UrlGenerator` to allow encoding query strings according to RFC 1738.
89

910
4.0.0
1011
-----

‎src/Symfony/Component/Routing/Generator/Dumper/PhpGeneratorDumper.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,14 @@ class {$options['class']} extends {$options['base_class']}
5555
{
5656
private static \$declaredRoutes;
5757
private \$defaultLocale;
58-
59-
public function __construct(RequestContext \$context, LoggerInterface \$logger = null, string \$defaultLocale = null)
58+
protected \$queryEncodingType;
59+
60+
public function __construct(RequestContext \$context, LoggerInterface \$logger = null, string \$defaultLocale = null, int \$queryEncodingType = PHP_QUERY_RFC3986)
6061
{
6162
\$this->context = \$context;
6263
\$this->logger = \$logger;
6364
\$this->defaultLocale = \$defaultLocale;
65+
\$this->queryEncodingType = \$queryEncodingType;
6466
if (null === self::\$declaredRoutes) {
6567
self::\$declaredRoutes = {$this->generateDeclaredRoutes()};
6668
}

‎src/Symfony/Component/Routing/Generator/UrlGenerator.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt
3535
*/
3636
protected $strictRequirements = true;
3737

38+
/**
39+
* By default, http_build_query() uses PHP_QUERY_RFC1738 as its fourth
40+
* parameter. This implementation passes PHP_QUERY_RFC3986 as its preferred
41+
* encoding type, but it can be configured to use PHP_QUERY_RFC1738.
42+
*
43+
* @var int
44+
*/
45+
protected $queryEncodingType = PHP_QUERY_RFC3986;
46+
3847
protected $logger;
3948

4049
private $defaultLocale;
@@ -67,12 +76,13 @@ class UrlGenerator implements UrlGeneratorInterface, ConfigurableRequirementsInt
6776
'%7C' => '|',
6877
);
6978

70-
public function __construct(RouteCollection $routes, RequestContext $context, LoggerInterface $logger = null, string $defaultLocale = null)
79+
public function __construct(RouteCollection $routes, RequestContext $context, LoggerInterface $logger = null, string $defaultLocale = null, int $queryEncodingType = PHP_QUERY_RFC3986)
7180
{
7281
$this->routes = $routes;
7382
$this->context = $context;
7483
$this->logger = $logger;
7584
$this->defaultLocale = $defaultLocale;
85+
$this->queryEncodingType = $queryEncodingType;
7686
}
7787

7888
/**
@@ -269,7 +279,7 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
269279
unset($extra['_fragment']);
270280
}
271281

272-
if ($extra && $query = http_build_query($extra, '', '&', PHP_QUERY_RFC3986)) {
282+
if ($extra && $query = http_build_query($extra, '', '&', $this->queryEncodingType)) {
273283
// "/" and "?" can be left decoded for better user experience, see
274284
// http://tools.ietf.org/html/rfc3986#section-3.4
275285
$url .= '?'.strtr($query, array('%2F' => '/'));

‎src/Symfony/Component/Routing/Router.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ public function __construct(LoaderInterface $loader, $resource, array $options =
117117
* * resource_type: Type hint for the main resource (optional)
118118
* * strict_requirements: Configure strict requirement checking for generators
119119
* implementing ConfigurableRequirementsInterface (default is true)
120+
* * query_encoding_type: Configure the encoding type passed by generators to http_build_query
120121
*
121122
* @param array $options An array of options
122123
*
@@ -137,6 +138,7 @@ public function setOptions(array $options)
137138
'matcher_cache_class' => 'ProjectUrlMatcher',
138139
'resource_type' => null,
139140
'strict_requirements' => true,
141+
'query_encoding_type' => PHP_QUERY_RFC3986,
140142
);
141143

142144
// check option names and live merge, if errors are encountered Exception will be thrown
@@ -321,7 +323,7 @@ public function getGenerator()
321323
}
322324

323325
if (null === $this->options['cache_dir'] || null === $this->options['generator_cache_class']) {
324-
$this->generator = new $this->options['generator_class']($this->getRouteCollection(), $this->context, $this->logger);
326+
$this->generator = new $this->options['generator_class']($this->getRouteCollection(), $this->context, $this->logger, null, $this->options['query_encoding_type']);
325327
} else {
326328
$cache = $this->getConfigCacheFactory()->cache($this->options['cache_dir'].'/'.$this->options['generator_cache_class'].'.php',
327329
function (ConfigCacheInterface $cache) {
@@ -340,7 +342,7 @@ function (ConfigCacheInterface $cache) {
340342
require_once $cache->getPath();
341343
}
342344

343-
$this->generator = new $this->options['generator_cache_class']($this->context, $this->logger);
345+
$this->generator = new $this->options['generator_cache_class']($this->context, $this->logger, null, $this->options['query_encoding_type']);
344346
}
345347

346348
if ($this->generator instanceof ConfigurableRequirementsInterface) {

‎src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,4 +253,32 @@ public function testDumpWithSchemeRequirement()
253253
$this->assertEquals('https://localhost/app.php/testing', $absoluteUrl);
254254
$this->assertEquals('/app.php/testing', $relativeUrl);
255255
}
256+
257+
public function testDumpWithDefaultQueryEncoding()
258+
{
259+
$this->routeCollection->add('Test1', new Route('/with space'));
260+
261+
file_put_contents($this->testTmpFilepath, $this->generatorDumper->dump(array('class' => 'Rfc3986UrlGenerator')));
262+
include $this->testTmpFilepath;
263+
264+
$rfc3986UrlGenerator = new \Rfc3986UrlGenerator(new RequestContext());
265+
266+
$url = $rfc3986UrlGenerator->generate('Test1', array('query' => 'with space', '_fragment' => 'with space'));
267+
268+
$this->assertEquals('/with%20space?query=with%20space#with%20space', $url);
269+
}
270+
271+
public function testDumpWithRfc1738QueryEncoding()
272+
{
273+
$this->routeCollection->add('Test1', new Route('/with space'));
274+
275+
file_put_contents($this->testTmpFilepath, $this->generatorDumper->dump(array('class' => 'Rfc1738UrlGenerator')));
276+
include $this->testTmpFilepath;
277+
278+
$rfc1738UrlGenerator = new \Rfc1738UrlGenerator(new RequestContext(), null, null, PHP_QUERY_RFC1738);
279+
280+
$url = $rfc1738UrlGenerator->generate('Test1', array('query' => 'with space', '_fragment' => 'with space'));
281+
282+
$this->assertEquals('/with%20space?query=with+space#with%20space', $url);
283+
}
256284
}

0 commit comments

Comments
 (0)