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

Fix OPTIONS request for not found routes #73

Merged
merged 1 commit into from
Mar 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/Middleware/ImplicitOptionsMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
return $handler->handle($request);
}

if ($result->isFailure() && ! $result->isMethodFailure()) {
return $handler->handle($request);
}

if ($result->getMatchedRoute()) {
return $handler->handle($request);
}
Expand Down
47 changes: 47 additions & 0 deletions src/Test/ImplicitMethodsIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -363,4 +363,51 @@ public function testImplicitOptionsRequest(

$this->assertSame($finalResponse->reveal(), $response);
}

public function testImplicitOptionsRequestRouteNotFound()
{
$router = $this->getRouter();

$pipeline = new MiddlewarePipe();
$pipeline->pipe(new RouteMiddleware($router));
$pipeline->pipe($this->getImplicitOptionsMiddleware());
$pipeline->pipe(new MethodNotAllowedMiddleware($this->createInvalidResponseFactory()));
$pipeline->pipe(new DispatchMiddleware());

$finalResponse = (new Response())
->withStatus(StatusCode::STATUS_IM_A_TEAPOT)
->withHeader('foo-bar', 'baz');
$finalResponse->getBody()->write('FOO BAR BODY');

$request = new ServerRequest(
['REQUEST_METHOD' => RequestMethod::METHOD_OPTIONS],
[],
'/not-found',
RequestMethod::METHOD_OPTIONS
);

$finalHandler = $this->prophesize(RequestHandlerInterface::class);
$finalHandler
->handle(Argument::that(function (ServerRequestInterface $request) {
Assert::assertSame(RequestMethod::METHOD_OPTIONS, $request->getMethod());

$routeResult = $request->getAttribute(RouteResult::class);
Assert::assertInstanceOf(RouteResult::class, $routeResult);
Assert::assertTrue($routeResult->isFailure());
Assert::assertFalse($routeResult->isSuccess());
Assert::assertFalse($routeResult->isMethodFailure());
Assert::assertFalse($routeResult->getMatchedRoute());

return true;
}))
->willReturn($finalResponse)
->shouldBeCalledTimes(1);

$response = $pipeline->process($request, $finalHandler->reveal());

$this->assertEquals(StatusCode::STATUS_IM_A_TEAPOT, $response->getStatusCode());
$this->assertSame('FOO BAR BODY', (string) $response->getBody());
$this->assertTrue($response->hasHeader('foo-bar'));
$this->assertSame('baz', $response->getHeaderLine('foo-bar'));
}
}
28 changes: 21 additions & 7 deletions test/Middleware/ImplicitOptionsMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,11 @@ public function testReturnsResultOfHandlerWhenRouteSupportsOptionsExplicitly()
{
$route = $this->prophesize(Route::class);

$result = $this->prophesize(RouteResult::class);
$result->getMatchedRoute()->will([$route, 'reveal']);
$result = RouteResult::fromRoute($route->reveal());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed here to not mocking the RouteResult because if we use more methods we need always extend our mock.
And what's more we can easily mock RouteResult wrong way. We can create new instance only by using fromRoute or fromRouteFailure but we can create invalid mock as we have to mock 4-5 public methods...


$request = $this->prophesize(ServerRequestInterface::class);
$request->getMethod()->willReturn(RequestMethod::METHOD_OPTIONS);
$request->getAttribute(RouteResult::class)->will([$result, 'reveal']);
$request->getAttribute(RouteResult::class)->willReturn($result);

$response = $this->prophesize(ResponseInterface::class)->reveal();

Expand All @@ -93,13 +92,11 @@ public function testInjectsAllowHeaderInResponseProvidedToConstructorDuringOptio
{
$allowedMethods = [RequestMethod::METHOD_GET, RequestMethod::METHOD_POST];

$result = $this->prophesize(RouteResult::class);
$result->getAllowedMethods()->willReturn($allowedMethods);
$result->getMatchedRoute()->willReturn(false);
$result = RouteResult::fromRouteFailure($allowedMethods);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same comment here as above about mocking RouteResult.


$request = $this->prophesize(ServerRequestInterface::class);
$request->getMethod()->willReturn(RequestMethod::METHOD_OPTIONS);
$request->getAttribute(RouteResult::class)->will([$result, 'reveal']);
$request->getAttribute(RouteResult::class)->willReturn($result);

$handler = $this->prophesize(RequestHandlerInterface::class);
$handler->handle($request->reveal())->shouldNotBeCalled();
Expand All @@ -111,4 +108,21 @@ public function testInjectsAllowHeaderInResponseProvidedToConstructorDuringOptio
$result = $this->middleware->process($request->reveal(), $handler->reveal());
$this->assertSame($this->response->reveal(), $result);
}

public function testReturnsResultOfHandlerWhenRouteNotFound()
{
$result = RouteResult::fromRouteFailure(Route::HTTP_METHOD_ANY);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main test, so in case route NOT FOUND we have allowed any methods, but the result is failure.
So isFailure === true and isMethodFailure === false. Then -> 404.


$request = $this->prophesize(ServerRequestInterface::class);
$request->getMethod()->willReturn(RequestMethod::METHOD_OPTIONS);
$request->getAttribute(RouteResult::class)->willReturn($result);

$response = $this->prophesize(ResponseInterface::class)->reveal();

$handler = $this->prophesize(RequestHandlerInterface::class);
$handler->handle($request->reveal())->willReturn($response);

$result = $this->middleware->process($request->reveal(), $handler->reveal());
$this->assertSame($response, $result);
}
}