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

Clarify that HEAD and OPTIONS should ALWAYS be allowed #27

Merged
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
49 changes: 38 additions & 11 deletions src/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ class Route
* @var bool If HEAD was not provided to the Route instance, indicate
* support for the method is implicit.
*/
private $implicitHead = true;
private $implicitHead = false;

/**
* @var bool If OPTIONS was not provided to the Route instance, indicate
* support for the method is implicit.
*/
private $implicitOptions = true;
private $implicitOptions = false;

/**
* @var int|string[] HTTP methods allowed with this route.
Expand Down Expand Up @@ -99,14 +99,9 @@ public function __construct($path, $middleware, $methods = self::HTTP_METHOD_ANY
if (empty($name)) {
$name = ($this->methods === self::HTTP_METHOD_ANY)
? $path
: $path . '^' . implode(self::HTTP_METHOD_SEPARATOR, $this->methods);
: $path . '^' . implode(self::HTTP_METHOD_SEPARATOR, $this->retrieveExplicitMethods($this->methods));
}
$this->name = $name;

$this->implicitHead = is_array($this->methods)
&& ! in_array(RequestMethod::METHOD_HEAD, $this->methods, true);
$this->implicitOptions = is_array($this->methods)
&& ! in_array(RequestMethod::METHOD_OPTIONS, $this->methods, true);
}

/**
Expand Down Expand Up @@ -160,9 +155,7 @@ public function getAllowedMethods()
public function allowsMethod($method)
{
$method = strtoupper($method);
if (RequestMethod::METHOD_HEAD === $method
|| RequestMethod::METHOD_OPTIONS === $method
|| $this->methods === self::HTTP_METHOD_ANY
if ($this->methods === self::HTTP_METHOD_ANY
|| in_array($method, $this->methods, true)
) {
return true;
Expand Down Expand Up @@ -236,6 +229,40 @@ private function validateHttpMethods(array $methods)
throw new Exception\InvalidArgumentException('One or more HTTP methods were invalid');
}

if (! in_array(RequestMethod::METHOD_HEAD, $methods, true)) {
$this->implicitHead = true;
$methods[] = RequestMethod::METHOD_HEAD;
}

if (! in_array(RequestMethod::METHOD_OPTIONS, $methods, true)) {
$this->implicitOptions = true;
$methods[] = RequestMethod::METHOD_OPTIONS;
}

return array_map('strtoupper', $methods);
}

/**
* Return a list of methods explicitly set when creating the route.
*
* Filters out HEAD and/or OPTIONS if they were set implicitly.
*/
private function retrieveExplicitMethods(array $methods)
{
return array_filter($methods, function ($method) {
if (! in_array($method, [RequestMethod::METHOD_HEAD, RequestMethod::METHOD_OPTIONS], true)) {
return true;
}

if ($method === RequestMethod::METHOD_HEAD && ! $this->implicitHead) {
return true;
}

if ($method === RequestMethod::METHOD_OPTIONS && ! $this->implicitOptions) {
return true;
}

return false;
});
}
}
8 changes: 7 additions & 1 deletion test/RouteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ public function testRouteAllowsSpecifyingHttpMethods()
{
$methods = [RequestMethod::METHOD_GET, RequestMethod::METHOD_POST];
$route = new Route('/foo', $this->noopMiddleware, $methods);
$this->assertSame($methods, $route->getAllowedMethods($methods));
foreach ($methods as $method) {
$this->assertContains($method, $route->getAllowedMethods());
}
}

public function testRouteCanMatchMethod()
Expand Down Expand Up @@ -181,6 +183,8 @@ public function testProvidingArrayOfMethodsWithoutHeadOrOptionsImpliesBoth()
$route = new Route('/test', $this->noopMiddleware, [RequestMethod::METHOD_GET, RequestMethod::METHOD_POST]);
$this->assertTrue($route->implicitHead());
$this->assertTrue($route->implicitOptions());
$this->assertContains(RequestMethod::METHOD_HEAD, $route->getAllowedMethods());
$this->assertContains(RequestMethod::METHOD_OPTIONS, $route->getAllowedMethods());
}

public function headAndOptions()
Expand All @@ -198,6 +202,8 @@ public function testPassingHeadOrOptionsInMethodArrayDoesNotMarkAsImplicit($http
{
$route = new Route('/test', $this->noopMiddleware, [$httpMethod]);
$this->assertFalse($route->{$implicitMethod}());
$this->assertContains(RequestMethod::METHOD_HEAD, $route->getAllowedMethods());
$this->assertContains(RequestMethod::METHOD_OPTIONS, $route->getAllowedMethods());
}

public function testPassingWildcardMethodDoesNotMarkAsImplicit()
Expand Down