Intercept exceptions from the ServerRequest factory

As noted in zendframework/zend-diactoros#171, the
`ServerRequestFactory::fromGlobals()` method raises exceptions on
malformed information, such as:

- malformed file upload data
- invalid HTTP protocol versions

At the application level, we should catch these, and invoke the final
handler with a 400 response when caught.

Author: weierophinney

composer.json

@@ -31,7 +31,8 @@
         "zendframework/zend-expressive-aurarouter": "^1.0",
         "zendframework/zend-expressive-fastroute": "^1.0",
         "zendframework/zend-expressive-zendrouter": "^1.0",
-        "zendframework/zend-servicemanager": "^2.6"
+        "zendframework/zend-servicemanager": "^2.6",
+        "mockery/mockery": "^0.9.5"
     },
     "autoload": {
       "psr-4": {

src/Application.php

@@ -11,12 +11,15 @@
 
 use Interop\Container\ContainerInterface;
 use Interop\Container\Exception\ContainerException;
+use InvalidArgumentException;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
+use UnexpectedValueException;
 use Zend\Diactoros\Request;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\Response\EmitterInterface;
 use Zend\Diactoros\Response\SapiEmitter;
+use Zend\Diactoros\ServerRequest;
 use Zend\Diactoros\ServerRequestFactory;
 use Zend\Stratigility\MiddlewarePipe;
 
@@ -99,6 +102,13 @@ class Application extends MiddlewarePipe implements Router\RouteResultSubjectInt
      */
     private $routes = [];
 
+    /**
+     * Internal; factory class for server request (for mocking)
+     *
+     * @var string
+     */
+    private $serverRequestFactory = ServerRequestFactory::class;
+
     /**
      * Constructor
      *
@@ -552,10 +562,21 @@ public function route($path, $middleware = null, array $methods = null, $name =
      */
     public function run(ServerRequestInterface $request = null, ResponseInterface $response = null)
     {
-        $request  = $request ?: ServerRequestFactory::fromGlobals();
-        $response = $response ?: new Response();
+        $factory = $this->serverRequestFactory;
+
+        try {
+            $request  = $request ?: $factory::fromGlobals();
+        } catch (InvalidArgumentException $e) {
+            // Unable to parse uploaded files
+            $this->emitMarshalServerRequestException($e);
+            return;
+        } catch (UnexpectedValueException $e) {
+            // Invalid request method
+            $this->emitMarshalServerRequestException($e);
+            return;
+        }
 
-        $response = $this($request, $response);
+        $response = $this($request, $response ?: new Response());
 
         $emitter = $this->getEmitter();
         $emitter->emit($response);
@@ -654,4 +675,18 @@ private function checkForDuplicateRoute($path, $methods = null)
             );
         }
     }
+
+    /**
+     * @var \Exception|\Throwable $exception
+     * @return void
+     */
+    private function emitMarshalServerRequestException($exception)
+    {
+        $response = (new Response())
+            ->withStatus(400);
+        $finalHandler = $this->getFinalHandler();
+        $response = $finalHandler(new ServerRequest(), $response, $exception);
+        $emitter = $this->getEmitter();
+        $emitter->emit($response);
+    }
 }

test/ApplicationTest.php

@@ -12,12 +12,15 @@
 use BadMethodCallException;
 use DomainException;
 use Interop\Container\ContainerInterface;
+use InvalidArgumentException;
+use Mockery;
 use PHPUnit_Framework_TestCase as TestCase;
 use Prophecy\Argument;
 use Psr\Http\Message\ResponseInterface;
 use ReflectionProperty;
 use RuntimeException;
 use SplQueue;
+use UnexpectedValueException;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\Response\EmitterInterface;
 use Zend\Diactoros\Response\SapiEmitter;
@@ -622,4 +625,58 @@ public function testPipingNotInvokableErrorMiddlewareRisesException()
         $this->setExpectedException(InvalidMiddlewareException::class);
         $handler('foo', 'bar', 'baz', 'bat');
     }
+
+    public function invalidRequestExceptions()
+    {
+        return [
+            'invalid file'             => [
+                TestAsset\ServerRequestFactoryProducingInvalidFiles::class,
+                InvalidArgumentException::class,
+                'Invalid value in files specification',
+            ],
+            'invalid protocol version' => [
+                TestAsset\ServerRequestFactoryProducingUnknownProtocolVersion::class,
+                UnexpectedValueException::class,
+                'Unrecognized protocol version (foo-bar)',
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider invalidRequestExceptions
+     */
+    public function testRunInvokesFinalHandlerWhenServerRequestFactoryRaisesException(
+        $serverRequestFactory,
+        $expectedException,
+        $message
+    ) {
+        $requestFactory = Mockery::mock($serverRequestFactory)
+            ->shouldReceive('fromGlobals')
+            ->withNoArgs()
+            ->andThrow($expectedException, $message)
+            ->once()
+            ->getMock();
+
+        $finalHandler = function ($request, $response, $err = null) use ($expectedException, $message) {
+            $this->assertEquals(400, $response->getStatusCode());
+            $this->assertInstanceOf($expectedException, $err);
+            $this->assertEquals($message, $err->getMessage());
+            return $response;
+        };
+
+        $emitter = $this->prophesize(EmitterInterface::class);
+        $emitter->emit(Argument::that(function ($response) {
+            $this->assertInstanceOf(ResponseInterface::class, $response, 'Emitter did not receive a response');
+            $this->assertEquals(400, $response->getStatusCode());
+            return true;
+        }))->shouldBeCalled();
+
+        $app = new Application($this->router->reveal(), null, $finalHandler, $emitter->reveal());
+
+        $r = new ReflectionProperty($app, 'serverRequestFactory');
+        $r->setAccessible(true);
+        $r->setValue($app, $requestFactory);
+
+        $app->run();
+    }
 }

test/TestAsset/ServerRequestFactoryProducingInvalidFiles.php

@@ -0,0 +1,26 @@
+<?php
+/**
+ * @link      http://github.com/zendframework/zend-expresive for the canonical source repository
+ * @copyright Copyright (c) 2016 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license   http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace ZendTest\Expressive\TestAsset;
+
+/**
+ * Required due to limitations in Mockery when mocking static methods.
+ *
+ * When mocking static methods, the first mock created "wins", meaning
+ * its behavior is used for all subsequent calls. In order to test
+ * multiple different behaviors, we need multiple classes that mimic
+ * the method.
+ */
+class ServerRequestFactoryProducingInvalidFiles
+{
+    /**
+     * Method to mock as ServerRequest factory.
+     */
+    public static function fromGlobals()
+    {
+    }
+}

test/TestAsset/ServerRequestFactoryProducingUnknownProtocolVersion.php

@@ -0,0 +1,26 @@
+<?php
+/**
+ * @link      http://github.com/zendframework/zend-expresive for the canonical source repository
+ * @copyright Copyright (c) 2016 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license   http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace ZendTest\Expressive\TestAsset;
+
+/**
+ * Required due to limitations in Mockery when mocking static methods.
+ *
+ * When mocking static methods, the first mock created "wins", meaning
+ * its behavior is used for all subsequent calls. In order to test
+ * multiple different behaviors, we need multiple classes that mimic
+ * the method.
+ */
+class ServerRequestFactoryProducingUnknownProtocolVersion
+{
+    /**
+     * Method to mock as ServerRequest factory.
+     */
+    public static function fromGlobals()
+    {
+    }
+}