8000 bug #10540 [HttpKernel] made parsing controllers more robust (fabpot) · webmozart/symfony@9068875 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9068875

Browse files
fabpotfabpot
committed
bug symfony#10540 [HttpKernel] made parsing controllers more robust (fabpot)
This PR was merged into the 2.3 branch. Discussion ---------- [HttpKernel] made parsing controllers more robust | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#10465 | License | MIT | Doc PR | n/a Under some circumstances (like passing an object without an __invoke method), the Controller resolver would lead to a PHP fatal. This PR fixes that, improved error messages, and refactored the unit tests. Commits ------- 6dba229 made parsing controllers more robust
2 parents e4ff8ae + 6dba229 commit 9068875

File tree

2 files changed

+94
-33
lines changed

2 files changed

+94
-33
lines changed

‎src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,18 @@ public function getController(Request $request)
6464
return false;
6565
}
6666

67-
if (is_array($controller) || (is_object($controller) && method_exists($controller, '__invoke'))) {
67+
if (is_array($controller)) {
6868
return $controller;
6969
}
7070

71+
if (is_object($controller)) {
72+
if (method_exists($controller, '__invoke')) {
73+
return $controller;
74+
}
75+
76+
throw new \InvalidArgumentException(sprintf('Controller "%s" for URI "%s" is not callable.', get_class($controller), $request->getPathInfo()));
77+
}
78+
7179
if (false === strpos($controller, ':')) {
7280
if (method_exists($controller, '__invoke')) {
7381
return new $controller();
@@ -79,7 +87,7 @@ public function getController(Request $request)
7987
$callable = $this->createController($controller);
8088

8189
if (!is_callable($callable)) {
82-
throw new \InvalidArgumentException(sprintf('The controller for URI "%s" is not callable.', $request->getPathInfo()));
90+
throw new \InvalidArgumentException(sprintf('Controller "%s" for URI "%s" is not callable.', $controller, $request->getPathInfo()));
8391
}
8492

8593
return $callable;

‎src/Symfony/Component/HttpKernel/Tests/Controller/ControllerResolverTest.php

Lines changed: 84 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -24,66 +24,119 @@ protected function setUp()
2424
}
2525
}
2626

27-
public function testGetController()
27+
public function testGetControllerWithoutControllerParameter()
2828
{
29-
$logger = new Logger();
29+
$logger = $this->getMock('Psr\Log\LoggerInterface');
30+
$logger->expects($this->once())->method('warning')->with('Unable to look for the controller as the "_controller" parameter is missing');
3031
$resolver = new ControllerResolver($logger);
3132

3233
$request = Request::create('/');
3334
$this->assertFalse($resolver->getController($request), '->getController() returns false when the request has no _controller attribute');
34-
$this->assertEquals(array('Unable to look for the controller as the "_controller" parameter is missing'), $logger->getLogs('warning'));
35+
}
3536

36-
$request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::testGetController');
37-
$controller = $resolver->getController($request);
38-
$this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller 10000 \ControllerResolverTest', $controller[0], '->getController() returns a PHP callable');
37+
public function testGetControllerWithLambda()
38+
{
39+
$resolver = new ControllerResolver();
3940

41+
$request = Request::create('/');
4042
$request->attributes->set('_controller', $lambda = function () {});
4143
$controller = $resolver->getController($request);
4244
$this->assertSame($lambda, $controller);
45+
}
4346

47+
public function testGetControllerWithObjectAndInvokeMethod()
48+
{
49+
$resolver = new ControllerResolver();
50+
51+
$request = Request::create('/');
4452
$request->attributes->set('_controller', $this);
4553
$controller = $resolver->getController($request);
4654
$this->assertSame($this, $controller);
55+
}
4756

48-
$request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest');
49-
$controller = $resolver->getController($request);
50-
$this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', $controller);
57+
public function testGetControllerWithObjectAndMethod()
58+
{
59+
$resolver = new ControllerResolver();
5160

61+
$request = Request::create('/');
5262
$request->attributes->set('_controller', array($this, 'controllerMethod1'));
5363
$controller = $resolver->getController($request);
5464
$this->assertSame(array($this, 'controllerMethod1'), $controller);
65+
}
5566

67+
public function testGetControllerWithClassAndMethod()
68+
{
69+
$resolver = new ControllerResolver();
70+
71+
$request = Request::create('/');
5672
$request->attributes->set('_controller', array('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', 'controllerMethod4'));
5773
$controller = $resolver->getController($request);
5874
$this->assertSame(array('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', 'controllerMethod4'), $controller);
75+
}
76+
77+
public function testGetControllerWithObjectAndMethodAsString()
78+
{
79+
$resolver = new ControllerResolver();
80+
81+
$request = Request::create('/');
82+
$request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::controllerMethod1');
83+
$controller = $resolver->getController($request);
84+
$this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', $controller[0], '->getController() returns a PHP callable');
85+
}
86+
87+
public function testGetControllerWithClassAndInvokeMethod()
88+
{
89+
$resolver = new ControllerResolver();
90+
91+
$request = Request::create('/');
92+
$request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest');
93+
$controller = $resolver->getController($request);
94+
$this->assertInstanceOf('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest', $controller);
95+
}
96+
97+
/**
98+
* @expectedException \InvalidArgumentException
99+
*/
100+
public function testGetControllerOnObjectWithoutInvokeMethod()
101+
{
102+
$resolver = new ControllerResolver();
103+
104+
$request = Request::create('/');
105+
$request->attributes->set('_controller', new \stdClass());
106+
$resolver->getController($request);
107+
}
108+
109+
public function testGetControllerWithFunction()
110+
{
111+
$resolver = new ControllerResolver();
59112

113+
$request = Request::create('/');
60114
$request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\some_controller_function');
61115
$controller = $resolver->getController($request);
62116
$this->assertSame('Symfony\Component\HttpKernel\Tests\Controller\some_controller_function', $controller);
117+
}
63118

64-
$request->attributes->set('_controller', 'foo');
65-
try {
66-
$resolver->getController($request);
67-
$this->fail('->getController() throws an \InvalidArgumentException if the _controller attribute is not well-formatted');
68-
} catch (\Exception $e) {
69-
$this->assertInstanceOf('\InvalidArgumentException', $e, '->getController() throws an \InvalidArgumentException if the _controller attribute is not well-formatted');
70-
}
119+
/**
120+
* @dataProvider getUndefinedControllers
121+
* @expectedException \InvalidArgumentException
122+
*/
123+
public function testGetControllerOnNonUndefinedFunction($controller)
124+
{
125+
$resolver = new ControllerResolver();
71126

72-
$request->attributes->set('_controller', 'foo::bar');
73-
try {
74-
$resolver->getController($request);
75-
$this->fail('->getController() throws an \InvalidArgumentException if the _controller attribute contains a non-existent class');
76-
} catch (\Exception $e) {
77-
$this->assertInstanceOf('\InvalidArgumentException', $e, '->getController() throws an \InvalidArgumentException if the _controller attribute contains a non-existent class');
78-
}
127+
$request = Request::create('/');
128+
$request->attributes->set('_controller', $controller);
129+
$resolver->getController($request);
130+
}
79131

80-
$request->attributes->set('_controller', 'Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::bar');
81-
try {
82-
$resolver->getController($request);
83-
$this->fail('->getController() throws an \InvalidArgumentException if the _controller attribute contains a non-existent method');
84-
} catch (\Exception $e) {
85-
$this->assertInstanceOf('\InvalidArgumentException', $e, '->getController() throws an \InvalidArgumentException if the _controller attribute contains a non-existent method');
86-
}
132+
public function getUndefinedControllers()
133+
{
134+
return array(
135+
array('foo'),
136+
array('foo::bar'),
137+
array('stdClass'),
138+
array('Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest::bar'),
139+
);
87140
}
88141

89142
public function testGetArguments()
@@ -165,7 +218,7 @@ public function __invoke($foo, $bar = null)
165218
{
166219
}
167220

168-
protected function controllerMethod1($foo)
221+
public function controllerMethod1($foo)
169222
{
170223
}
171224

0 commit comments

Comments
 (0)
0