bug #9601 [Routing] Remove usage of deprecated _scheme requirement (D…

…anez)

This PR was merged into the 2.3 branch.

Discussion
----------

[Routing] Remove usage of deprecated _scheme requirement

**This is exact the same commit as it was in #9585, which was not merged due to my fault. Sorry for the noise.**

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #8898, #8176
| License       | MIT
| Doc PR        |

I removed all usages of the deprecated _scheme requirement inside the Routing Component.
Most parts were pretty easy and after multiple refactorings I came up with the solution to have a Route::hasScheme() method and check against this method.

I also checked for performance and after trying in_array, arra_flip+isset and foreach, the last one was clearly the winner.
https://gist.github.com/Danez/7609898#file-test_performance-php

I also adjusted all tests that test '_scheme' to also check the new schemes-requirement.

Commits
-------

557dfaa Remove usage of deprecated _scheme in Routing Component

src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RedirectableUrlMatcherTest.php

@@ -38,7 +38,7 @@ public function testRedirectWhenNoSlash()
         );
     }
 
-    public function testSchemeRedirect()
+    public function testSchemeRedirectBC()
     {
         $coll = new RouteCollection();
         $coll->add('foo', new Route('/foo', array(), array('_scheme' => 'https')));
@@ -57,4 +57,24 @@ public function testSchemeRedirect()
             $matcher->match('/foo')
         );
     }
+
+    public function testSchemeRedirect()
+    {
+        $coll = new RouteCollection();
+        $coll->add('foo', new Route('/foo', array(), array(), array(), '', array('https')));
+
+        $matcher = new RedirectableUrlMatcher($coll, $context = new RequestContext());
+
+        $this->assertEquals(array(
+                '_controller' => 'Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction',
+                'path'        => '/foo',
+                'permanent'   => true,
+                'scheme'      => 'https',
+                'httpPort'    => $context->getHttpPort(),
+                'httpsPort'   => $context->getHttpsPort(),
+                '_route'      => 'foo',
+            ),
+            $matcher->match('/foo')
+        );
+    }
 }

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

@@ -92,6 +92,7 @@ private function generateDeclaredRoutes()
             $properties[] = $route->getRequirements();
             $properties[] = $compiledRoute->getTokens();
             $properties[] = $compiledRoute->getHostTokens();
+            $properties[] = $route->getSchemes();
 
             $routes .= sprintf("        '%s' => %s,\n", $name, str_replace("\n", '', var_export($properties, true)));
         }
@@ -114,9 +115,9 @@ public function generate(\$name, \$parameters = array(), \$referenceType = self:
             throw new RouteNotFoundException(sprintf('Unable to generate a URL for the named route "%s" as such route does not exist.', \$name));
         }
 
-        list(\$variables, \$defaults, \$requirements, \$tokens, \$hostTokens) = self::\$declaredRoutes[\$name];
+        list(\$variables, \$defaults, \$requirements, \$tokens, \$hostTokens, \$requiredSchemes) = self::\$declaredRoutes[\$name];
 
-        return \$this->doGenerate(\$variables, \$defaults, \$requirements, \$tokens, \$parameters, \$name, \$referenceType, \$hostTokens);
+        return \$this->doGenerate(\$variables, \$defaults, \$requirements, \$tokens, \$parameters, \$name, \$referenceType, \$hostTokens, \$requiredSchemes);
     }
 EOF;
     }

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

@@ -137,15 +137,15 @@ public function generate($name, $parameters = array(), $referenceType = self::AB
         // the Route has a cache of its own and is not recompiled as long as it does not get modified
         $compiledRoute = $route->compile();
 
-        return $this->doGenerate($compiledRoute->getVariables(), $route->getDefaults(), $route->getRequirements(), $compiledRoute->getTokens(), $parameters, $name, $referenceType, $compiledRoute->getHostTokens());
+        return $this->doGenerate($compiledRoute->getVariables(), $route->getDefaults(), $route->getRequirements(), $compiledRoute->getTokens(), $parameters, $name, $referenceType, $compiledRoute->getHostTokens(), $route->getSchemes());
     }
 
     /**
      * @throws MissingMandatoryParametersException When some parameters are missing that are mandatory for the route
      * @throws InvalidParameterException           When a parameter value for a placeholder is not correct because
      *                                             it does not match the requirement
      */
