Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[Routing] fixed route generation when pattern already has question mark #4191

Closed
wants to merge 1 commit into from

3 participants

@danielholmes

Bug fix: yes
Feature addition: no
Backwards compatibility break: If someone relied no this bug, yes
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

I sometimes have routes like the following used purely for generating a url, whether that be to the symfony app or an external app:

blog_news:
    pattern:  /blog/?category=news

When generating this route with extra parameters, the url it generates is invalid.

$router->generate('blog_news', array('page' => 2));
// results in /blog/?category=news?page=2

I've changed the generator to use & if the url already has a ?

@fabpot
Owner

The route pattern is just about the URL path, and as such it must not contain the query string.

@Tobion
Collaborator

The question mark in the path should be encoded. See #4166.
Then the route will be valid. But as fabpot said, the query string doesn't belong in the route pattern anyway.

@danielholmes

Yea you're right, I didn't think that one through enough. Thanks for the comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
7 src/Symfony/Component/Routing/Generator/UrlGenerator.php
@@ -147,7 +147,12 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
// add a query string if needed
$extra = array_diff_key($originParameters, $variables, $defaults);
if ($extra && $query = http_build_query($extra, '', '&')) {
- $url .= '?'.$query;
+ if (false === strpos($url, '?')) {
+ $url .= '?';
+ } else {
+ $url .= '&';
+ }
+ $url .= $query;
}
$url = $this->context->getBaseUrl().$url;
View
8 tests/Symfony/Tests/Component/Routing/Generator/UrlGeneratorTest.php
@@ -125,6 +125,14 @@ public function testUrlWithExtraParametersFromGlobals()
$this->assertEquals('/app.php/testing?foo=bar', $url);
}
+
+ public function testUrlWithExtraParametersAndQuestionMarkAlready()
+ {
+ $routes = $this->getRoutes('test', new Route('/testing?foo=bar'));
+ $url = $this->getGenerator($routes)->generate('test', array('extra' => '123'));
+
+ $this->assertSame('/app.php/testing?foo=bar&extra=123', $url);
+ }
public function testUrlWithGlobalParameter()
{
Something went wrong with that request. Please try again.