Issue #2826407 by Wim Leers, dawehner, Jo Fitzgerald: PATCH + POST allowed...

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

@@ -111,16 +111,6 @@ public function routes() {
       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;
 

core/modules/rest/src/RequestHandler.php

@@ -12,7 +12,6 @@
 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,43 +88,33 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
     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.
+      // 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->decode($received, $format, ['request_method' => $method]);
+          $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) {
-          // 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());
+          throw new UnprocessableEntityHttpException($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());
-          }
+        catch (InvalidArgumentException $e) {
+          throw new UnprocessableEntityHttpException($e->getMessage());
         }
       }
-      else {
-        throw new UnsupportedMediaTypeHttpException();
-      }
     }
 
     // Determine the request parameters that should be passed to the resource

core/modules/rest/src/Routing/ResourceRoutes.php

@@ -113,8 +113,15 @@ protected function getRoutesForResourceConfig(RestResourceConfigInterface $rest_
           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);

core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php

@@ -724,10 +724,7 @@ public function testPost() {
 
     // 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 @@ public function testPatch() {
 
     // 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;