-    protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens)
+    protected function doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $referenceType, $hostTokens, array $requiredSchemes = array())
     {
         $variables = array_flip($variables);
         $mergedParams = array_replace($defaults, $this->context->getParameters(), $parameters);
@@ -204,7 +204,24 @@ protected function doGenerate($variables, $defaults, $requirements, $tokens, $pa
         $schemeAuthority = '';
         if ($host = $this->context->getHost()) {
             $scheme = $this->context->getScheme();
-            if (isset($requirements['_scheme']) && ($req = strtolower($requirements['_scheme'])) && $scheme !== $req) {
+
+            if ($requiredSchemes) {
+                $schemeMatched = false;
+                foreach ($requiredSchemes as $requiredScheme) {
+                    if ($scheme === $requiredScheme) {
+                        $schemeMatched = true;
+
+                        break;
+                    }
+                }
+
+                if (!$schemeMatched) {
+                    $referenceType = self::ABSOLUTE_URL;
+                    $scheme = current($requiredSchemes);
+                }
+
+            } elseif (isset($requirements['_scheme']) && ($req = strtolower($requirements['_scheme'])) && $scheme !== $req) {
+                // We do this for BC; to be removed if _scheme is not supported anymore
                 $referenceType = self::ABSOLUTE_URL;
                 $scheme = $req;
             }

src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

@@ -280,14 +280,15 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
 EOF;
         }
 
-        if ($scheme = $route->getRequirement('_scheme')) {
+        if ($schemes = $route->getSchemes()) {
             if (!$supportsRedirections) {
-                throw new \LogicException('The "_scheme" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
+                throw new \LogicException('The "schemes" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
             }
-
+            $schemes = str_replace("\n", '', var_export(array_flip($schemes), true));
             $code .= <<<EOF
-            if (\$this->context->getScheme() !== '$scheme') {
-                return \$this->redirect(\$pathinfo, '$name', '$scheme');
+            \$requiredSchemes = $schemes;
+            if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
+                return \$this->redirect(\$pathinfo, '$name', key(\$requiredSchemes));
             }
 
 
@@ -305,8 +306,11 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
             }
             $vars[] = "array('_route' => '$name')";
 
-            $code .= sprintf("            return \$this->mergeDefaults(array_replace(%s), %s);\n"
-                , implode(', ', $vars), str_replace("\n", '', var_export($route->getDefaults(), true)));
+            $code .= sprintf(
+                "            return \$this->mergeDefaults(array_replace(%s), %s);\n",
+                implode(', ', $vars),
+                str_replace("\n", '', var_export($route->getDefaults(), true))
+            );
 
         } elseif ($route->getDefaults()) {
             $code .= sprintf("            return %s;\n", str_replace("\n", '', var_export(array_replace($route->getDefaults(), array('_route' => $name)), true)));

src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php

@@ -51,9 +51,10 @@ public function match($pathinfo)
     protected function handleRouteRequirements($pathinfo, $name, Route $route)
     {
         // check HTTP scheme requirement
-        $scheme = $route->getRequirement('_scheme');
-        if ($scheme && $this->context->getScheme() !== $scheme) {
-            return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, $scheme));
+        $scheme = $this->context->getScheme();
+        $schemes = $route->getSchemes();
+        if ($schemes && !$route->hasScheme($scheme)) {
+            return array(self::ROUTE_MATCH, $this->redirect($pathinfo, $name, current($schemes)));
         }
 
         return array(self::REQUIREMENT_MATCH, null);

src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php

@@ -95,9 +95,11 @@ protected function matchCollection($pathinfo, RouteCollection $routes)
             }
 
             // check HTTP scheme requirement
-            if ($scheme = $route->getRequirement('_scheme')) {
-                if ($this->context->getScheme() !== $scheme) {
-                    $this->addTrace(sprintf('Scheme "%s" does not match the requirement ("%s"); the user will be redirected', $this->context->getScheme(), $scheme), self::ROUTE_ALMOST_MATCHES, $name, $route);
+            if ($requiredSchemes = $route->getSchemes()) {
+                $scheme = $this->context->getScheme();
+
+                if (!$route->hasScheme($scheme)) {
+                    $this->addTrace(sprintf('Scheme "%s" does not match any of the required schemes ("%s"); the user will be redirected to first required scheme', $scheme, implode(', ', $requiredSchemes)), self::ROUTE_ALMOST_MATCHES, $name, $route);
 
                     return true;
                 }

src/Symfony/Component/Routing/Matcher/UrlMatcher.php

@@ -181,8 +181,8 @@ protected function getAttributes(Route $route, $name, array $attributes)
     protected function handleRouteRequirements($pathinfo, $name, Route $route)
     {
         // check HTTP scheme requirement
-        $scheme = $route->getRequirement('_scheme');
-        $status = $scheme && $scheme !== $this->context->getScheme() ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;
+        $scheme = $this->context->getScheme();
+        $status = $route->getSchemes() && !$route->hasScheme($scheme) ? self::REQUIREMENT_MISMATCH : self::REQUIREMENT_MATCH;
 
         return array($status, null);
     }

src/Symfony/Component/Routing/Route.php

@@ -241,6 +241,25 @@ public function setSchemes($schemes)
         return $this;
     }
 
+    /**
+     * Checks if a scheme requirement has been set.
+     *
+     * @param string $scheme
+     *
+     * @return Boolean true if the scheme requirement exists, otherwise false
+     */
+    public function hasScheme($scheme)
+    {
+        $scheme = strtolower($scheme);
+        foreach ($this->schemes as $requiredScheme) {
+            if ($scheme === $requiredScheme) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Returns the uppercased HTTP methods this route is restricted to.
      * So an empty array means that any method is allowed.

src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php

@@ -319,17 +319,19 @@ public function match($pathinfo)
 
         // secure
         if ($pathinfo === '/secure') {
-            if ($this->context->getScheme() !== 'https') {
-                return $this->redirect($pathinfo, 'secure', 'https');
+            $requiredSchemes = array (  'https' => 0,);
+            if (!isset($requiredSchemes[$this->context->getScheme()])) {
+                return $this->redirect($pathinfo, 'secure', key($requiredSchemes));
             }
 
             return array('_route' => 'secure');
         }
 
         // nonsecure
         if ($pathinfo === '/nonsecure') {
-            if ($this->context->getScheme() !== 'http') {
-                return $this->redirect($pathinfo, 'nonsecure', 'http');
+            $requiredSchemes = array (  'http' => 0,);
+            if (!isset($requiredSchemes[$this->context->getScheme()])) {
+                return $this->redirect($pathinfo, 'nonsecure', key($requiredSchemes));
             }
 
             return array('_route' => 'nonsecure');

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

@@ -114,4 +114,37 @@ public function testDumpForRouteWithDefaults()
 
         $this->assertEquals($url, '/testing');
     }
+
+    public function testDumpWithSchemeRequirement()
+    {
+        $this->routeCollection->add('Test1', new Route('/testing', array(), array(), array(), '', array('ftp', 'https')));
+        $this->routeCollection->add('Test2', new Route('/testing_bc', array(), array('_scheme' => 'https'))); // BC
+
+        file_put_contents($this->testTmpFilepath, $this->generatorDumper->dump(array('class' => 'SchemeUrlGenerator')));
+        include ($this->testTmpFilepath);
+
+        $projectUrlGenerator = new \SchemeUrlGenerator(new RequestContext('/app.php'));
+
+        $absoluteUrl = $projectUrlGenerator->generate('Test1', array(), true);
+        $absoluteUrlBC = $projectUrlGenerator->generate('Test2', array(), true);
+        $relativeUrl = $projectUrlGenerator->generate('Test1', array(), false);
+        $relativeUrlBC = $projectUrlGenerator->generate('Test2', array(), false);
+
+        $this->assertEquals($absoluteUrl, 'ftp://localhost/app.php/testing');
+        $this->assertEquals($absoluteUrlBC, 'https://localhost/app.php/testing_bc');
+        $this->assertEquals($relativeUrl, 'ftp://localhost/app.php/testing');
+        $this->assertEquals($relativeUrlBC, 'https://localhost/app.php/testing_bc');
+
+        $projectUrlGenerator = new \SchemeUrlGenerator(new RequestContext('/app.php', 'GET', 'localhost', 'https'));
+
+        $absoluteUrl = $projectUrlGenerator->generate('Test1', array(), true);
+        $absoluteUrlBC = $projectUrlGenerator->generate('Test2', array(), true);
+        $relativeUrl = $projectUrlGenerator->generate('Test1', array(), false);
+        $relativeUrlBC = $projectUrlGenerator->generate('Test2', array(), false);
+
+        $this->assertEquals($absoluteUrl, 'https://localhost/app.php/testing');
+        $this->assertEquals($absoluteUrlBC, 'https://localhost/app.php/testing_bc');
+        $this->assertEquals($relativeUrl, '/app.php/testing');
+        $this->assertEquals($relativeUrlBC, '/app.php/testing_bc');
+    }
 }

src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php

@@ -248,22 +248,40 @@ public function testRequiredParamAndEmptyPassed()
 
     public function testSchemeRequirementDoesNothingIfSameCurrentScheme()
     {
-        $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http')));
+        $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); // BC
         $this->assertEquals('/app.php/', $this->getGenerator($routes)->generate('test'));
 
-        $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https')));
+        $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); // BC
+        $this->assertEquals('/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
+
+        $routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('http')));
+        $this->assertEquals('/app.php/', $this->getGenerator($routes)->generate('test'));
+
+        $routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('https')));
         $this->assertEquals('/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
     }
 
     public function testSchemeRequirementForcesAbsoluteUrl()
     {
-        $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https')));
+        $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'https'))); // BC
+        $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test'));
+
+        $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http'))); // BC
+        $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
+
+        $routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('https')));
         $this->assertEquals('https://localhost/app.php/', $this->getGenerator($routes)->generate('test'));
 
-        $routes = $this->getRoutes('test', new Route('/', array(), array('_scheme' => 'http')));
+        $routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('http')));
         $this->assertEquals('http://localhost/app.php/', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test'));
     }
 
