FinalHandler should return new responses

If no error is present, and the response passed on invocation is
different than the response at initialization, the `FinalHandler` should
return that response intact.

This allows middleware to create a response, but pass it on to `$next()`
for post-processing further up the chain. In particular, this is useful
for things like routing middleware, so as to allow for application-wide
concerns such as logging, finalizing sessions, etc., without requiring
onion-style middleware for these behaviors.

src/FinalHandler.php

@@ -10,6 +10,8 @@
 namespace Zend\Stratigility;
 
 use Exception;
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
 use Zend\Escaper\Escaper;
 
 /**
@@ -22,34 +24,55 @@ class FinalHandler
      */
     private $options;
 
+    /**
+     * Original response provided to the middleware.
+     *
+     * @var null|Http\Response
+     */
+    private $response;
+
     /**
      * @param array $options Options that change default override behavior.
+     * @param null|ResponseInterface $response Original response, if any.
      */
-    public function __construct(array $options = [])
+    public function __construct(array $options = [], ResponseInterface $response = null)
     {
-        $this->options = $options;
+        $this->options  = $options;
+        $this->response = $response;
     }
 
     /**
      * Handle incomplete requests
      *
      * This handler should only ever be invoked if Next exhausts its stack.
-     * When that happens, we determine if an $err is present, and, if so,
-     * create a 500 status with error details.
      *
-     * Otherwise, a 404 status is created.
+     * When that happens, one of three possibilities exists:
+     *
+     * - If an $err is present, create a 500 status with error details.
+     * - If the instance composes a response, and it differs from the response
+     *   passed during invocation, return the invocation response; this is
+     *   indicative of middleware calling $next to allow post-processing of
+     *   a populated response.
+     * - Otherwise, a 404 status is created.
      *
-     * @param Http\Request $request Request instance.
-     * @param Http\Response $response Response instance.
+     * @param RequestInterface $request Request instance.
+     * @param ResponseInterface $response Response instance.
      * @param mixed $err
-     * @return Http\Response
+     * @return ResponseInterface
      */
-    public function __invoke(Http\Request $request, Http\Response $response, $err = null)
+    public function __invoke(RequestInterface $request, ResponseInterface $response, $err = null)
     {
         if ($err) {
             return $this->handleError($err, $request, $response);
         }
 
+        // Return provided response if it does not match the one provided at
+        // instantiation; this is an indication of calling `$next` in the fina
+        // registered middleware and providing a new response instance.
+        if ($this->response && $this->response !== $response) {
+            return $response;
+        }
+
         return $this->create404($request, $response);
     }
 
