Make sure the controller & redirect_to configurations conflict with a correct error message

sroze · sroze · commit 62e7de800b0f · 2017-12-03T11:44:24.000Z
diff --git a/src/Symfony/Component/Routing/Loader/XmlFileLoader.php b/src/Symfony/Component/Routing/Loader/XmlFileLoader.php
@@ -244,6 +244,10 @@ private function parseConfigs(\DOMElement $node, $path)
         }
 
         if ($redirectTo = $node->getAttribute('redirect-to')) {
+            if (isset($defaults['_controller'])) {
+                throw new \InvalidArgumentException(sprintf('The routing file "%s" must not specify both a controller and a redirection.', $path));
+            }
+
             $defaults['_redirect_to'] = $redirectTo;
             $defaults['_redirect_permanent'] = (bool) $node->getAttribute('redirect-permanent');
         }
diff --git a/src/Symfony/Component/Routing/Loader/YamlFileLoader.php b/src/Symfony/Component/Routing/Loader/YamlFileLoader.php
@@ -120,6 +120,10 @@ protected function parseRoute(RouteCollection $collection, $name, array $config,
         }
 
         if (isset($config['redirect_to'])) {
+            if (isset($defaults['_controller'])) {
+                throw new \InvalidArgumentException(sprintf('The routing file "%s" must not specify both a controller and a redirection.', $path));
+            }
+
             $defaults['_redirect_to'] = $config['redirect_to'];
             $defaults['_redirect_permanent'] = $config['redirect_permanent'] ?? false;
         }
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/redirect_to/redirect_with_controller.xml b/src/Symfony/Component/Routing/Tests/Fixtures/redirect_to/redirect_with_controller.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<routes xmlns="http://symfony.com/schema/routing"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://symfony.com/schema/routing
+        http://symfony.com/schema/routing/routing-1.0.xsd">
+
+    <route id="app_symfony" path="/symfony" redirect-to="https://symfony.com" controller="App\Controller\Symfony" />
+</routes>
diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/redirect_to/redirect_with_controller.yml b/src/Symfony/Component/Routing/Tests/Fixtures/redirect_to/redirect_with_controller.yml
@@ -0,0 +1,4 @@
+app_symfony:
+    path: /symfony
+    controller: App\Controller\Symfony
+    redirect_to: https://symfony.com
diff --git a/src/Symfony/Component/Routing/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/XmlFileLoaderTest.php
@@ -389,4 +389,14 @@ public function testRedirections()
         $this->assertSame('/', $route->getDefault('_redirect_to'));
         $this->assertTrue($route->getDefault('_redirect_permanent'));
     }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessageRegExp /The routing file "[^"]*" must not specify both a controller and a redirection./
+     */
+    public function testRedirectionCannotBeUsedWithController()
+    {
+        $loader = new XmlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/redirect_to')));
+        $loader->load('redirect_with_controller.xml');
+    }
 }
diff --git a/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php
@@ -210,4 +210,14 @@ public function testRedirections()
         $this->assertSame('/', $route->getDefault('_redirect_to'));
         $this->assertTrue($route->getDefault('_redirect_permanent'));
     }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessageRegExp /The routing file "[^"]*" must not specify both a controller and a redirection./
+     */
+    public function testRedirectionCannotBeUsedWithController()
+    {
+        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures/redirect_to')));
+        $loader->load('redirect_with_controller.yml');
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -244,6 +244,10 @@ private function parseConfigs(\DOMElement $node, $path)`
`244`	`244`	`}`
`245`	`245`
`246`	`246`	`if ($redirectTo = $node->getAttribute('redirect-to')) {`
	`247`	`+ if (isset($defaults['_controller'])) {`
	`248`	`+ throw new \InvalidArgumentException(sprintf('The routing file "%s" must not specify both a controller and a redirection.', $path));`
	`249`	`+ }`
	`250`	`+`
`247`	`251`	`$defaults['_redirect_to'] = $redirectTo;`
`248`	`252`	`$defaults['_redirect_permanent'] = (bool) $node->getAttribute('redirect-permanent');`
`249`	`253`	`}`
Original file line number	Diff line number	Diff line change
`@@ -120,6 +120,10 @@ protected function parseRoute(RouteCollection $collection, $name, array $config,`
`120`	`120`	`}`
`121`	`121`
`122`	`122`	`if (isset($config['redirect_to'])) {`
	`123`	`+ if (isset($defaults['_controller'])) {`
	`124`	`+ throw new \InvalidArgumentException(sprintf('The routing file "%s" must not specify both a controller and a redirection.', $path));`
	`125`	`+ }`
	`126`	`+`
`123`	`127`	`$defaults['_redirect_to'] = $config['redirect_to'];`
`124`	`128`	`$defaults['_redirect_permanent'] = $config['redirect_permanent'] ?? false;`
`125`	`129`	`}`