Add way to handle protected constructors on reflection safely

thephpleague · Jul 9, 2021 · 0efb82b · 0efb82b
1 parent 3a15637
commit 0efb82b
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 All Notable changes to `League\Container` will be documented in this file
 
+## 4.1.0
+
+### Added
+- Way to handle non-public controllers safely (@beryllium)
+
 ## 4.0.0
 
 ### Added
@@ -16,6 +21,12 @@ All Notable changes to `League\Container` will be documented in this file
 - `ServiceProviderInterface` now defines return types
 - Service providers now require implementation of a `provides` method rather than relying on a class property.
 
+## 3.4.1
+
+### Added
+- Way to handle non-public controllers safely (@beryllium)
+- PHPUnit ^7.0 for PHP versions that support it (@beryllium)
+
 ## 3.4.0
 
 ### Removed

diff --git a/docs/_data/releases.yml b/docs/_data/releases.yml
@@ -22,7 +22,7 @@
   name: League\Container 4.x
   type: Current
   requires: PHP >= 7.2
-  release: 4.0.0 - 2021-07
+  release: 4.1.0 - 2021-07
   support: Ongoing
   url: /4.x/
   menu:
@@ -40,7 +40,7 @@
   name: League\Container 3.x
   type: Old
   requires: PHP >= 7.0
-  release: 3.4.0 - 2021-07
+  release: 3.4.1 - 2021-07
   support: 2022-02
   url: /3.x/
   menu:

diff --git a/src/Argument/ArgumentResolverTrait.php b/src/Argument/ArgumentResolverTrait.php
@@ -41,20 +41,22 @@ public function resolveArguments(array $arguments): array
             // resolve the argument from the container, if it happens to be another
             // argument wrapper, use that value
             if ($container instanceof ContainerInterface && $container->has($argValue)) {
-                $arg = $container->get($argValue);
+                try {
+                    $arg = $container->get($argValue);
 
-                if ($arg instanceof ArgumentInterface) {
-                    $arg = $arg->getValue();
-                }
+                    if ($arg instanceof ArgumentInterface) {
+                        $arg = $arg->getValue();
+                    }
 
-                continue;
+                    continue;
+                } catch (NotFoundException $e) {
+                }
             }
 
             // if we have a default value, we use that, no more resolution as
             // we expect a default/optional argument value to be literal
             if ($arg instanceof DefaultValueInterface) {
                 $arg = $arg->getDefaultValue();
-                continue;
             }
         }
 

diff --git a/src/ReflectionContainer.php b/src/ReflectionContainer.php
@@ -38,7 +38,7 @@ public function get($id, array $args = [])
             return $this->cache[$id];
         }
 
-        if (! $this->has($id)) {
+        if (!$this->has($id)) {
             throw new NotFoundException(
                 sprintf('Alias (%s) is not an existing class and therefore cannot be resolved', $id)
             );
@@ -47,9 +47,15 @@ public function get($id, array $args = [])
         $reflector = new ReflectionClass($id);
         $construct = $reflector->getConstructor();
 
+        if ($construct && !$construct->isPublic()) {
+            throw new NotFoundException(
+                sprintf('Alias (%s) has a non-public constructor and therefore cannot be instantiated', $id)
+            );
+        }
+
         $resolution = $construct === null
             ? new $id()
-            : $resolution = $reflector->newInstanceArgs($this->reflectArguments($construct, $args))
+            : $reflector->newInstanceArgs($this->reflectArguments($construct, $args))
         ;
 
         if ($this->cacheResolutions === true) {

diff --git a/tests/Asset/ProBar.php b/tests/Asset/ProBar.php
@@ -0,0 +1,15 @@
+<?php
+
+namespace League\Container\Test\Asset;
+
+class ProBar implements BarInterface
+{
+    protected function __construct()
+    {
+    }
+
+    public static function factory(): ProBar
+    {
+        return new self();
+    }
+}
diff --git a/tests/Asset/ProFoo.php b/tests/Asset/ProFoo.php
@@ -0,0 +1,13 @@
+<?php
+
+namespace League\Container\Test\Asset;
+
+class ProFoo
+{
+    public $bar;
+
+    public function __construct(?ProBar $bar = null)
+    {
+        $this->bar = $bar;
+    }
+}
diff --git a/tests/ReflectionContainerTest.php b/tests/ReflectionContainerTest.php
@@ -7,7 +7,7 @@
 use League\Container\Container;
 use League\Container\Exception\NotFoundException;
 use League\Container\ReflectionContainer;
-use League\Container\Test\Asset\{Foo, FooCallable, Bar};
+use League\Container\Test\Asset\{Foo, FooCallable, Bar, ProFoo, ProBar};
 use PHPUnit\Framework\TestCase;
 
 class ReflectionContainerTest extends TestCase
@@ -191,4 +191,33 @@ public function testCallResolvesFunction(): void
         self::assertInstanceOf(Foo::class, $foo);
         self::assertInstanceOf(Bar::class, $foo->bar);
     }
+
+    public function testGetInstantiatesClassWithConstructorAndSkipsProtectedConstructor(): void
+    {
+        $classWithConstructor = ProFoo::class;
+
+        $container = new Container();
+        $container->delegate(new ReflectionContainer());
+
+        $item = $container->get($classWithConstructor);
+
+        $this->assertInstanceOf($classWithConstructor, $item);
+        $this->assertNull($item->bar);
+    }
+
+    public function testGetInstantiatesClassWithConstructorAndUsesFactory(): void
+    {
+        $classWithConstructor = ProFoo::class;
+        $dependencyClass = ProBar::class;
+
+        $container = new Container();
+        $container->delegate(new ReflectionContainer());
+
+        $container->add($dependencyClass, [$dependencyClass, 'factory']);
+
+        $item = $container->get($classWithConstructor);
+
+        $this->assertInstanceOf($classWithConstructor, $item);
+        $this->assertInstanceOf($dependencyClass, $item->bar);
+    }
 }