Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Commit

Permalink
Provides more robust is_callable logic
Browse files Browse the repository at this point in the history
Provides a new method, `IsCallableInteropMiddlewareTrait::isCallable()`,
which runs its argument through `is_callable()`. If the value passes, it
then checks to see if it is an array, and, if so, if:

- the first argument is an object or valid class name, AND
- the second argument is a valid method of the first argument.

This is done because `is_callable()` returns a false positive when a
two-element array is passed where the first argument is an object and
the second a string; the function does not verify that the second
element is a valid method of the first.
  • Loading branch information
weierophinney committed Dec 11, 2017
1 parent b404854 commit 63fe808
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 2 deletions.
31 changes: 30 additions & 1 deletion src/IsCallableInteropMiddlewareTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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?
*
Expand All @@ -21,7 +50,7 @@ trait IsCallableInteropMiddlewareTrait
*/
private function isCallableInteropMiddleware($middleware)
{
if (! is_callable($middleware)) {
if (! $this->isCallable($middleware)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/MarshalMiddlewareTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private function prepareMiddleware(
return new CallableInteropMiddlewareWrapper($middleware);
}

if (is_callable($middleware)) {
if ($this->isCallable($middleware)) {
return new CallableMiddlewareWrapper($middleware, $responsePrototype);
}

Expand Down
24 changes: 24 additions & 0 deletions test/ApplicationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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));
}
}

0 comments on commit 63fe808

Please sign in to comment.