+    public function testSchemeRequirementCreatesUrlForFirstRequiredScheme()
+    {
+        $routes = $this->getRoutes('test', new Route('/', array(), array(), array(), '', array('Ftp', 'https')));
+        $this->assertEquals('ftp://localhost/app.php/', $this->getGenerator($routes)->generate('test'));
+    }
+
     public function testPathWithTwoStartingSlashes()
     {
         $routes = $this->getRoutes('test', new Route('//path-and-not-domain'));
@@ -447,7 +465,7 @@ public function testUrlWithInvalidParameterInHostInNonStrictMode()
         $this->assertNull($generator->generate('test', array('foo' => 'baz'), false));
     }
 
-    public function testGenerateNetworkPath()
+    public function testGenerateNetworkPathBC()
     {
         $routes = $this->getRoutes('test', new Route('/{name}', array(), array('_scheme' => 'http'), array(), '{locale}.example.com'));
 
@@ -465,13 +483,32 @@ public function testGenerateNetworkPath()
         );
     }
 
+    public function testGenerateNetworkPath()
+    {
+        $routes = $this->getRoutes('test', new Route('/{name}', array(), array(), array(), '{locale}.example.com', array('http')));
+
+        $this->assertSame('//fr.example.com/app.php/Fabien', $this->getGenerator($routes)->generate('test',
+            array('name' =>'Fabien', 'locale' => 'fr'), UrlGeneratorInterface::NETWORK_PATH), 'network path with different host'
+        );
+        $this->assertSame('//fr.example.com/app.php/Fabien?query=string', $this->getGenerator($routes, array('host' => 'fr.example.com'))->generate('test',
+            array('name' =>'Fabien', 'locale' => 'fr', 'query' => 'string'), UrlGeneratorInterface::NETWORK_PATH), 'network path although host same as context'
+        );
+        $this->assertSame('http://fr.example.com/app.php/Fabien', $this->getGenerator($routes, array('scheme' => 'https'))->generate('test',
+            array('name' =>'Fabien', 'locale' => 'fr'), UrlGeneratorInterface::NETWORK_PATH), 'absolute URL because scheme requirement does not match context'
+        );
+        $this->assertSame('http://fr.example.com/app.php/Fabien', $this->getGenerator($routes)->generate('test',
+            array('name' =>'Fabien', 'locale' => 'fr'), UrlGeneratorInterface::ABSOLUTE_URL), 'absolute URL with same scheme because it is requested'
+        );
+    }
+
     public function testGenerateRelativePath()
     {
         $routes = new RouteCollection();
         $routes->add('article', new Route('/{author}/{article}/'));
         $routes->add('comments', new Route('/{author}/{article}/comments'));
         $routes->add('host', new Route('/{article}', array(), array(), array(), '{author}.example.com'));
-        $routes->add('scheme', new Route('/{author}', array(), array('_scheme' => 'https')));
+        $routes->add('schemeBC', new Route('/{author}', array(), array('_scheme' => 'https'))); // BC
+        $routes->add('scheme', new Route('/{author}/blog', array(), array(), array(), '', array('https')));
         $routes->add('unrelated', new Route('/about'));
 
         $generator = $this->getGenerator($routes, array('host' => 'example.com', 'pathInfo' => '/fabien/symfony-is-great/'));
@@ -491,9 +528,12 @@ public function testGenerateRelativePath()
         $this->assertSame('//bernhard.example.com/app.php/forms-are-great', $generator->generate('host',
             array('author' =>'bernhard', 'article' => 'forms-are-great'), UrlGeneratorInterface::RELATIVE_PATH)
         );
-        $this->assertSame('https://example.com/app.php/bernhard', $generator->generate('scheme',
+        $this->assertSame('https://example.com/app.php/bernhard', $generator->generate('schemeBC',
             array('author' =>'bernhard'), UrlGeneratorInterface::RELATIVE_PATH)
         );
+        $this->assertSame('https://example.com/app.php/bernhard/blog', $generator->generate('scheme',
+                array('author' =>'bernhard'), UrlGeneratorInterface::RELATIVE_PATH)
+        );
         $this->assertSame('../../about', $generator->generate('unrelated',
             array(), UrlGeneratorInterface::RELATIVE_PATH)
         );