Merge pull request #2757 from teohhanhui/fix/handling-empty-request-c…

…ontent Fix handling of empty request content by allowing to disable listeners per operation
api-platform · May 6, 2019 · d806c72 · d806c72
2 parents 09e8006 + b1590b8
commit d806c72
Show file tree

Hide file tree

Showing 16 changed files with 381 additions and 166 deletions.
diff --git a/features/main/crud.feature b/features/main/crud.feature
@@ -91,6 +91,12 @@ Feature: Create-Retrieve-Update-Delete
     }
     """
 
+  Scenario: Create a resource with empty body
+    When I add "Content-Type" header equal to "application/ld+json"
+    And I send a "POST" request to "/dummies"
+    Then the response status code should be 400
+    And the JSON node "hydra:description" should be equal to "Syntax error"
+
   Scenario: Get a not found exception
     When I send a "GET" request to "/dummies/42"
     Then the response status code should be 404
@@ -526,42 +532,8 @@ Feature: Create-Retrieve-Update-Delete
   Scenario: Update a resource with empty body
     When I add "Content-Type" header equal to "application/ld+json"
     And I send a "PUT" request to "/dummies/1"
-    Then the response status code should be 200
-    And the response should be in JSON
-    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
-    And the header "Content-Location" should be equal to "/dummies/1"
-    And the JSON should be equal to:
-    """
-    {
-      "@context": "/contexts/Dummy",
-      "@id": "/dummies/1",
-      "@type": "Dummy",
-      "description": null,
-      "dummy": null,
-      "dummyBoolean": null,
-      "dummyDate": "2018-12-01T13:12:00+00:00",
-      "dummyFloat": null,
-      "dummyPrice": null,
-      "relatedDummy": null,
-      "relatedDummies": [],
-      "jsonData": [
-        {
-          "key": "value1"
-        },
-        {
-          "key": "value2"
-        }
-      ],
-      "arrayData": [],
-      "name_converted": null,
-      "relatedOwnedDummy": null,
-      "relatedOwningDummy": null,
-      "id": 1,
-      "name": "A nice dummy",
-      "alias": null,
-      "foo": null
-    }
-    """
+    Then the response status code should be 400
+    And the JSON node "hydra:description" should be equal to "Syntax error"
 
   Scenario: Delete a resource
     When I send a "DELETE" request to "/dummies/1"

diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php
@@ -32,7 +32,7 @@
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Messenger\MessageBusInterface;
-use Symfony\Component\Serializer\Exception\ExceptionInterface;
+use Symfony\Component\Serializer\Exception\ExceptionInterface as SerializerExceptionInterface;
 
 /**
  * The configuration of the bundle.
@@ -369,7 +369,7 @@ private function addExceptionToStatusSection(ArrayNodeDefinition $rootNode): voi
             ->children()
                 ->arrayNode('exception_to_status')
                     ->defaultValue([
-                        ExceptionInterface::class => Response::HTTP_BAD_REQUEST,
+                        SerializerExceptionInterface::class => Response::HTTP_BAD_REQUEST,
                         InvalidArgumentException::class => Response::HTTP_BAD_REQUEST,
                         FilterValidationException::class => Response::HTTP_BAD_REQUEST,
                         OptimisticLockException::class => Response::HTTP_CONFLICT,

diff --git a/src/Bridge/Symfony/Bundle/Resources/config/api.xml b/src/Bridge/Symfony/Bundle/Resources/config/api.xml
@@ -154,14 +154,15 @@
             <argument type="service" id="api_platform.subresource_data_provider" />
             <argument type="service" id="api_platform.serializer.context_builder" />
             <argument type="service" id="api_platform.identifier.converter" />
+            <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
 
             <tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" priority="4" />
         </service>
 
         <service id="api_platform.listener.view.write" class="ApiPlatform\Core\EventListener\WriteListener">
             <argument type="service" id="api_platform.data_persister" />
             <argument type="service" id="api_platform.iri_converter" on-invalid="null" />
-            <argument type="service" id="api_platform.metadata.resource.metadata_factory" on-invalid="null" />
+            <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
 
             <tag name="kernel.event_listener" event="kernel.view" method="onKernelView" priority="32" />
         </service>
@@ -170,13 +171,15 @@
             <argument type="service" id="api_platform.serializer" />
             <argument type="service" id="api_platform.serializer.context_builder" />
             <argument type="service" id="api_platform.formats_provider" />
+            <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
 
             <tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" priority="2" />
         </service>
 
         <service id="api_platform.listener.view.serialize" class="ApiPlatform\Core\EventListener\SerializeListener">
             <argument type="service" id="api_platform.serializer" />
             <argument type="service" id="api_platform.serializer.context_builder" />
+            <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
 
             <tag name="kernel.event_listener" event="kernel.view" method="onKernelView" priority="16" />
         </service>

diff --git a/src/EventListener/DeserializeListener.php b/src/EventListener/DeserializeListener.php
@@ -16,6 +16,8 @@
 use ApiPlatform\Core\Api\FormatMatcher;
 use ApiPlatform\Core\Api\FormatsProviderInterface;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
+use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
+use ApiPlatform\Core\Metadata\Resource\ToggleableOperationAttributeTrait;
 use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface;
 use ApiPlatform\Core\Util\RequestAttributesExtractor;
 use Symfony\Component\HttpFoundation\Request;
@@ -31,6 +33,10 @@
  */
 final class DeserializeListener
 {
+    use ToggleableOperationAttributeTrait;
+
+    public const OPERATION_ATTRIBUTE_KEY = 'deserialize';
+
     private $serializer;
     private $serializerContextBuilder;
     private $formats = [];
@@ -40,7 +46,7 @@ final class DeserializeListener
     /**
      * @throws InvalidArgumentException
      */
-    public function __construct(SerializerInterface $serializer, SerializerContextBuilderInterface $serializerContextBuilder, /* FormatsProviderInterface */$formatsProvider)
+    public function __construct(SerializerInterface $serializer, SerializerContextBuilderInterface $serializerContextBuilder, /* FormatsProviderInterface */$formatsProvider, ResourceMetadataFactoryInterface $resourceMetadataFactory = null)
     {
         $this->serializer = $serializer;
         $this->serializerContextBuilder = $serializerContextBuilder;
@@ -54,6 +60,7 @@ public function __construct(SerializerInterface $serializer, SerializerContextBu
 
             $this->formatsProvider = $formatsProvider;
         }
+        $this->resourceMetadataFactory = $resourceMetadataFactory;
     }
 
     /**
@@ -69,18 +76,12 @@ public function onKernelRequest(GetResponseEvent $event): void
             || $request->isMethodSafe(false)
             || !($attributes = RequestAttributesExtractor::extractAttributes($request))
             || !$attributes['receive']
-            || (
-                    '' === ($requestContent = $request->getContent())
-                    && ('POST' === $method || 'PUT' === $method)
-               )
+            || $this->isOperationAttributeDisabled($attributes, self::OPERATION_ATTRIBUTE_KEY)
         ) {
             return;
         }
 
         $context = $this->serializerContextBuilder->createFromRequest($request, false, $attributes);
-        if (isset($context['input']) && \array_key_exists('class', $context['input']) && null === $context['input']['class']) {
-            return;
-        }
 
         // BC check to be removed in 3.0
         if (null !== $this->formatsProvider) {
@@ -96,9 +97,7 @@ public function onKernelRequest(GetResponseEvent $event): void
 
         $request->attributes->set(
             'data',
-            $this->serializer->deserialize(
-                $requestContent, $context['resource_class'], $format, $context
-            )
+            $this->serializer->deserialize($request->getContent(), $context['resource_class'], $format, $context)
         );
     }
 

diff --git a/src/EventListener/ReadListener.php b/src/EventListener/ReadListener.php
@@ -20,6 +20,8 @@
 use ApiPlatform\Core\Exception\InvalidIdentifierException;
 use ApiPlatform\Core\Exception\RuntimeException;
 use ApiPlatform\Core\Identifier\IdentifierConverterInterface;
+use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
+use ApiPlatform\Core\Metadata\Resource\ToggleableOperationAttributeTrait;
 use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface;
 use ApiPlatform\Core\Util\RequestAttributesExtractor;
 use ApiPlatform\Core\Util\RequestParser;
@@ -34,16 +36,20 @@
 final class ReadListener
 {
     use OperationDataProviderTrait;
+    use ToggleableOperationAttributeTrait;
+
+    public const OPERATION_ATTRIBUTE_KEY = 'read';
 
     private $serializerContextBuilder;
 
-    public function __construct(CollectionDataProviderInterface $collectionDataProvider, ItemDataProviderInterface $itemDataProvider, SubresourceDataProviderInterface $subresourceDataProvider = null, SerializerContextBuilderInterface $serializerContextBuilder = null, IdentifierConverterInterface $identifierConverter = null)
+    public function __construct(CollectionDataProviderInterface $collectionDataProvider, ItemDataProviderInterface $itemDataProvider, SubresourceDataProviderInterface $subresourceDataProvider = null, SerializerContextBuilderInterface $serializerContextBuilder = null, IdentifierConverterInterface $identifierConverter = null, ResourceMetadataFactoryInterface $resourceMetadataFactory = null)
     {
         $this->collectionDataProvider = $collectionDataProvider;
         $this->itemDataProvider = $itemDataProvider;
         $this->subresourceDataProvider = $subresourceDataProvider;
         $this->serializerContextBuilder = $serializerContextBuilder;
         $this->identifierConverter = $identifierConverter;
+        $this->resourceMetadataFactory = $resourceMetadataFactory;
     }
 
     /**
@@ -57,6 +63,8 @@ public function onKernelRequest(GetResponseEvent $event): void
         if (
             !($attributes = RequestAttributesExtractor::extractAttributes($request))
             || !$attributes['receive']
+            || $request->isMethod('POST') && isset($attributes['collection_operation_name'])
+            || $this->isOperationAttributeDisabled($attributes, self::OPERATION_ATTRIBUTE_KEY)
         ) {
             return;
         }
@@ -74,7 +82,7 @@ public function onKernelRequest(GetResponseEvent $event): void
         }
 
         if (isset($attributes['collection_operation_name'])) {
-            $request->attributes->set('data', $request->isMethod('POST') ? null : $this->getCollectionData($attributes, $context));
+            $request->attributes->set('data', $this->getCollectionData($attributes, $context));
 
             return;
         }

diff --git a/src/EventListener/SerializeListener.php b/src/EventListener/SerializeListener.php
@@ -14,6 +14,8 @@
 namespace ApiPlatform\Core\EventListener;
 
 use ApiPlatform\Core\Exception\RuntimeException;
+use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
+use ApiPlatform\Core\Metadata\Resource\ToggleableOperationAttributeTrait;
 use ApiPlatform\Core\Serializer\ResourceList;
 use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface;
 use ApiPlatform\Core\Util\RequestAttributesExtractor;
@@ -32,13 +34,18 @@
  */
 final class SerializeListener
 {
+    use ToggleableOperationAttributeTrait;
+
+    public const OPERATION_ATTRIBUTE_KEY = 'serialize';
+
     private $serializer;
     private $serializerContextBuilder;
 
-    public function __construct(SerializerInterface $serializer, SerializerContextBuilderInterface $serializerContextBuilder)
+    public function __construct(SerializerInterface $serializer, SerializerContextBuilderInterface $serializerContextBuilder, ResourceMetadataFactoryInterface $resourceMetadataFactory = null)
     {
         $this->serializer = $serializer;
         $this->serializerContextBuilder = $serializerContextBuilder;
+        $this->resourceMetadataFactory = $resourceMetadataFactory;
     }
 
     /**
@@ -49,7 +56,11 @@ public function onKernelView(GetResponseForControllerResultEvent $event): void
         $controllerResult = $event->getControllerResult();
         $request = $event->getRequest();
 
-        if ($controllerResult instanceof Response || !(($attributes = RequestAttributesExtractor::extractAttributes($request))['respond'] ?? $request->attributes->getBoolean('_api_respond', false))) {
+        if (
+            $controllerResult instanceof Response
+            || !(($attributes = RequestAttributesExtractor::extractAttributes($request))['respond'] ?? $request->attributes->getBoolean('_api_respond', false))
+            || $attributes && $this->isOperationAttributeDisabled($attributes, self::OPERATION_ATTRIBUTE_KEY)
+        ) {
             return;
         }
 

diff --git a/src/EventListener/WriteListener.php b/src/EventListener/WriteListener.php
@@ -16,7 +16,9 @@
 use ApiPlatform\Core\Api\IriConverterInterface;
 use ApiPlatform\Core\DataPersister\DataPersisterInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
+use ApiPlatform\Core\Metadata\Resource\ToggleableOperationAttributeTrait;
 use ApiPlatform\Core\Util\RequestAttributesExtractor;
+use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent;
 
 /**
@@ -27,6 +29,10 @@
  */
 final class WriteListener
 {
+    use ToggleableOperationAttributeTrait;
+
+    public const OPERATION_ATTRIBUTE_KEY = 'write';
+
     private $dataPersister;
     private $iriConverter;
     private $resourceMetadataFactory;
@@ -43,12 +49,19 @@ public function __construct(DataPersisterInterface $dataPersister, IriConverterI
      */
     public function onKernelView(GetResponseForControllerResultEvent $event): void
     {
+        $controllerResult = $event->getControllerResult();
         $request = $event->getRequest();
-        if ($request->isMethodSafe(false) || !($attributes = RequestAttributesExtractor::extractAttributes($request)) || !$attributes['persist']) {
+
+        if (
+            $controllerResult instanceof Response
+            || $request->isMethodSafe(false)
+            || !($attributes = RequestAttributesExtractor::extractAttributes($request))
+            || !$attributes['persist']
+            || $this->isOperationAttributeDisabled($attributes, self::OPERATION_ATTRIBUTE_KEY)
+        ) {
             return;
         }
 
-        $controllerResult = $event->getControllerResult();
         if (!$this->dataPersister->supports($controllerResult, $attributes)) {
             return;
         }

diff --git a/src/Metadata/Resource/Factory/InputOutputResourceMetadataFactory.php b/src/Metadata/Resource/Factory/InputOutputResourceMetadataFactory.php
@@ -66,12 +66,20 @@ private function getTransformedOperations(array $operations, array $resourceAttr
             $operation['output'] = isset($operation['output']) ? $this->transformInputOutput($operation['output']) : $resourceAttributes['output'];
 
             if (
-                !isset($operation['status'])
-                && isset($operation['output'])
+                isset($operation['input'])
+                && \array_key_exists('class', $operation['input'])
+                && null === $operation['input']['class']
+            ) {
+                $operation['deserialize'] ?? $operation['deserialize'] = false;
+                $operation['validate'] ?? $operation['validate'] = false;
+            }
+
+            if (
+                isset($operation['output'])
                 && \array_key_exists('class', $operation['output'])
                 && null === $operation['output']['class']
             ) {
-                $operation['status'] = 204;
+                $operation['status'] ?? $operation['status'] = 204;
             }
         }
 

diff --git a/src/Metadata/Resource/ToggleableOperationAttributeTrait.php b/src/Metadata/Resource/ToggleableOperationAttributeTrait.php
@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Core\Metadata\Resource;
+
+use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
+
+/**
+ * @internal
+ */
+trait ToggleableOperationAttributeTrait
+{
+    /**
+     * @var ResourceMetadataFactoryInterface|null
+     */
+    private $resourceMetadataFactory;
+
+    private function isOperationAttributeDisabled(array $attributes, string $attribute, bool $default = false, bool $resourceFallback = true): bool
+    {
+        if (null === $this->resourceMetadataFactory) {
+            return $default;
+        }
+
+        $resourceMetadata = $this->resourceMetadataFactory->create($attributes['resource_class']);
+
+        return !((bool) $resourceMetadata->getOperationAttribute($attributes, $attribute, !$default, $resourceFallback));
+    }
+}