@@ -59,11 +82,11 @@ public function __invoke(Http\Request $request, Http\Response $response, $err =
      * Use the $error to create details for the response.
      *
      * @param mixed $error
-     * @param Http\Request $request Request instance.
-     * @param Http\Response $response Response instance.
+     * @param RequestInterface $request Request instance.
+     * @param ResponseInterface $response Response instance.
      * @return Http\Response
      */
-    private function handleError($error, Http\Request $request, Http\Response $response)
+    private function handleError($error, RequestInterface $request, ResponseInterface $response)
     {
         $response = $response->withStatus(
             Utils::getStatusCode($error, $response)
@@ -76,7 +99,7 @@ private function handleError($error, Http\Request $request, Http\Response $respo
             $message = $this->createDevelopmentErrorMessage($error);
         }
 
-        $response = $response->end($message);
+        $response = $this->completeResponse($response, $message);
 
         $this->triggerError($error, $request, $response);
 
@@ -86,24 +109,41 @@ private function handleError($error, Http\Request $request, Http\Response $respo
     /**
      * Create a 404 status in the response
      *
-     * @param Http\Request $request Request instance.
-     * @param Http\Response $response Response instance.
+     * @param RequestInterface $request Request instance.
+     * @param ResponseInterface $response Response instance.
      * @return Http\Response
      */
-    private function create404(Http\Request $request, Http\Response $response)
+    private function create404(RequestInterface $request, ResponseInterface $response)
     {
         $response        = $response->withStatus(404);
-        $originalRequest = $request->getOriginalRequest();
-        $uri             = $originalRequest->getUri();
+        $uri             = $this->getUriFromRequest($request);
         $escaper         = new Escaper();
         $message         = sprintf(
             "Cannot %s %s\n",
             $escaper->escapeHtml($request->getMethod()),
             $escaper->escapeHtml((string) $uri)
         );
-        return $response->end($message);
+
+        return $this->completeResponse($response, $message);
     }
 
+    /**
+     * Create a complete error message for development purposes.
+     *
+     * Creates an error message with full error details:
+     *
+     * - If the error is an exception, creates a message that includes the full
+     *   stack trace.
+     * - If the error is an object that defines `__toString()`, creates a
+     *   message by casting the error to a string.
+     * - If the error is not an object, casts the error to a string.
+     * - Otherwise, cerates a generic error message indicating the class type.
+     *
+     * In all cases, the error message is escaped for use in HTML.
+     *
+     * @param mixed $error
+     * @return string
+     */
     private function createDevelopmentErrorMessage($error)
     {
         if ($error instanceof Exception) {
@@ -122,11 +162,17 @@ private function createDevelopmentErrorMessage($error)
     /**
      * Trigger the error listener, if present
      *
+     * If no `onerror` option is present, or if it is not callable, does
+     * nothing.
+     *
+     * If the request is not an Http\Request, casts it to one prior to invoking
+     * the error handler.
+     *
      * @param mixed $error
-     * @param Http\Request $request
+     * @param RequestInterface $request
      * @param Http\Response $response
      */
-    private function triggerError($error, Http\Request $request, Http\Response $response)
+    private function triggerError($error, RequestInterface $request, Http\Response $response)
     {
         if (! isset($this->options['onerror'])
             || ! is_callable($this->options['onerror'])
@@ -135,6 +181,49 @@ private function triggerError($error, Http\Request $request, Http\Response $resp
         }
 
         $onError = $this->options['onerror'];
-        $onError($error, $request, $response);
+        $onError(
+            $error,
+            ($request instanceof Http\Request) ? $request : new Http\Request($request),
+            $response
+        );
+    }
+
+    /**
+     * Retrieve the URI from the request.
+     *
+     * If the request instance is a Stratigility decorator, pull the URI from
+     * the original request; otherwise, pull it directly.
+     *
+     * @param RequestInterface $request
+     * @return \Psr\Http\Message\UriInterface
+     */
+    private function getUriFromRequest(RequestInterface $request)
+    {
+        if ($request instanceof Http\Request) {
+            $original = $request->getOriginalRequest();
+            return $original->getUri();
+        }
+
+        return $request->getUri();
+    }
+
+    /**
+     * Write the given message to the response and mark it complete.
+     *
+     * If the message is an Http\Response decorator, call and return its
+     * `end()` method; otherwise, decorate the response and `end()` it.
+     *
+     * @param ResponseInterface $response
+     * @param string $message
+     * @return Http\Response
+     */
+    private function completeResponse(ResponseInterface $response, $message)
+    {
+        if ($response instanceof Http\Response) {
+            return $response->end($message);
+        }
+
+        $response = new Http\Response($response);
+        return $response->end($message);
     }
 }

src/MiddlewarePipe.php

@@ -71,7 +71,7 @@ public function __invoke(Request $request, Response $response, callable $out = n
         $request  = $this->decorateRequest($request);
         $response = $this->decorateResponse($response);
 
-        $done   = $out ?: new FinalHandler();
+        $done   = $out ?: new FinalHandler([], $response);
         $next   = new Next($this->pipeline, $done);
         $result = $next($request, $response);
 

test/FinalHandlerTest.php

@@ -121,4 +121,15 @@ public function test404ResponseIncludesOriginalRequestUri()
         $response = call_user_func($final, $request, $this->response, null);
         $this->assertContains($originalUrl, (string) $response->getBody());
     }
+
+    public function testReturnsResponseIfItDoesNotMatchResponsePassedToConstructor()
+    {
+        $psrResponse = new PsrResponse();
+        $originalResponse = new Response($psrResponse);
+        $final = new FinalHandler([], $originalResponse);
+
+        $passedResponse = new Response($psrResponse);
+        $result = $final(new Request(new PsrRequest()), $passedResponse);
+        $this->assertSame($passedResponse, $result);
+    }
 }

test/MiddlewarePipeTest.php

@@ -441,4 +441,36 @@ public function testNestedMiddlewareMatchesOnlyAtPathBoundaries($topPath, $neste
             )
         );
     }
+
+    /**
+     * Test that FinalHandler is passed the original response.
+     *
+     * Tests that MiddlewarePipe passes the original response passed to it when
+     * creating the FinalHandler instance, and that FinalHandler compares the
+     * response passed to it on invocation to its original response.
+     *
+     * If the two differ, the response passed during invocation should be
+     * returned unmodified; this is an indication that a middleware has provided
+     * a response, and is simply passing further up the chain to allow further
+     * processing (e.g., to allow an application-wide logger at the end of the
+     * request).
+     *
+     * @group nextChaining
+     */
+    public function testPassesOriginalResponseToFinalHandler()
+    {
+        $request  = new Request([], [], 'http://local.example.com/foo', 'GET', 'php://memory');
+        $response = new Response();
+        $test     = new Response();
+
+        $pipeline = new MiddlewarePipe();
+        $pipeline->pipe(function ($req, $res, $next) use ($test) {
+            return $next($req, $test);
+        });
+
+        // Pipeline MUST return response passed to $next if it differs from the
+        // original.
+        $result = $pipeline($request, $response);
+        $this->assertSame($test, $result);
+    }
 }