Avoid loading internal classes from PSR-11 container

docs/best-practices/controllers.md

@@ -482,3 +482,14 @@ $app = new FrameworkX\App(new FrameworkX\Container($container));
 We expect most applications to work just fine with the built-in DI container.
 If you need to use a custom container, the above logic should work with any of the
 [PSR-11 container implementations](https://packagist.org/providers/psr/container-implementation).
+
+X requests any of your custom classes, such as your controllers and middleware,
+as well as any configuration such as environment variables from the given
+container, which is then responsible for loading these and their dependencies.
+X only creates some of its internal classes itself, such as its HTTP server
+runner, as autowiring containers may otherwise create these incorrectly.
+
+If you want to configure the HTTP server runner, such as for a larger
+[request body limit](deployment.md#request-body-limit), you can define it in
+your custom container and explicitly load it by setting `X_EXPERIMENTAL_RUNNER`
+to its class name instead of the default `null` value.

src/Container.php

@@ -150,7 +150,8 @@ public function getEnv(string $name): ?string
      */
     public function getObject(string $class) /*: object (PHP 7.2+) */
     {
-        if ($this->container instanceof ContainerInterface && $this->container->has($class)) {
+        // never load Container itself or RouteHandler from PSR-11 adapter, as autowiring containers may otherwise create these incorrectly
+        if ($this->container instanceof ContainerInterface && !\in_array($class, [self::class, RouteHandler::class], true) && $this->container->has($class)) {
             $value = $this->container->get($class);
             if (!$value instanceof $class) {
                 throw new \TypeError(
@@ -160,7 +161,7 @@ public function getObject(string $class) /*: object (PHP 7.2+) */
             return $value;
         } elseif ($this->container instanceof ContainerInterface) {
             // fallback for missing required internal classes from PSR-11 adapter
-            if ($class === Container::class) {
+            if ($class === self::class) {
                 return $this; // @phpstan-ignore-line returns instanceof `T`
             } elseif ($class === RouteHandler::class) {
                 return new RouteHandler($this); // @phpstan-ignore-line returns instanceof `T`
@@ -194,8 +195,14 @@ public function getObject(string $class) /*: object (PHP 7.2+) */
      */
     public function getRunner(): callable /*: HttpServerRunner|SapiRunner|callable (PHP 8.0+) */
     {
+        $class = $this->getEnv('X_EXPERIMENTAL_RUNNER');
+        if ($class === null && \PHP_SAPI === 'cli' && $this->container instanceof ContainerInterface) {
+            // always create default runner for PSR-11 adapter, as autowiring containers may otherwise fail to create it
+            return new HttpServerRunner(new LogStreamHandler('php://output'), $this->getEnv('X_LISTEN'));
+        }
+
         // @phpstan-ignore-next-line `getObject()` already performs type checks if `getEnv()` returns an invalid class
-        $runner = $this->getObject($this->getEnv('X_EXPERIMENTAL_RUNNER') ?? (\PHP_SAPI === 'cli' ? HttpServerRunner::class : SapiRunner::class));
+        $runner = $this->getObject($class ?? (\PHP_SAPI === 'cli' ? HttpServerRunner::class : SapiRunner::class));
         if (!\is_callable($runner)) {
             throw new \TypeError(
                 'Return value of ' . __METHOD__ . '() must be of type callable, ' . $this->gettype($runner) . ' returned'

tests/AppTest.php

@@ -22,6 +22,7 @@
 use FrameworkX\Tests\Fixtures\InvalidInterface;
 use FrameworkX\Tests\Fixtures\InvalidTrait;
 use PHPUnit\Framework\TestCase;
+use Psr\Container\ContainerInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use React\Http\Message\Response;
@@ -174,6 +175,44 @@ public function testConstructWithContainerInstanceAssignsDefaultHandlersAndConta
         $this->assertSame($container, $ref->getValue($routeHandler));
     }
 
+    public function testConstructWithPsrContainerAssignsDefaultRunnerAndRouteHandlerWithoutLoadingFromPsrContainer(): void
+    {
+        $psr = $this->createMock(ContainerInterface::class);
+        $psr->expects($this->exactly(4))->method('has')->willReturnMap([
+            [AccessLogHandler::class, true],
+            [ErrorHandler::class, true],
+            ['X_EXPERIMENTAL_RUNNER', false],
+            ['X_LISTEN', false],
+        ]);
+        $psr->expects($this->exactly(2))->method('get')->willReturnMap([
+            [AccessLogHandler::class, new AccessLogHandler()],
+            [ErrorHandler::class, new ErrorHandler()],
+        ]);
+        assert($psr instanceof ContainerInterface);
+
+        $container = new Container($psr);
+        $app = new App($container);
+
+        $ref = new \ReflectionProperty($app, 'runner');
+        if (PHP_VERSION_ID < 80100) {
+            $ref->setAccessible(true);
+        }
+        $this->assertInstanceOf(HttpServerRunner::class, $ref->getValue($app));
+
+        $ref = new ReflectionProperty($app, 'router');
+        if (PHP_VERSION_ID < 80100) {
+            $ref->setAccessible(true);
+        }
+        $routeHandler = $ref->getValue($app);
+        assert($routeHandler instanceof RouteHandler);
+
+        $ref = new ReflectionProperty($routeHandler, 'container');
+        if (PHP_VERSION_ID < 80100) {
+            $ref->setAccessible(true);
+        }
+        $this->assertSame($container, $ref->getValue($routeHandler));
+    }
+
     public function testConstructWithContainerAndMiddlewareClassNameAssignsCallableFromContainerAsMiddleware(): void
     {
         $middleware = function (ServerRequestInterface $request, callable $next) { };

tests/ContainerTest.php

@@ -3026,10 +3026,10 @@ public function testGetObjectReturnsDefaultAccessLogHandlerInstanceIfPsrContaine
         $this->assertInstanceOf(AccessLogHandler::class, $accessLogHandler);
     }
 
-    public function testGetObjectReturnsSelfContainerIfPsrContainerHasNoEntry(): void
+    public function testGetObjectReturnsSelfContainerWithoutLoadingFromPsrContainer(): void
     {
         $psr = $this->createMock(ContainerInterface::class);
-        $psr->expects($this->once())->method('has')->with(Container::class)->willReturn(false);
+        $psr->expects($this->never())->method('has');
         $psr->expects($this->never())->method('get');
 
         assert($psr instanceof ContainerInterface);
@@ -3040,10 +3040,10 @@ public function testGetObjectReturnsSelfContainerIfPsrContainerHasNoEntry(): voi
         $this->assertSame($container, $ret);
     }
 
-    public function testGetObjectReturnsDefaultRouteHandlerInstanceIfPsrContainerHasNoEntry(): void
+    public function testGetObjectReturnsDefaultRouteHandlerInstanceWithoutLoadingFromPsrContainer(): void
     {
         $psr = $this->createMock(ContainerInterface::class);
-        $psr->expects($this->once())->method('has')->with(RouteHandler::class)->willReturn(false);
+        $psr->expects($this->never())->method('has');
         $psr->expects($this->never())->method('get');
 
         assert($psr instanceof ContainerInterface);
@@ -3259,16 +3259,19 @@ public function testGetRunnerReturnsHttpServerRunnerInstanceFromConfig(): void
         $this->assertSame($runner, $ret);
     }
 
-    public function testGetRunnerReturnsHttpServerRunnerInstanceFromPsrContainer(): void
+    public function testGetRunnerReturnsHttpServerRunnerInstanceFromPsrContainerIfExplicitlyRequested(): void
     {
         $runner = new HttpServerRunner(new LogStreamHandler('php://output'), null);
 
         $psr = $this->createMock(ContainerInterface::class);
         $psr->expects($this->exactly(2))->method('has')->willReturnMap([
-            ['X_EXPERIMENTAL_RUNNER', false],
+            ['X_EXPERIMENTAL_RUNNER', true],
             [HttpServerRunner::class, true],
         ]);
-        $psr->expects($this->once())->method('get')->with(HttpServerRunner::class)->willReturn($runner);
+        $psr->expects($this->exactly(2))->method('get')->willReturnMap([
+            ['X_EXPERIMENTAL_RUNNER', HttpServerRunner::class],
+            [HttpServerRunner::class, $runner],
+        ]);
 
         assert($psr instanceof ContainerInterface);
         $container = new Container($psr);
@@ -3278,14 +3281,31 @@ public function testGetRunnerReturnsHttpServerRunnerInstanceFromPsrContainer():
         $this->assertSame($runner, $ret);
     }
 
-    public function testGetRunnerReturnsDefaultHttpServerRunnerInstanceWithDefaultListenAddressIfPsrContainerHasNoEntry(): void
+    public function testGetRunnerReturnsDefaultHttpServerRunnerInstanceIfExplicitlyRequestedButPsrContainerHasNoEntry(): void
     {
         $psr = $this->createMock(ContainerInterface::class);
         $psr->expects($this->exactly(3))->method('has')->willReturnMap([
-            ['X_EXPERIMENTAL_RUNNER', false],
+            ['X_EXPERIMENTAL_RUNNER', true],
             [HttpServerRunner::class, false],
             ['X_LISTEN', false],
         ]);
+        $psr->expects($this->once())->method('get')->with('X_EXPERIMENTAL_RUNNER')->willReturn(HttpServerRunner::class);
+
+        assert($psr instanceof ContainerInterface);
+        $container = new Container($psr);
+
+        $runner = $container->getRunner();
+
+        $this->assertInstanceOf(HttpServerRunner::class, $runner);
+    }
+
+    public function testGetRunnerReturnsDefaultHttpServerRunnerInstanceWithDefaultListenAddressWithoutLoadingFromPsrContainer(): void
+    {
+        $psr = $this->createMock(ContainerInterface::class);
+        $psr->expects($this->exactly(2))->method('has')->willReturnMap([
+            ['X_EXPERIMENTAL_RUNNER', false],
+            ['X_LISTEN', false],
+        ]);
         $psr->expects($this->never())->method('get');
 
         assert($psr instanceof ContainerInterface);
@@ -3305,12 +3325,11 @@ public function testGetRunnerReturnsDefaultHttpServerRunnerInstanceWithDefaultLi
         $this->assertEquals('127.0.0.1:8080', $listenAddress);
     }
 
-    public function testGetRunnerReturnsDefaultHttpServerRunnerInstanceWithCustomListenAddressIfPsrContainerHasNoEntryButCustomListenAddress(): void
+    public function testGetRunnerReturnsDefaultHttpServerRunnerInstanceWithCustomListenAddressWithoutLoadingFromPsrContainer(): void
     {
         $psr = $this->createMock(ContainerInterface::class);
-        $psr->expects($this->exactly(3))->method('has')->willReturnMap([
+        $psr->expects($this->exactly(2))->method('has')->willReturnMap([
             ['X_EXPERIMENTAL_RUNNER', false],
-            [HttpServerRunner::class, false],
             ['X_LISTEN', true],
         ]);
         $psr->expects($this->once())->method('get')->with('X_LISTEN')->willReturn('127.0.0.1:8081');