From d2fdd97d11dbeb7878afcf98be878f6195bc683a Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Tue, 14 Feb 2017 14:09:58 +0000
Subject: [PATCH] Issue #2827084 by damiankloip, Wim Leers, dawehner, prics,
 larowlan, catch, darrenwh: EntityNormalizer::denormalize should not throw
 UnexpectedValueException, but
 \Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException, so
 we get a 422 response instead of 400

---
 .../HalEntityNormalizationTrait.php           |  8 ++---
 core/modules/rest/src/RequestHandler.php      | 31 ++++++++++++++-----
 .../EntityResource/EntityResourceTestBase.php | 10 +++---
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/core/modules/hal/tests/src/Functional/EntityResource/HalEntityNormalizationTrait.php b/core/modules/hal/tests/src/Functional/EntityResource/HalEntityNormalizationTrait.php
index cefe7438201b..18655727ba62 100644
--- a/core/modules/hal/tests/src/Functional/EntityResource/HalEntityNormalizationTrait.php
+++ b/core/modules/hal/tests/src/Functional/EntityResource/HalEntityNormalizationTrait.php
@@ -101,18 +101,18 @@ protected function assertNormalizationEdgeCases($method, Url $url, array $reques
       $normalization['_links']['type'] = Url::fromUri('base:rest/type/' . static::$entityTypeId . '/bad_bundle_name');
       $request_options[RequestOptions::BODY] = $this->serializer->encode($normalization, static::$format);
 
-      // DX: 400 when incorrect entity type bundle is specified.
+      // DX: 422 when incorrect entity type bundle is specified.
       $response = $this->request($method, $url, $request_options);
-      $this->assertResourceErrorResponse(400, 'No entity type(s) specified', $response);
+      $this->assertResourceErrorResponse(422, 'No entity type(s) specified', $response);
 
 
       unset($normalization['_links']['type']);
       $request_options[RequestOptions::BODY] = $this->serializer->encode($normalization, static::$format);
 
 
-      // DX: 400 when no entity type bundle is specified.
+      // DX: 422 when no entity type bundle is specified.
       $response = $this->request($method, $url, $request_options);
-      $this->assertResourceErrorResponse(400, 'The type link relation must be specified.', $response);
+      $this->assertResourceErrorResponse(422, 'The type link relation must be specified.', $response);
     }
   }
 
diff --git a/core/modules/rest/src/RequestHandler.php b/core/modules/rest/src/RequestHandler.php
index 0e9f41bbab48..d7a6be71afb6 100644
--- a/core/modules/rest/src/RequestHandler.php
+++ b/core/modules/rest/src/RequestHandler.php
@@ -11,8 +11,10 @@
 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;
 
 /**
  * Acts as intermediate request forwarder for resource plugins.
@@ -96,19 +98,32 @@ public function handle(RouteMatchInterface $route_match, Request $request) {
       $request_method = $request->getMethod();
       if (in_array($format, $resource_config->getFormats($request_method))) {
         $definition = $resource->getPluginDefinition();
+
+        // First decode the request data. We can then determine if the
+        // serialized data was malformed.
         try {
-          if (!empty($definition['serialization_class'])) {
-            $unserialized = $serializer->deserialize($received, $definition['serialization_class'], $format, array('request_method' => $method));
-          }
-          // If the plugin does not specify a serialization class just decode
-          // the received data.
-          else {
-            $unserialized = $serializer->decode($received, $format, array('request_method' => $method));
-          }
+          $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());
+          }
+        }
       }
       else {
         throw new UnsupportedMediaTypeHttpException();
diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
index 73dfc628daae..2759bc66fa22 100644
--- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
@@ -1035,10 +1035,9 @@ protected function assertNormalizationEdgeCases($method, Url $url, array $reques
         $request_options[RequestOptions::BODY] = $this->serializer->encode($normalization, static::$format);
 
 
-        // DX: 400 when incorrect entity type bundle is specified.
-        // @todo Change to 422 in https://www.drupal.org/node/2827084.
+        // DX: 422 when incorrect entity type bundle is specified.
         $response = $this->request($method, $url, $request_options);
-        $this->assertResourceErrorResponse(400, '"bad_bundle_name" is not a valid bundle type for denormalization.', $response);
+        $this->assertResourceErrorResponse(422, '"bad_bundle_name" is not a valid bundle type for denormalization.', $response);
       }
 
 
@@ -1046,10 +1045,9 @@ protected function assertNormalizationEdgeCases($method, Url $url, array $reques
       $request_options[RequestOptions::BODY] = $this->serializer->encode($normalization, static::$format);
 
 
-      // DX: 400 when no entity type bundle is specified.
-      // @todo Change to 422 in https://www.drupal.org/node/2827084.
+      // DX: 422 when no entity type bundle is specified.
       $response = $this->request($method, $url, $request_options);
-      $this->assertResourceErrorResponse(400, sprintf('Could not determine entity type bundle: "%s" field is missing.', $bundle_field_name), $response);
+      $this->assertResourceErrorResponse(422, sprintf('Could not determine entity type bundle: "%s" field is missing.', $bundle_field_name), $response);
     }
   }
 
-- 
GitLab