Merge branch 'hotfix/347'

Close #405
Fixes #347

Author: weierophinney

CHANGELOG.md

@@ -25,6 +25,14 @@ All notable changes to this project will be documented in this file, in reverse
     is not available.
   - no router is specified, and the class `Zend\Expressive\Router\FastRouteRouter`
     is not available.
+- [#405](https://github.com/zendframework/zend-expressive/pull/405) fixes how
+  the `TemplatedErrorHandler` injects templated content into the response.
+  Previously, it would `write()` directly to the existing response body, which
+  could lead to issues if previous middleware had written to the response (as
+  the templated contents would append the previous contents). With this release,
+  it now creates a new `Zend\Diactoros\Stream`, writes to that, and returns a
+  new response with that new stream, guaranteeing it only contains the new
+  contents.
 
 ## 1.0.4 - 2016-12-07
 

src/TemplatedErrorHandler.php

@@ -9,6 +9,7 @@
 
 use Psr\Http\Message\RequestInterface as Request;
 use Psr\Http\Message\ResponseInterface as Response;
+use Zend\Diactoros\Stream;
 use Zend\Stratigility\Utils;
 
 /**
@@ -136,7 +137,8 @@ public function __invoke(Request $request, Response $response, $err = null)
     protected function handleError($error, Request $request, Response $response)
     {
         if ($this->renderer) {
-            $response->getBody()->write(
+            $stream = new Stream('php://temp', 'wb+');
+            $stream->write(
                 $this->renderer->render($this->templateError, [
                     'uri'      => $request->getUri(),
                     'error'    => $error,
@@ -146,6 +148,7 @@ protected function handleError($error, Request $request, Response $response)
                     'response' => $response,
                 ])
             );
+            return $response->withBody($stream);
         }
 
         return $response;
@@ -248,9 +251,11 @@ private function marshalReceivedResponse(Request $request, Response $response)
     private function create404(Request $request, Response $response)
     {
         if ($this->renderer) {
-            $response->getBody()->write(
+            $stream = new Stream('php://temp', 'wb+');
+            $stream->write(
                 $this->renderer->render($this->template404, [ 'uri' => $request->getUri() ])
             );
+            $response = $response->withBody($stream);
         }
         return $response->withStatus(404);
     }

test/TemplatedErrorHandlerTest.php

@@ -202,21 +202,32 @@ public function testInvocationWithoutErrorAndEmptyResponseCanReturnTemplated404R
             'error::500'
         );
 
-        $expected = $this->getResponse($this->getStream());
-
         $stream   = $this->getStream();
         $stream->getSize()->willReturn(0);
-        $stream->write('Templated contents')->shouldBeCalled();
 
-        $response = $this->getResponse($stream);
+        $response = $this->prophesize(ResponseInterface::class);
         $response->getStatusCode()->willReturn(200);
-        $response->withStatus(404)->willReturn($expected->reveal());
+        $response->getBody()->will([$stream, 'reveal']);
+        $response
+            ->withBody(Argument::that(function ($body) use ($stream) {
+                if (! $body instanceof StreamInterface) {
+                    return false;
+                }
+
+                if ($body === $stream) {
+                    return false;
+                }
+
+                return 'Templated contents' === (string) $body;
+            }))
+            ->will([$response, 'reveal']);
+        $response->withStatus(404)->will([$response, 'reveal']);
 
         $request = $this->getRequest($this->getStream());
         $request->getUri()->shouldBeCalled();
 
         $result = $handler($request->reveal(), $response->reveal());
-        $this->assertSame($expected->reveal(), $result);
+        $this->assertSame($response->reveal(), $result);
     }
 
     /**
@@ -238,15 +249,26 @@ public function testInvocationWithoutErrorAndResponseSameAsOriginalCanReturnTemp
             'error::500'
         );
 
-        $expected = $this->getResponse($this->getStream());
-
         $stream   = $this->getStream();
         $stream->getSize()->willReturn(100);
-        $stream->write('Templated contents')->shouldBeCalled();
 
-        $response = $this->getResponse($stream);
+        $response = $this->prophesize(ResponseInterface::class);
         $response->getStatusCode()->willReturn(200);
-        $response->withStatus(404)->willReturn($expected->reveal());
+        $response->getBody()->will([$stream, 'reveal']);
+        $response
+            ->withBody(Argument::that(function ($body) use ($stream) {
+                if (! $body instanceof StreamInterface) {
+                    return false;
+                }
+
+                if ($body === $stream) {
+                    return false;
+                }
+
+                return 'Templated contents' === (string) $body;
+            }))
+            ->will([$response, 'reveal']);
+        $response->withStatus(404)->will([$response, 'reveal']);
 
         $handler->setOriginalResponse($response->reveal());
 
@@ -257,7 +279,7 @@ public function testInvocationWithoutErrorAndResponseSameAsOriginalCanReturnTemp
             ->will([$response, 'reveal']);
 
         $result = $handler($request->reveal(), $response->reveal());
-        $this->assertSame($expected->reveal(), $result);
+        $this->assertSame($response->reveal(), $result);
     }
 
     /* Tests covering handleErrorResponse() paths; NO TEMPLATE */
@@ -361,54 +383,35 @@ public function testInvalidExceptionErrorCodeDefaultsToHTTP500Status($code)
 
     /* Tests covering handleErrorResponse() paths; WITH TEMPLATE */
 
-    /**
-     * @group templated
-     */
-    public function testNonExceptionErrorReturnsResponseWith500StatusAndTemplateResultsWhenTemplatingInjected()
+    public function errors()
     {
-        $renderer = $this->getTemplateImplementation();
-        $renderer
-            ->render(
-                'error::500',
-                Argument::type('array')
-            )
-            ->willReturn('Templated contents');
-
-        $handler = new TemplatedErrorHandler(
-            $renderer->reveal(),
-            'error::404',
-            'error::500'
-        );
-
-        $stream   = $this->getStream();
-        $stream->getSize()->willReturn(0);
-        $stream->write('Templated contents')->shouldBeCalled();
-
-        $expected = $this->getResponse($stream);
-        $expected->getStatusCode()->willReturn(500)->shouldBeCalled();
-        $expected->getReasonPhrase()->shouldBeCalled();
-
-        $response = $this->getResponse($stream);
-        $response->getStatusCode()->willReturn(200);
-        $response->withStatus(500)->willReturn($expected->reveal());
-
-        $request = $this->getRequest($this->getStream());
-        $request->getUri()->shouldBeCalled();
-
-        $result = $handler($request->reveal(), $response->reveal(), 'error');
-        $this->assertSame($expected->reveal(), $result);
+        return [
+            'error'     => ['error'],
+            'exception' => [new Exception('exception')],
+        ];
     }
 
     /**
      * @group templated
+     * @dataProvider errors
      */
-    public function testExceptionErrorReturnsResponseWith500StatusAndTemplateResultsWhenTemplatingInjected()
+    public function testReturns500ResponseWithNewStreamWhenReturningAnErrorResponse($error)
     {
         $renderer = $this->getTemplateImplementation();
         $renderer
             ->render(
                 'error::500',
-                Argument::type('array')
+                Argument::that(function ($params) use ($error) {
+                    if (! is_array($params)) {
+                        return false;
+                    }
+
+                    if (empty($params['error'])) {
+                        return false;
+                    }
+
+                    return $error === $params['error'];
+                })
             )
             ->willReturn('Templated contents');
 
@@ -418,22 +421,27 @@ public function testExceptionErrorReturnsResponseWith500StatusAndTemplateResults
             'error::500'
         );
 
-        $stream   = $this->getStream();
-        $stream->getSize()->willReturn(0);
-        $stream->write('Templated contents')->shouldBeCalled();
-
-        $expected = $this->getResponse($stream);
+        $expected = $this->prophesize(ResponseInterface::class);
         $expected->getStatusCode()->willReturn(500)->shouldBeCalled();
         $expected->getReasonPhrase()->shouldBeCalled();
+        $expected
+            ->withBody(Argument::that(function ($stream) {
+                if (! $stream instanceof StreamInterface) {
+                    return false;
+                }
 
-        $response = $this->getResponse($stream);
+                return 'Templated contents' === (string) $stream;
+            }))
+            ->will([$expected, 'reveal']);
+
+        $response = $this->prophesize(ResponseInterface::class);
         $response->getStatusCode()->willReturn(200);
         $response->withStatus(500)->willReturn($expected->reveal());
 
         $request = $this->getRequest($this->getStream());
         $request->getUri()->shouldBeCalled();
 
-        $result = $handler($request->reveal(), $response->reveal(), new Exception());
+        $result = $handler($request->reveal(), $response->reveal(), $error);
         $this->assertSame($expected->reveal(), $result);
     }
 }