Skip to content

Commit

Permalink
bug #28120 [Routing] Fixed scheme redirecting for root path (twoleds)
Browse files Browse the repository at this point in the history
This PR was merged into the 4.1 branch.

Discussion
----------

[Routing] Fixed scheme redirecting for root path

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

I and my friend found a bug with routing / matching and redirecting from http to https by forcing routes (https://symfony.com/doc/current/routing/scheme.html). It works good for all routes except the homepage (root path /). The problem is probably here (https://github.com/symfony/routing/blob/6912cfebc0ea4e7a46fdd15c9bd1f427dd39ff1b/Matcher/Dumper/PhpMatcherDumper.php#L196-L199). Symfony tries to display the welcome page instead of redirecting to https.

Commits
-------

2d7fdff [Routing] Fixed scheme redirecting for root path
  • Loading branch information
nicolas-grekas committed Aug 7, 2018
2 parents d8da0a0 + 2d7fdff commit d59e97f
Show file tree
Hide file tree
Showing 16 changed files with 29 additions and 15 deletions.
Expand Up @@ -194,7 +194,7 @@ private function compileRoutes(RouteCollection $routes, bool $matchHost): string
}

// used to display the Welcome Page in apps that don't define a homepage
$code .= " if ('/' === \$pathinfo && !\$allow) {\n";
$code .= " if ('/' === \$pathinfo && !\$allow && !\$allowSchemes) {\n";
$code .= " throw new Symfony\Component\Routing\Exception\NoConfigurationException();\n";
$code .= " }\n";

Expand Down
Expand Up @@ -26,7 +26,7 @@ public function match($rawPathinfo)
$canonicalMethod = 'GET';
}

if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -238,7 +238,7 @@ public function match($rawPathinfo)
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -2821,7 +2821,7 @@ public function match($rawPathinfo)
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -142,7 +142,7 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -91,7 +91,7 @@ public function match($rawPathinfo)
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -60,7 +60,7 @@ public function match($rawPathinfo)
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -275,7 +275,7 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -103,7 +103,7 @@ public function match($rawPathinfo)
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -75,7 +75,7 @@ public function match($rawPathinfo)
return $ret;
}

if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -145,7 +145,7 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -122,7 +122,7 @@ public function match($rawPathinfo)
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -157,7 +157,7 @@ private function doMatch(string $rawPathinfo, array &$allow = array(), array &$a
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -79,7 +79,7 @@ public function match($rawPathinfo)
$offset += strlen($m);
}
}
if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -44,7 +44,7 @@ public function match($rawPathinfo)
break;
}

if ('/' === $pathinfo && !$allow) {
if ('/' === $pathinfo && !$allow && !$allowSchemes) {
throw new Symfony\Component\Routing\Exception\NoConfigurationException();
}

Expand Down
Expand Up @@ -93,6 +93,20 @@ public function testSchemeRedirectWithParams()
$this->assertEquals(array('_route' => 'foo', 'bar' => 'baz', 'redirect' => 'value'), $matcher->match('/foo/baz'));
}

public function testSchemeRedirectForRoot()
{
$coll = new RouteCollection();
$coll->add('foo', new Route('/', array(), array(), array(), '', array('https')));

$matcher = $this->getUrlMatcher($coll);
$matcher
->expects($this->once())
->method('redirect')
->with('/', 'foo', 'https')
->will($this->returnValue(array('redirect' => 'value')));
$this->assertEquals(array('_route' => 'foo', 'redirect' => 'value'), $matcher->match('/'));
}

public function testSlashRedirectWithParams()
{
$coll = new RouteCollection();
Expand Down

0 comments on commit d59e97f

Please sign in to comment.