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

Commit

Permalink
Ensures that path segregated middleware may provide an altered request
Browse files Browse the repository at this point in the history
Discovered while backporting to the 2.X series. Essentially, a path
segregated middleware should still be able to pass a new request on to
the next layer. However, in doing so, the _path_ should be reset.

This patch updates the `PathMiddlewareDecoratorIntegrationTest` to
reflect this behavior, and updates the anonymous request handler created
by the `PathMiddlewareDecorator` to invoke the composed handler with the
runtime request after resetting its URI instance with the original path.
  • Loading branch information
weierophinney committed Jan 11, 2018
1 parent aa0e8d5 commit 127502b
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 29 deletions.
13 changes: 11 additions & 2 deletions src/Middleware/PathMiddlewareDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,20 @@ public function __construct(RequestHandlerInterface $handler, ServerRequestInter
}

/**
* Invokes the composed handler with the original server request.
* Invokes the composed handler with a request using the original URI.
*
* The decorated middleware may provide an altered response. However,
* we want to reset the path to the original path on invocation, as
* that is the part we originally modified, and is a part the decorated
* middleware should not modify.
*
* {@inheritDoc}
*/
public function handle(ServerRequestInterface $request) : ResponseInterface
{
return $this->handler->handle($this->originalRequest);
$uri = $request->getUri()
->withPath($this->originalRequest->getUri()->getPath());
return $this->handler->handle($request->withUri($uri));
}
};
}
Expand Down
73 changes: 48 additions & 25 deletions test/Middleware/PathMiddlewareDecoratorIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,39 @@ public function testPipelineComposingPathDecoratedMiddlewareExecutesAsExpected()

$pipeline = new MiddlewarePipe();

$first = $this->createPassThroughMiddleware($request);
$first = $this->createPassThroughMiddleware(function ($received) use ($request) {
Assert::assertSame(
$request,
$received,
'First middleware did not receive original request, but should have'
);
return $request;
});
$second = new PathMiddlewareDecorator('/foo', $this->createNestedPipeline($request));
$last = $this->createPassThroughMiddleware($request);
$last = $this->createPassThroughMiddleware(function ($received) use ($request) {
Assert::assertNotSame(
$request,
$received,
'Last middleware received original request, but should not have'
);

$originalUri = $request->getUri();
$receivedUri = $received->getUri();

Assert::assertNotSame(
$originalUri,
$receivedUri,
'Last middleware received original URI instance, but should not have'
);

Assert::assertSame(
$originalUri->getPath(),
$receivedUri->getPath(),
'Last middleware did not receive original URI path, but should have'
);

return $request;
});

$pipeline->pipe($first);
$pipeline->pipe($second);
Expand All @@ -51,6 +81,22 @@ public function testPipelineComposingPathDecoratedMiddlewareExecutesAsExpected()
);
}

public function createPassThroughMiddleware(callable $requestAssertion) : MiddlewareInterface
{
$middleware = $this->prophesize(MiddlewareInterface::class);
$middleware
->process(
Argument::that($requestAssertion),
Argument::type(RequestHandlerInterface::class)
)
->will(function ($args) {
$request = $args[0];
$next = $args[1];
return $next->handle($request);
});
return $middleware->reveal();
}

public function createNestedPipeline(ServerRequestInterface $originalRequest) : MiddlewareInterface
{
$pipeline = new MiddlewarePipe();
Expand Down Expand Up @@ -111,27 +157,4 @@ public function createNestedPipeline(ServerRequestInterface $originalRequest) :

return $pipeline;
}

public function createPassThroughMiddleware(ServerRequestInterface $originalRequest) : MiddlewareInterface
{
$middleware = $this->prophesize(MiddlewareInterface::class);
$middleware
->process(
Argument::that(function ($request) use ($originalRequest) {
Assert::assertSame(
$originalRequest,
$request,
'Non-segregated middleware did not receive original request, but should have'
);
return $request;
}),
Argument::type(RequestHandlerInterface::class)
)
->will(function ($args) {
$request = $args[0];
$next = $args[1];
return $next->handle($request);
});
return $middleware->reveal();
}
}
18 changes: 16 additions & 2 deletions test/Middleware/PathMiddlewareDecoratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -348,14 +348,28 @@ public function testRequestPathPassedToDecoratedMiddlewareTrimsPathPrefix()
$decorator->process($request, $finalHandler->reveal());
}

public function testInvocationOfHandlerByDecoratedMiddlewareWillInvokeWithOriginalRequest()
public function testInvocationOfHandlerByDecoratedMiddlewareWillInvokeWithOriginalRequestPath()
{
$request = new ServerRequest([], [], 'http://local.example.com/test', 'GET', 'php://memory');
$expectedResponse = new Response();

$finalHandler = $this->prophesize(RequestHandlerInterface::class);
$finalHandler
->handle($request)
->handle(Argument::that(function ($received) use ($request) {
Assert::assertNotSame(
$request,
$received,
'Final handler received same request, and should not have'
);

Assert::assertSame(
$request->getUri()->getPath(),
$received->getUri()->getPath(),
'Final handler received request with different path'
);

return $received;
}))
->willReturn($expectedResponse);

$segregatedMiddleware = $this->prophesize(MiddlewareInterface::class);
Expand Down

0 comments on commit 127502b

Please sign in to comment.