From cb7e3f5edb2c1372077674ad422c9e041ebb0a25 Mon Sep 17 00:00:00 2001
From: Tobias Schultze <webmaster@tubo-world.de>
Date: Fri, 31 Aug 2012 08:36:21 +0200
Subject: [PATCH] [Routing] added route compile check to identify a default
 value of a required variable that does not match the requirement

---
 .../Component/Routing/RouteCompiler.php        | 18 ++++++++++++++----
 .../Tests/Generator/UrlGeneratorTest.php       |  6 +++---
 .../Routing/Tests/RouteCompilerTest.php        | 13 ++++++++++++-
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/src/Symfony/Component/Routing/RouteCompiler.php b/src/Symfony/Component/Routing/RouteCompiler.php
index 5aaae19c34..e19f8d50d5 100644
--- a/src/Symfony/Component/Routing/RouteCompiler.php
+++ b/src/Symfony/Component/Routing/RouteCompiler.php
@@ -23,7 +23,8 @@ class RouteCompiler implements RouteCompilerInterface
     /**
      * {@inheritDoc}
      *
-     * @throws \LogicException If a variable is referenced more than once
+     * @throws \LogicException If a variable is referenced more than once or if a required variable
+     *                         has a default value that doesn't meet its own requirement.
      */
     public function compile(Route $route)
     {
@@ -67,14 +68,23 @@ class RouteCompiler implements RouteCompilerInterface
             $tokens[] = array('text', substr($pattern, $pos));
         }
 
-        // find the first optional token
+        // find the first optional token and validate the default values for non-optional variables
+        $optional = true;
         $firstOptional = INF;
         for ($i = count($tokens) - 1; $i >= 0; $i--) {
             $token = $tokens[$i];
             if ('variable' === $token[0] && $route->hasDefault($token[3])) {
-                $firstOptional = $i;
+                if ($optional) {
+                    $firstOptional = $i;
+                } elseif (!preg_match('#^'.$token[2].'$#', $route->getDefault($token[3]))) {
+                    throw new \LogicException(sprintf('The default value "%s" of the required variable "%s" in pattern "%s" does not match the requirement "%s". ' .
+                        'This route definition makes no sense because this default can neither be used as default for generating URLs nor can it ever be returned by the matching process. ' .
+                        'You should change the default to something that meets the requirement or remove it.',
+                        $route->getDefault($token[3]), $token[3], $route->getPattern(), $token[2]
+                    ));
+                }
             } else {
-                break;
+                $optional = false;
             }
         }
 
diff --git a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
index 7dd80d3c23..d8991db32c 100644
--- a/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
+++ b/src/Symfony/Component/Routing/Tests/Generator/UrlGeneratorTest.php
@@ -75,13 +75,13 @@ class UrlGeneratorTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @expectedException Symfony\Component\Routing\Exception\InvalidParameterException
+     * @expectedException \LogicException
      */
     public function testRelativeUrlWithNullParameterButNotOptional()
     {
         $routes = $this->getRoutes('test', new Route('/testing/{foo}/bar', array('foo' => null)));
-        // This must raise an exception because the default requirement for "foo" is "[^/]+" which is not met with these params.
-        // Generating path "/testing//bar" would be wrong as matching this route would fail.
+        // It should raise an exception when compiling this route because the given default value is absolutely
+        // irrelevant for both matching and generating URLs.
         $this->getGenerator($routes)->generate('test', array(), false);
     }
 
diff --git a/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php b/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
index 5288ca68fa..4a32387fd0 100644
--- a/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
+++ b/src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
@@ -131,6 +131,17 @@ class RouteCompilerTest extends \PHPUnit_Framework_TestCase
     {
         $route = new Route('/{name}/{name}');
 
-        $compiled = $route->compile();
+        $route->compile();
+    }
+
+    /**
+     * @expectedException \LogicException
+     */
+    public function testRouteWithRequiredVariableAndBadDefault()
+    {
+        $route = new Route('/{foo}/', array('foo' => null));
+        // It should raise an exception when compiling this route because the given default value is absolutely
+        // irrelevant for both matching and generating URLs.
+        $route->compile();
     }
 }