Add safeguards against private or abstract services

KnpLabs · Jun 16, 2015 · 1e3c934 · 1e3c934
1 parent c0766ad
commit 1e3c934
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 0 deletions.
diff --git a/DependencyInjection/Compiler/MenuBuilderPass.php b/DependencyInjection/Compiler/MenuBuilderPass.php
@@ -18,6 +18,16 @@ public function process(ContainerBuilder $container)
 
  $menuBuilders = array();
  foreach ($container->findTaggedServiceIds('knp_menu.menu_builder') as $id => $tags) {
+ $builderDefinition = $container->getDefinition($id);
+
+ if (!$builderDefinition->isPublic()) {
+ throw new \InvalidArgumentException(sprintf('Menu builder services must be public but "%s" is a private service.', $id));
+ }
+
+ if ($builderDefinition->isAbstract()) {
+ throw new \InvalidArgumentException(sprintf('Abstract services cannot be registered as menu builders but "%s" is.', $id));
+ }
+
  foreach ($tags as $attributes) {
  if (empty($attributes['alias'])) {
  throw new \InvalidArgumentException(sprintf('The alias is not defined in the "knp_menu.menu_builder" tag for the service "%s"', $id));

diff --git a/Tests/DependencyInjection/Compiler/MenuBuilderPassTest.php b/Tests/DependencyInjection/Compiler/MenuBuilderPassTest.php
@@ -8,6 +8,7 @@ class MenuBuilderPassTest extends \PHPUnit_Framework_TestCase
 {
  private $containerBuilder;
  private $definition;
+ private $builderDefinition;
 
  /**
  * @var MenuBuilderPass
@@ -18,9 +19,38 @@ protected function setUp()
  {
  $this->containerBuilder = $this->prophesize('Symfony\Component\DependencyInjection\ContainerBuilder');
  $this->definition = $this->prophesize('Symfony\Component\DependencyInjection\Definition');
+ $this->builderDefinition = $this->prophesize('Symfony\Component\DependencyInjection\Definition');
  $this->pass = new MenuBuilderPass();
 
  $this->containerBuilder->getDefinition('knp_menu.menu_provider.builder_service')->willReturn($this->definition);
+ $this->containerBuilder->getDefinition('id')->willReturn($this->builderDefinition);
+
+ $this->builderDefinition->isPublic()->willReturn(true);
+ $this->builderDefinition->isAbstract()->willReturn(false);
+ }
+
+ /**
+ * @expectedException \InvalidArgumentException
+ * @expectedExceptionMessage Abstract services cannot be registered as menu builders but "id" is.
+ */
+ public function testFailsWhenServiceIsAbstract()
+ {
+ $this->builderDefinition->isAbstract()->willReturn(true);
+ $this->containerBuilder->findTaggedServiceIds('knp_menu.menu_builder')->willReturn(array('id' => array(array('alias' => 'foo'))));
+
+ $this->pass->process($this->containerBuilder->reveal());
+ }
+
+ /**
+ * @expectedException \InvalidArgumentException
+ * @expectedExceptionMessage Menu builder services must be public but "id" is a private service.
+ */
+ public function testFailsWhenServiceIsPrivate()
+ {
+ $this->builderDefinition->isPublic()->willReturn(false);
+ $this->containerBuilder->findTaggedServiceIds('knp_menu.menu_builder')->willReturn(array('id' => array(array('alias' => 'foo'))));
+
+ $this->pass->process($this->containerBuilder->reveal());
  }
 
  /**
@@ -47,6 +77,9 @@ public function testFailsWhenMethodIsMissing()
 
  public function testReplaceArgument()
  {
+ $this->containerBuilder->getDefinition('id1')->willReturn($this->builderDefinition);
+ $this->containerBuilder->getDefinition('id2')->willReturn($this->builderDefinition);
+
  $taggedServiceIds = array(
  'id1' => array(array('alias' => 'foo', 'method' => 'fooMenu'), array('alias' => 'bar', 'method' => 'bar')),
  'id2' => array(array('alias' => 'foo', 'method' => 'fooBar'), array('alias' => 'baz', 'method' => 'bar')),