Merging develop to master in preparation for 3.2.0 release.

CHANGELOG.md

@@ -6,15 +6,59 @@ Versions prior to 1.0 were originally released as `phly/conduit`; please visit
 its [CHANGELOG](https://github.com/phly/conduit/blob/master/CHANGELOG.md) for
 details.
 
-## 3.1.1 - TBD
+## 3.2.0 - 2019-06-12
 
 ### Added
 
 - Nothing.
 
 ### Changed
 
-- Nothing.
+- [#186](https://github.com/zendframework/zend-stratigility/pull/186) adds a safeguard to middleware pipes to prevent them from being called
+  multiple times within the same middleware. As an example, consider the
+  following middleware:
+
+  ```php
+  public function process(
+      ServerRequestInterface $request,
+      RequestHandlerInterface $handler
+  ) : Response Interface {
+      $session = $request->getAttribute('session');
+      if (! $session) {
+          $response = $handler->handle($request);
+      }
+
+      // Inject another attribute before handling
+      $response = $handler->handle($request->withAttribute(
+          'sessionWasEnabled',
+          true
+      );
+      return $response;
+  }
+  ```
+
+  When using Stratigility, the `$handler` is an instance of
+  `Zend\Stratigility\Next`, which encapsulates the middleware pipeline and
+  advances through it on each call to `handle()`.
+
+  The example demonstrates a subtle error: the response from the first
+  conditional should have been returned, but wasn't, which has led to invoking
+  the handler a second time. This scenario can have unexpected behaviors,
+  including always returning a "not found" response, or returning a response
+  from a handler that was not supposed to execute (as an earlier middleware
+  already returned early in the original call).
+
+  These bugs are hard to locate, as calling `handle()` is a normal part of any
+  middleware, and multiple conditional calls to it are a standard workflow.
+
+  With this new version, `Next` will pass a **clone** of itself to the next
+  middleware in the pipeline, and unset its own internal pipeline queue. Any
+  subsequent requests to `handle()` within the same scope will therefore result
+  in the exception `Zend\Stratigility\Exception\MiddlewarePipeNextHandlerAlreadyCalledException`.
+
+  If you depended on calling `$handler->handle()` multiple times in succession
+  within middleware, we recommend that you compose the specific pipeline(s)
+  and/or handler(s) you wish to call as class dependencies.
 
 ### Deprecated
 
@@ -909,26 +953,6 @@ details.
 
 - Nothing.
 
-## 1.2.2 - TBD
-
-### Added
-
-- [#58](https://github.com/zendframework/zend-stratigility/pull/58) updates the
-  documentation to use mkdocs for generation, and pushes the documentation to
-  https://zendframework.github.io/zend-stratigility/
-
-### Deprecated
-
-- Nothing.
-
-### Removed
-
-- Nothing.
-
-### Fixed
-
-- Nothing.
-
 ## 1.2.1 - 2016-03-24
 
 ### Added

composer.json

@@ -58,8 +58,8 @@
     },
     "extra": {
         "branch-alias": {
-            "dev-master": "3.1.x-dev",
-            "dev-develop": "3.2.x-dev"
+            "dev-master": "3.2.x-dev",
+            "dev-develop": "3.3.x-dev"
         }
     },
     "scripts": {

src/Exception/MiddlewarePipeNextHandlerAlreadyCalledException.php

@@ -0,0 +1,21 @@
+<?php
+/**
+ * @see       https://github.com/zendframework/zend-stratigility for the canonical source repository
+ * @copyright Copyright (c) 2019 Zend Technologies USA Inc. (https://www.zend.com)
+ * @license   https://github.com/zendframework/zend-stratigility/blob/master/LICENSE.md New BSD License
+ */
+
+declare(strict_types=1);
+
+namespace Zend\Stratigility\Exception;
+
+use DomainException;
+
+class MiddlewarePipeNextHandlerAlreadyCalledException extends DomainException implements ExceptionInterface
+{
+
+    public static function create(): self
+    {
+        return new self('Cannot invoke pipeline handler $handler->handle() more than once');
+    }
+}

src/Next.php

@@ -1,7 +1,7 @@
 <?php
 /**
  * @see       https://github.com/zendframework/zend-stratigility for the canonical source repository
- * @copyright Copyright (c) 2015-2018 Zend Technologies USA Inc. (https://www.zend.com)
+ * @copyright Copyright (c) 2015-2019 Zend Technologies USA Inc. (https://www.zend.com)
  * @license   https://github.com/zendframework/zend-stratigility/blob/master/LICENSE.md New BSD License
  */
 
@@ -13,6 +13,7 @@
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use SplQueue;
+use Zend\Stratigility\Exception\MiddlewarePipeNextHandlerAlreadyCalledException;
 
 /**
  * Iterate a queue of middlewares and execute them.
@@ -25,7 +26,7 @@ final class Next implements RequestHandlerInterface
     private $fallbackHandler;
 
     /**
-     * @var SplQueue
+     * @var null|SplQueue
      */
     private $queue;
 
@@ -43,12 +44,19 @@ public function __construct(SplQueue $queue, RequestHandlerInterface $fallbackHa
 
     public function handle(ServerRequestInterface $request) : ResponseInterface
     {
+        if ($this->queue === null) {
+            throw MiddlewarePipeNextHandlerAlreadyCalledException::create();
+        }
+
         if ($this->queue->isEmpty()) {
+            $this->queue = null;
             return $this->fallbackHandler->handle($request);
         }
 
         $middleware = $this->queue->dequeue();
+        $next = clone $this; // deep clone is not used intentionally
+        $this->queue = null; // mark queue as processed at this nesting level
 
-        return $middleware->process($request, $this);
+        return $middleware->process($request, $next);
     }
 }

test/NextTest.php

@@ -1,7 +1,7 @@
 <?php
 /**
  * @see       https://github.com/zendframework/zend-stratigility for the canonical source repository
- * @copyright Copyright (c) 2015-2018 Zend Technologies USA Inc. (https://www.zend.com)
+ * @copyright Copyright (c) 2015-2019 Zend Technologies USA Inc. (https://www.zend.com)
  * @license   https://github.com/zendframework/zend-stratigility/blob/master/LICENSE.md New BSD License
  */
 
@@ -18,9 +18,12 @@
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use SplQueue;
+use ZendTest\Stratigility\TestAsset\DelegatingMiddleware;
+use ZendTest\Stratigility\TestAsset\ShortCircuitingMiddleware;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\ServerRequest as Request;
 use Zend\Diactoros\Uri;
+use Zend\Stratigility\Exception\MiddlewarePipeNextHandlerAlreadyCalledException;
 use Zend\Stratigility\Next;
 
 class NextTest extends TestCase
@@ -214,4 +217,101 @@ public function testMiddlewareReturningResponseShortCircuitsProcess()
 
         $this->assertSame($response, $next->handle($this->request));
     }
+
+    public function testNextHandlerCannotBeInvokedTwice()
+    {
+        $fallbackHandler = $this->prophesize(RequestHandlerInterface::class);
+        $fallbackHandler
+            ->handle(Argument::any())
+            ->willReturn(new Response());
+
+        $this->queue->push(new DelegatingMiddleware());
+
+        $next = new Next($this->queue, $fallbackHandler->reveal());
+        $next->handle($this->request);
+
+        $this->expectException(MiddlewarePipeNextHandlerAlreadyCalledException::class);
+        $next->handle($this->request);
+    }
+
+    public function testSecondInvocationAttemptDoesNotInvokeFinalHandler()
+    {
+        $fallBackHandler = $this->prophesize(RequestHandlerInterface::class);
+        $fallBackHandler
+            ->handle(Argument::any())
+            ->willReturn(new Response())
+            ->shouldBeCalledTimes(1);
+
+        $this->queue->push(new DelegatingMiddleware());
+
+        $next = new Next($this->queue, $fallBackHandler->reveal());
+        $next->handle($this->request);
+
+        $this->expectException(MiddlewarePipeNextHandlerAlreadyCalledException::class);
+        $next->handle($this->request);
+    }
+
+    public function testSecondInvocationAttemptDoesNotInvokeMiddleware()
+    {
+        $fallBackHandler = $this->prophesize(RequestHandlerInterface::class);
+        $fallBackHandler
+            ->handle(Argument::any())
+            ->willReturn(new Response());
+
+        $middleware = $this->prophesize(MiddlewareInterface::class);
+        $middleware
+            ->process(
+                Argument::type(ServerRequestInterface::class),
+                Argument::type(RequestHandlerInterface::class)
+            )
+            ->will(function (array $args): ResponseInterface {
+                return $args[1]->handle($args[0]);
+            })
+            ->shouldBeCalledTimes(1);
+
+        $this->queue->push($middleware->reveal());
+
+        $next = new Next($this->queue, $fallBackHandler->reveal());
+        $next->handle($this->request);
+
+        $this->expectException(MiddlewarePipeNextHandlerAlreadyCalledException::class);
+        $next->handle($this->request);
+    }
+
+    public function testShortCircuitingMiddlewareDoesNotEnableSecondInvocation()
+    {
+        $fallBackHandler = $this->prophesize(RequestHandlerInterface::class);
+        $fallBackHandler
+            ->handle(Argument::any())
+            ->willReturn(new Response())
+            ->shouldNotBeCalled();
+
+        $this->queue->push(new ShortCircuitingMiddleware());
+
+        // The middleware above shorcircuits (when handler is invoked first)
+        // The middleware below still exists in the queue (when handler is invoked again)
+        $this->queue->push(new DelegatingMiddleware());
+
+        $next = new Next($this->queue, $fallBackHandler->reveal());
+        $next->handle($this->request);
+
+        $this->expectException(MiddlewarePipeNextHandlerAlreadyCalledException::class);
+        $next->handle($this->request);
+    }
+
+    public function testSecondInvocationAttemptWithEmptyQueueDoesNotInvokeFinalHandler()
+    {
+        $fallBackHandler = $this->prophesize(RequestHandlerInterface::class);
+        $fallBackHandler
+            ->handle(Argument::any())
+            ->willReturn(new Response())
+            ->shouldBeCalledTimes(1);
+
+        $next = new Next($this->queue, $fallBackHandler->reveal());
+
+        $next->handle($this->request);
+
+        $this->expectException(MiddlewarePipeNextHandlerAlreadyCalledException::class);
+        $next->handle($this->request);
+    }
 }

test/TestAsset/DelegatingMiddleware.php

@@ -0,0 +1,23 @@
+<?php
+/**
+ * @see       https://github.com/zendframework/zend-stratigility for the canonical source repository
+ * @copyright Copyright (c) 2019 Zend Technologies USA Inc. (https://www.zend.com)
+ * @license   https://github.com/zendframework/zend-stratigility/blob/master/LICENSE.md New BSD License
+ */
+
+declare(strict_types=1);
+
+namespace ZendTest\Stratigility\TestAsset;
+
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\MiddlewareInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+
+class DelegatingMiddleware implements MiddlewareInterface
+{
+    public function process(ServerRequestInterface $req, RequestHandlerInterface $handler): ResponseInterface
+    {
+        return $handler->handle($req);
+    }
+}

test/TestAsset/ShortCircuitingMiddleware.php

@@ -0,0 +1,24 @@
+<?php
+/**
+ * @see       https://github.com/zendframework/zend-stratigility for the canonical source repository
+ * @copyright Copyright (c) 2019 Zend Technologies USA Inc. (https://www.zend.com)
+ * @license   https://github.com/zendframework/zend-stratigility/blob/master/LICENSE.md New BSD License
+ */
+
+declare(strict_types=1);
+
+namespace ZendTest\Stratigility\TestAsset;
+
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\MiddlewareInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+use Zend\Diactoros\Response;
+
+class ShortCircuitingMiddleware implements MiddlewareInterface
+{
+    public function process(ServerRequestInterface $req, RequestHandlerInterface $handler): ResponseInterface
+    {
+        return new Response();
+    }
+}