From a8fec3492e495452b5b044d923f39caa13205a57 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Tue, 4 Apr 2017 17:50:53 +0100 Subject: [PATCH] Issue #2826407 by Wim Leers, dawehner, Jo Fitzgerald: PATCH + POST allowed format validation happens in RequestHandler::handle(), rather than in the routing system --- core/modules/rest/src/Plugin/ResourceBase.php | 10 ---- core/modules/rest/src/RequestHandler.php | 59 ++++++++----------- .../rest/src/Routing/ResourceRoutes.php | 11 +++- .../EntityResource/EntityResourceTestBase.php | 10 +--- 4 files changed, 35 insertions(+), 55 deletions(-) diff --git a/core/modules/rest/src/Plugin/ResourceBase.php b/core/modules/rest/src/Plugin/ResourceBase.php index 3062aa29ed8b..50d8d758ced5 100644 --- a/core/modules/rest/src/Plugin/ResourceBase.php +++ b/core/modules/rest/src/Plugin/ResourceBase.php @@ -111,16 +111,6 @@ abstract class ResourceBase extends PluginBase implements ContainerFactoryPlugin switch ($method) { case 'POST': $route->setPath($create_path); - // Restrict the incoming HTTP Content-type header to the known - // serialization formats. - $route->addRequirements(['_content_type_format' => implode('|', $this->serializerFormats)]); - $collection->add("$route_name.$method", $route); - break; - - case 'PATCH': - // Restrict the incoming HTTP Content-type header to the known - // serialization formats. - $route->addRequirements(['_content_type_format' => implode('|', $this->serializerFormats)]); $collection->add("$route_name.$method", $route); break; diff --git a/core/modules/rest/src/RequestHandler.php b/core/modules/rest/src/RequestHandler.php index 9f5137905b27..e5437ccb6802 100644 --- a/core/modules/rest/src/RequestHandler.php +++ b/core/modules/rest/src/RequestHandler.php @@ -12,7 +12,6 @@ use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException; -use Symfony\Component\HttpKernel\Exception\UnsupportedMediaTypeHttpException; use Symfony\Component\Serializer\Exception\UnexpectedValueException; use Symfony\Component\Serializer\Exception\InvalidArgumentException; @@ -89,42 +88,32 @@ class RequestHandler implements ContainerAwareInterface, ContainerInjectionInter if (!empty($received)) { $format = $request->getContentType(); - // Only allow serialization formats that are explicitly configured. If no - // formats are configured allow all and hope that the serializer knows the - // format. If the serializer cannot handle it an exception will be thrown - // that bubbles up to the client. - $request_method = $request->getMethod(); - if (in_array($format, $resource_config->getFormats($request_method))) { - $definition = $resource->getPluginDefinition(); + $definition = $resource->getPluginDefinition(); - // First decode the request data. We can then determine if the - // serialized data was malformed. - try { - $unserialized = $serializer->decode($received, $format, ['request_method' => $method]); - } - catch (UnexpectedValueException $e) { - // If an exception was thrown at this stage, there was a problem - // decoding the data. Throw a 400 http exception. - throw new BadRequestHttpException($e->getMessage()); - } - - // Then attempt to denormalize if there is a serialization class. - if (!empty($definition['serialization_class'])) { - try { - $unserialized = $serializer->denormalize($unserialized, $definition['serialization_class'], $format, ['request_method' => $method]); - } - // These two serialization exception types mean there was a problem - // with the structure of the decoded data and it's not valid. - catch (UnexpectedValueException $e) { - throw new UnprocessableEntityHttpException($e->getMessage()); - } - catch (InvalidArgumentException $e) { - throw new UnprocessableEntityHttpException($e->getMessage()); - } - } + // First decode the request data. We can then determine if the + // serialized data was malformed. + try { + $unserialized = $serializer->decode($received, $format, ['request_method' => $method]); } - else { - throw new UnsupportedMediaTypeHttpException(); + catch (UnexpectedValueException $e) { + // If an exception was thrown at this stage, there was a problem + // decoding the data. Throw a 400 http exception. + throw new BadRequestHttpException($e->getMessage()); + } + + // Then attempt to denormalize if there is a serialization class. + if (!empty($definition['serialization_class'])) { + try { + $unserialized = $serializer->denormalize($unserialized, $definition['serialization_class'], $format, ['request_method' => $method]); + } + // These two serialization exception types mean there was a problem + // with the structure of the decoded data and it's not valid. + catch (UnexpectedValueException $e) { + throw new UnprocessableEntityHttpException($e->getMessage()); + } + catch (InvalidArgumentException $e) { + throw new UnprocessableEntityHttpException($e->getMessage()); + } } } diff --git a/core/modules/rest/src/Routing/ResourceRoutes.php b/core/modules/rest/src/Routing/ResourceRoutes.php index 6aec267db2ce..c21f13cd961c 100644 --- a/core/modules/rest/src/Routing/ResourceRoutes.php +++ b/core/modules/rest/src/Routing/ResourceRoutes.php @@ -113,8 +113,15 @@ class ResourceRoutes extends RouteSubscriberBase { continue; } - // The configuration seems legit at this point, so we set the - // authentication provider and add the route. + // The configuration has been validated, so we update the route to: + // - set the allowed request body content types/formats for methods that + // allow request bodies to be sent + // - set the allowed authentication providers + if (in_array($method, ['POST', 'PATCH', 'PUT'], TRUE)) { + // Restrict the incoming HTTP Content-type header to the allowed + // formats. + $route->addRequirements(['_content_type_format' => implode('|', $rest_resource_config->getFormats($method))]); + } $route->setOption('_auth', $rest_resource_config->getAuthenticationProviders($method)); $route->setDefault('_rest_resource_config', $rest_resource_config->id()); $collection->add("rest.$name", $route); diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 53796c8aeaf4..81baf9ee179d 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -724,10 +724,7 @@ abstract class EntityResourceTestBase extends ResourceTestBase { // DX: 415 when request body in existing but not allowed format. $response = $this->request('POST', $url, $request_options); - // @todo Update this in https://www.drupal.org/node/2826407. Also move it - // higher, before the "no request body" test. That's impossible right now, - // because the format validation happens too late. - $this->assertResourceErrorResponse(415, '', $response); + $this->assertResourceErrorResponse(415, 'No route found that matches "Content-Type: text/xml"', $response); $request_options[RequestOptions::HEADERS]['Content-Type'] = static::$mimeType; @@ -936,10 +933,7 @@ abstract class EntityResourceTestBase extends ResourceTestBase { // DX: 415 when request body in existing but not allowed format. $response = $this->request('PATCH', $url, $request_options); - // @todo Update this in https://www.drupal.org/node/2826407. Also move it - // higher, before the "no request body" test. That's impossible right now, - // because the format validation happens too late. - $this->assertResourceErrorResponse(415, '', $response); + $this->assertResourceErrorResponse(415, 'No route found that matches "Content-Type: text/xml"', $response); $request_options[RequestOptions::HEADERS]['Content-Type'] = static::$mimeType;