Skip to content

Commit 7cb768b

Browse files
author
Amrouche Hamza
committed
[HttpKernel] Add a better error messages when passing a private or non-tagged controller
1 parent 1f14f4d commit 7cb768b

File tree

2 files changed

+102
-2
lines changed

2 files changed

+102
-2
lines changed

src/Symfony/Component/HttpKernel/Controller/ContainerControllerResolver.php

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
use Psr\Container\ContainerInterface;
1515
use Psr\Log\LoggerInterface;
16+
use Symfony\Component\Debug\Exception\FatalThrowableError;
17+
use Symfony\Component\DependencyInjection\Container;
1618
use Symfony\Component\HttpFoundation\Request;
1719

1820
/**
@@ -39,7 +41,13 @@ public function getController(Request $request)
3941
{
4042
$controller = parent::getController($request);
4143

42-
if (is_array($controller) && isset($controller[0]) && is_string($controller[0]) && $this->container->has($controller[0])) {
44+
$isControllerValid = is_array($controller) && isset($controller[0]) && is_string($controller[0]);
45+
46+
if ($isControllerValid && !($hasController = $this->container->has($controller[0])) && $this->container instanceof Container && ($servicesAndAliases = $this->container->getServiceIds()) && false !== $key = array_search(strtolower($controller[0]), array_map('strtolower', $servicesAndAliases))) {
47+
throw new \InvalidArgumentException(sprintf('Case mismatch detected, did you mean "%s"?', $servicesAndAliases[$key]));
48+
}
49+
50+
if ($isControllerValid && $hasController) {
4351
$controller[0] = $this->instantiateController($controller[0]);
4452
}
4553

@@ -86,6 +94,18 @@ protected function instantiateController($class)
8694
return $this->container->get($class);
8795
}
8896

89-
return parent::instantiateController($class);
97+
$e = null;
98+
99+
try {
100+
return parent::instantiateController($class);
101+
} catch (FatalThrowableError $e) {
102+
} catch (\ArgumentCountError $e) {
103+
}
104+
105+
if (null !== $e && $this->container instanceof Container) {
106+
if (in_array($class, $this->container->getRemovedIds())) {
107+
throw new \InvalidArgumentException(sprintf('The controller %s seems to be removed from the container, which means that he maybe private, add the tag \'controller.service_arguments\' or made it public.', $class));
108+
}
109+
}
90110
}
91111
}

src/Symfony/Component/HttpKernel/Tests/Controller/ContainerControllerResolverTest.php

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Psr\Container\ContainerInterface;
1515
use Psr\Log\LoggerInterface;
16+
use Symfony\Component\DependencyInjection\ContainerBuilder;
1617
use Symfony\Component\HttpFoundation\Request;
1718
use Symfony\Component\HttpKernel\Controller\ContainerControllerResolver;
1819

@@ -106,6 +107,74 @@ public function testNonInstantiableController()
106107
$this->assertSame(array(NonInstantiableController::class, 'action'), $controller);
107108
}
108109

110+
public function testNonConstructController()
111+
{
112+
$container = $this->getMockBuilder(ContainerBuilder::class)->getMock();
113+
$container->expects($this->at(0))
114+
->method('has')
115+
->with(ImpossibleConstructController::class)
116+
->will($this->returnValue(true))
117+
;
118+
119+
$container->expects($this->at(1))
120+
->method('has')
121+
->with(ImpossibleConstructController::class)
122+
->will($this->returnValue(false))
123+
;
124+
125+
$container->expects($this->atLeastOnce())
126+
->method('getRemovedIds')
127+
->with()
128+
->will($this->returnValue(array(ImpossibleConstructController::class)))
129+
;
130+
131+
if (method_exists($this, 'expectException')) {
132+
$this->expectException(\InvalidArgumentException::class);
133+
$this->expectExceptionMessage('The controller Symfony\Component\HttpKernel\Tests\Controller\ImpossibleConstructController seems to be removed from the container, which means that he maybe private, add the tag \'controller.service_arguments\' or made it public.');
134+
} else {
135+
$this->setExpectedException(\InvalidArgumentException::class, 'The controller Symfony\Component\HttpKernel\Tests\Controller\ImpossibleConstructController seems to be removed from the container, which means that he maybe private, add the tag \'controller.service_arguments\' or made it public.');
136+
}
137+
138+
$resolver = $this->createControllerResolver(null, $container);
139+
$request = Request::create('/');
140+
$request->attributes->set('_controller', array(ImpossibleConstructController::class, 'action'));
141+
142+
$controller = $resolver->getController($request);
143+
144+
$this->assertSame(array(ImpossibleConstructController::class, 'action'), $controller);
145+
}
146+
147+
public function testNonCaseMisMatchController()
148+
{
149+
$container = $this->getMockBuilder(ContainerBuilder::class)->getMock();
150+
$container->expects($this->atLeastOnce())
151+
->method('has')
152+
->with('Symfony\Component\HttpKernel\Tests\Controller\impossibleconstructcontroller')
153+
->will($this->returnValue(false))
154+
;
155+
156+
$container->expects($this->atLeastOnce())
157+
->method('getServiceIds')
158+
->with()
159+
->will($this->returnValue(array(ImpossibleConstructController::class)))
160+
;
161+
162+
if (method_exists($this, 'expectException')) {
163+
$this->expectException(\InvalidArgumentException::class);
164+
$this->expectExceptionMessage('Case mismatch detected, did you mean "Symfony\Component\HttpKernel\Tests\Controller\ImpossibleConstructController"?');
165+
} else {
166+
$this->setExpectedException(\InvalidArgumentException::class, 'Case mismatch detected, did you mean "Symfony\Component\HttpKernel\Tests\Controller\ImpossibleConstructController"?');
167+
}
168+
169+
$resolver = $this->createControllerResolver(null, $container);
170+
$request = Request::create('/');
171+
$request->attributes->set('_controller', array('Symfony\Component\HttpKernel\Tests\Controller\impossibleconstructcontroller', 'action'));
172+
173+
$controller = $resolver->getController($request);
174+
175+
$this->assertSame(array(ImpossibleConstructController::class, 'action'), $controller);
176+
}
177+
109178
public function testNonInstantiableControllerWithCorrespondingService()
110179
{
111180
$service = new \stdClass();
@@ -196,3 +265,14 @@ public static function action()
196265
{
197266
}
198267
}
268+
269+
class ImpossibleConstructController
270+
{
271+
public function __construct($toto, $controller)
272+
{
273+
}
274+
275+
public function action()
276+
{
277+
}
278+
}

0 commit comments

Comments
 (0)