Merge branch 'hotfix/527-pipe-with-something-looking-callable'

Close #534
Fixes #527

Author: weierophinney

CHANGELOG.md

@@ -30,6 +30,13 @@ All notable changes to this project will be documented in this file, in reverse
   requires it. This addition fixes problems due to missing http-middleware
   interfaces.
 
+- [#534](https://github.com/zendframework/zend-expressive/pull/534) provides a
+  fix for how it detects `callable` middleware. Previously, it relied on PHP's
+  `is_callable()`, but that function can result in false positives when provided
+  a 2-element array where the first element is an object, as the function does
+  not verify that the second argument is a valid method of the first. We now
+  implement additional verifications to prevent such false positives.
+
 ## 2.0.4 - 2017-10-09
 
 ### Added

src/IsCallableInteropMiddlewareTrait.php

@@ -13,6 +13,35 @@
 
 trait IsCallableInteropMiddlewareTrait
 {
+    /**
+     * Is the provided $middleware argument callable?
+     *
+     * Runs the argument against is_callable(). If that returns true, and the
+     * value is an array with two elements, tests to ensure that the second
+     * element is a method of the first.
+     *
+     * @param mixed $middleware
+     * @return bool
+     */
+    private function isCallable($middleware)
+    {
+        if (! is_callable($middleware)) {
+            return false;
+        }
+
+        if (! is_array($middleware)) {
+            return true;
+        }
+
+        $classOrObject = array_shift($middleware);
+        if (! is_object($classOrObject) && ! class_exists($classOrObject)) {
+            return false;
+        }
+
+        $method = array_shift($middleware);
+        return method_exists($classOrObject, $method);
+    }
+
     /**
      * Is callable middleware interop middleware?
      *
@@ -21,7 +50,7 @@ trait IsCallableInteropMiddlewareTrait
      */
     private function isCallableInteropMiddleware($middleware)
     {
-        if (! is_callable($middleware)) {
+        if (! $this->isCallable($middleware)) {
             return false;
         }
 

src/MarshalMiddlewareTrait.php

@@ -63,7 +63,7 @@ private function prepareMiddleware(
             return new CallableInteropMiddlewareWrapper($middleware);
         }
 
-        if (is_callable($middleware)) {
+        if ($this->isCallable($middleware)) {
             return new CallableMiddlewareWrapper($middleware, $responsePrototype);
         }
 

test/ApplicationTest.php

@@ -11,6 +11,7 @@
 use DomainException;
 use Fig\Http\Message\StatusCodeInterface as StatusCode;
 use Interop\Http\ServerMiddleware\DelegateInterface;
+use Interop\Http\ServerMiddleware\MiddlewareInterface;
 use InvalidArgumentException;
 use Mockery;
 use PHPUnit\Framework\TestCase;
@@ -19,6 +20,7 @@
 use Psr\Container\ContainerInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
+use ReflectionMethod;
 use ReflectionProperty;
 use RuntimeException;
 use UnexpectedValueException;
@@ -720,4 +722,26 @@ public function testAllowsNestedMiddlewarePipelines()
 
         $this->assertSame($response, $app->process($request, $delegate->reveal()));
     }
+
+    public function testPreparingArrayWithPairOfObjectAndStringMiddlewaresShouldNotBeTreatedAsCallable()
+    {
+        $first  = $this->prophesize(MiddlewareInterface::class)->reveal();
+        $second = TestAsset\CallableInteropMiddleware::class;
+        $queue  = [$first, $second];
+
+        $router = $this->router->reveal();
+        $response = $this->prophesize(ResponseInterface::class)->reveal();
+
+        $app = $this->getApp();
+        $r = new ReflectionMethod($app, 'prepareMiddleware');
+        $r->setAccessible(true);
+
+        $middleware = $r->invoke($app, $queue, $router, $response);
+
+        $this->assertInstanceOf(MiddlewarePipe::class, $middleware);
+
+        $r = new ReflectionProperty($middleware, 'pipeline');
+        $r->setAccessible(true);
+        $this->assertCount(2, $r->getValue($middleware));
+    }
 }