Commit af80cb49 authored by alexpott's avatar alexpott

Issue #2659070 by Wim Leers, dawehner, tic2000, Crell: REST requests without...

Issue #2659070 by Wim Leers, dawehner, tic2000, Crell: REST requests without Content-Type header: unhelpful response significantly hinders DX, should receive a 415 response
parent 9b07bc54
......@@ -918,6 +918,10 @@ services:
class: Drupal\Core\Routing\RequestFormatRouteFilter
tags:
- { name: route_filter }
method_filter:
class: Drupal\Core\Routing\MethodFilter
tags:
- { name: route_filter, priority: 1 }
content_type_header_matcher:
class: Drupal\Core\Routing\ContentTypeHeaderMatcher
tags:
......
......@@ -82,4 +82,15 @@ public function on406(GetResponseForExceptionEvent $event) {
$event->setResponse($response);
}
/**
* Handles a 415 error for JSON.
*
* @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
* The event to process.
*/
public function on415(GetResponseForExceptionEvent $event) {
$response = new JsonResponse(['message' => $event->getException()->getMessage()], Response::HTTP_UNSUPPORTED_MEDIA_TYPE);
$event->setResponse($response);
}
}
<?php
namespace Drupal\Core\Routing;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
/**
* Filters routes based on the HTTP method.
*/
class MethodFilter implements RouteFilterInterface {
/**
* {@inheritdoc}
*/
public function filter(RouteCollection $collection, Request $request) {
$method = $request->getMethod();
$all_supported_methods = [];
foreach ($collection->all() as $name => $route) {
$supported_methods = $route->getMethods();
// A route not restricted to specific methods allows any method. If this
// is the case, we'll also have at least one route left in the collection,
// hence we don't need to calculate the set of all supported methods.
if (empty($supported_methods)) {
continue;
}
// If the GET method is allowed we also need to allow the HEAD method
// since HEAD is a GET method that doesn't return the body.
if (in_array('GET', $supported_methods, TRUE)) {
$supported_methods[] = 'HEAD';
}
if (!in_array($method, $supported_methods, TRUE)) {
$all_supported_methods = array_merge($supported_methods, $all_supported_methods);
$collection->remove($name);
}
}
if (count($collection)) {
return $collection;
}
throw new MethodNotAllowedException(array_unique($all_supported_methods));
}
/**
* {@inheritdoc}
*/
public function applies(Route $route) {
return !empty($route->getMethods());
}
}
......@@ -176,6 +176,10 @@ protected function httpRequest($url, $method, $body = NULL, $mime_type = NULL) {
break;
}
if ($mime_type === 'none') {
unset($curl_options[CURLOPT_HTTPHEADER]['Content-Type']);
}
$this->responseBody = $this->curlExec($curl_options);
// Ensure that any changes to variables in the other thread are picked up.
......
......@@ -8,6 +8,7 @@
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\entity_test\Entity\EntityTest;
use Symfony\Component\HttpFoundation\Response;
/**
* Tests the update of resources.
......@@ -67,6 +68,12 @@ public function testPatchUpdate() {
$patch_entity->set('uuid', NULL);
$serialized = $serializer->serialize($patch_entity, $this->defaultFormat, $context);
// Update the entity over the REST API but forget to specify a Content-Type
// header, this should throw the proper exception.
$this->httpRequest($entity->toUrl(), 'PATCH', $serialized, 'none');
$this->assertResponse(Response::HTTP_UNSUPPORTED_MEDIA_TYPE);
$this->assertRaw('No route found that matches &quot;Content-Type: none&quot;');
// Update the entity over the REST API.
$response = $this->httpRequest($entity->urlInfo(), 'PATCH', $serialized, $this->defaultMimeType);
$this->assertResponse(200);
......
......@@ -28,6 +28,19 @@ protected function setUp() {
$this->installEntitySchema('date_format');
}
/**
* Tests on a route with a non-supported HTTP method.
*/
public function test405() {
$request = Request::create('/router_test/test15', 'PATCH');
/** @var \Symfony\Component\HttpKernel\HttpKernelInterface $kernel */
$kernel = \Drupal::getContainer()->get('http_kernel');
$response = $kernel->handle($request);
$this->assertEqual(Response::HTTP_METHOD_NOT_ALLOWED, $response->getStatusCode());
}
/**
* Tests the exception handling for json and 403 status code.
*/
......
<?php
namespace Drupal\Tests\Core\Routing;
use Drupal\Core\Routing\MethodFilter;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;
/**
* @coversDefaultClass \Drupal\Core\Routing\MethodFilter
* @group Routing
*/
class MethodFilterTest extends \PHPUnit_Framework_TestCase {
/**
* @covers ::applies
* @dataProvider providerApplies
*/
public function testApplies(array $route_methods, $expected_applies) {
$route = new Route('/test', [], [], [], '', [], $route_methods);
$method_filter = new MethodFilter();
$this->assertSame($expected_applies, $method_filter->applies($route));
}
/**
* Data provider for testApplies().
*
* @return array
*/
public function providerApplies() {
return [
'only GET' => [['GET'], TRUE],
'only PATCH' => [['PATCH'], TRUE],
'only POST' => [['POST'], TRUE],
'only DELETE' => [['DELETE'], TRUE],
'only HEAD' => [['HEAD'], TRUE],
'all' => [['GET', 'PATCH', 'POST', 'DELETE', 'HEAD'], TRUE],
'none' => [[], FALSE],
];
}
/**
* @covers ::filter
*/
public function testWithAllowedMethod() {
$request = Request::create('/test', 'GET');
$collection = new RouteCollection();
$collection->add('test_route.get', new Route('/test', [], [], [], '', [], ['GET']));
$collection_before = clone $collection;
$method_filter = new MethodFilter();
$result_collection = $method_filter->filter($collection, $request);
$this->assertEquals($collection_before, $result_collection);
}
/**
* @covers ::filter
*/
public function testWithAllowedMethodAndMultipleMatchingRoutes() {
$request = Request::create('/test', 'GET');
$collection = new RouteCollection();
$collection->add('test_route.get', new Route('/test', [], [], [], '', [], ['GET']));
$collection->add('test_route2.get', new Route('/test', [], [], [], '', [], ['GET']));
$collection->add('test_route3.get', new Route('/test', [], [], [], '', [], ['GET']));
$collection_before = clone $collection;
$method_filter = new MethodFilter();
$result_collection = $method_filter->filter($collection, $request);
$this->assertEquals($collection_before, $result_collection);
}
/**
* @covers ::filter
*/
public function testMethodNotAllowedException() {
$request = Request::create('/test', 'PATCH');
$collection = new RouteCollection();
$collection->add('test_route.get', new Route('/test', [], [], [], '', [], ['GET']));
$this->setExpectedException(MethodNotAllowedException::class);
$method_filter = new MethodFilter();
$method_filter->filter($collection, $request);
}
/**
* @covers ::filter
*/
public function testMethodNotAllowedExceptionWithMultipleRoutes() {
$request = Request::create('/test', 'PATCH');
$collection = new RouteCollection();
$collection->add('test_route.get', new Route('/test', [], [], [], '', [], ['GET']));
$collection->add('test_route2.get', new Route('/test', [], [], [], '', [], ['GET']));
$collection->add('test_route3.get', new Route('/test', [], [], [], '', [], ['GET']));
$this->setExpectedException(MethodNotAllowedException::class);
$method_filter = new MethodFilter();
$method_filter->filter($collection, $request);
}
/**
* @covers ::filter
*/
public function testFilteredMethods() {
$request = Request::create('/test', 'PATCH');
$collection = new RouteCollection();
$collection->add('test_route.get', new Route('/test', [], [], [], '', [], ['GET']));
$collection->add('test_route2.get', new Route('/test', [], [], [], '', [], ['PATCH']));
$collection->add('test_route3.get', new Route('/test', [], [], [], '', [], ['POST']));
$expected_collection = new RouteCollection();
$expected_collection->add('test_route2.get', new Route('/test', [], [], [], '', [], ['PATCH']));
$method_filter = new MethodFilter();
$result_collection = $method_filter->filter($collection, $request);
$this->assertEquals($expected_collection, $result_collection);
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment