Prevent cloning generator when instanciating previous_data

Author: nitneuk

features/authorization/deny.feature

@@ -16,6 +16,12 @@ Feature: Authorization checking
     Then the response status code should be 200
     And the response should be in JSON
 
+  Scenario: Data provider that's return generator has null previous object
+    When I add "Accept" header equal to "application/ld+json"
+    And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
+    And I send a "GET" request to "/custom_data_provider_generator"
+    Then the response status code should be 200
+
   Scenario: A standard user cannot create a secured resource
     When I add "Accept" header equal to "application/ld+json"
     And I add "Content-Type" header equal to "application/ld+json"

src/EventListener/ReadListener.php

@@ -23,6 +23,7 @@
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Resource\ToggleableOperationAttributeTrait;
 use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface;
+use ApiPlatform\Core\Util\CloneTrait;
 use ApiPlatform\Core\Util\RequestAttributesExtractor;
 use ApiPlatform\Core\Util\RequestParser;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
@@ -35,6 +36,7 @@
  */
 final class ReadListener
 {
+    use CloneTrait;
     use OperationDataProviderTrait;
     use ToggleableOperationAttributeTrait;
 
@@ -115,6 +117,6 @@ public function onKernelRequest(GetResponseEvent $event): void
         }
 
         $request->attributes->set('data', $data);
-        $request->attributes->set('previous_data', \is_object($data) ? clone $data : $data);
+        $request->attributes->set('previous_data', $this->clone($data));
     }
 }

src/GraphQl/Resolver/Factory/CollectionResolverFactory.php

@@ -23,6 +23,7 @@
 use ApiPlatform\Core\GraphQl\Serializer\ItemNormalizer;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use ApiPlatform\Core\Security\ResourceAccessCheckerInterface;
+use ApiPlatform\Core\Util\CloneTrait;
 use GraphQL\Error\Error;
 use GraphQL\Type\Definition\ResolveInfo;
 use Symfony\Component\HttpFoundation\RequestStack;
@@ -38,6 +39,7 @@
  */
 final class CollectionResolverFactory implements ResolverFactoryInterface
 {
+    use CloneTrait;
     use FieldsToAttributesTrait;
     use ResourceAccessCheckerTrait;
 
@@ -97,7 +99,7 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
 
             $this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
                 'object' => $collection,
-                'previous_object' => \is_object($collection) ? clone $collection : $collection,
+                'previous_object' => $this->clone($collection),
             ], $operationName ?? 'query');
 
             if (!$this->paginationEnabled) {

src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

@@ -24,6 +24,7 @@
 use ApiPlatform\Core\Metadata\Resource\ResourceMetadata;
 use ApiPlatform\Core\Security\ResourceAccessCheckerInterface;
 use ApiPlatform\Core\Util\ClassInfoTrait;
+use ApiPlatform\Core\Util\CloneTrait;
 use ApiPlatform\Core\Validator\Exception\ValidationException;
 use ApiPlatform\Core\Validator\ValidatorInterface;
 use GraphQL\Error\Error;
@@ -41,6 +42,7 @@
 final class ItemMutationResolverFactory implements ResolverFactoryInterface
 {
     use ClassInfoTrait;
+    use CloneTrait;
     use FieldsToAttributesTrait;
     use ResourceAccessCheckerTrait;
 
@@ -96,7 +98,7 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
                     throw Error::createLocatedError(sprintf('Item "%s" did not match expected type "%s".', $args['input']['id'], $resourceMetadata->getShortName()), $info->fieldNodes, $info->path);
                 }
             }
-            $previousItem = \is_object($item) ? clone $item : $item;
+            $previousItem = $this->clone($item);
 
             $inputMetadata = $resourceMetadata->getGraphqlAttribute($operationName, 'input', null, true);
             $inputClass = null;

src/Util/CloneTrait.php

@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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\Util;
+
+/**
+ * Clones given data if cloneable.
+ *
+ * @internal
+ *
+ * @author Quentin Barloy <[email protected]>
+ */
+trait CloneTrait
+{
+    public function clone($data)
+    {
+        if (!\is_object($data)) {
+            return $data;
+        }
+
+        try {
+            return (new \ReflectionClass($data))->isCloneable() ? clone $data : null;
+        } catch (\ReflectionException $reflectionException) {
+            return null;
+        }
+    }
+}

tests/Fixtures/TestBundle/DataProvider/GeneratorDataProvider.php

@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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\Tests\Fixtures\TestBundle\DataProvider;
+
+use ApiPlatform\Core\DataProvider\CollectionDataProviderInterface;
+use ApiPlatform\Core\DataProvider\RestrictedDataProviderInterface;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\SecuredDummy;
+
+class GeneratorDataProvider implements CollectionDataProviderInterface, RestrictedDataProviderInterface
+{
+    public function supports(string $resourceClass, string $operationName = null, array $context = []): bool
+    {
+        return SecuredDummy::class === $resourceClass && 'get_from_data_provider_generator' === $operationName;
+    }
+
+    public function getCollection(string $resourceClass, string $operationName = null)
+    {
+        yield from [new class() {
+        }, new class() {
+        }];
+    }
+}

tests/Fixtures/TestBundle/Document/SecuredDummy.php

@@ -27,6 +27,11 @@
  *     attributes={"access_control"="is_granted('ROLE_USER')"},
  *     collectionOperations={
  *         "get",
+ *         "get_from_data_provider_generator"={
+ *             "method"="GET",
+ *             "path"="custom_data_provider_generator",
+ *             "access_control"="is_granted('ROLE_USER')"
+ *         },
  *         "post"={"access_control"="is_granted('ROLE_ADMIN')"}
  *     },
  *     itemOperations={

tests/Fixtures/TestBundle/Entity/SecuredDummy.php

@@ -26,6 +26,11 @@
  *     attributes={"access_control"="is_granted('ROLE_USER')"},
  *     collectionOperations={
  *         "get",
+ *         "get_from_data_provider_generator"={
+ *             "method"="GET",
+ *             "path"="custom_data_provider_generator",
+ *             "access_control"="is_granted('ROLE_USER')"
+ *         },
  *         "post"={"access_control"="is_granted('ROLE_ADMIN')"}
  *     },
  *     itemOperations={

tests/Fixtures/app/config/config_test.yml

@@ -73,6 +73,11 @@ services:
         arguments: [ { 'name': 'ipartial', 'description': 'ipartial' } ]
         tags:      [ { name: 'api_platform.filter', id: 'related_to_dummy_friend.name' } ]
 
+    ApiPlatform\Core\Tests\Fixtures\TestBundle\DataProvider\GeneratorDataProvider:
+        public: false
+        tags:
+            -   name: 'api_platform.item_data_provider'
+
     ApiPlatform\Core\Tests\Fixtures\TestBundle\DataProvider\ProductItemDataProvider:
         public: false
         arguments:

tests/Util/CloneTraitTest.php

@@ -0,0 +1,49 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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\Tests\Util;
+
+use ApiPlatform\Core\Util\CloneTrait;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @author Quentin Barloy <[email protected]>
+ */
+class CloneTraitTest extends TestCase
+{
+    use CloneTrait;
+
+    public function testScalarClone(): void
+    {
+        $this->assertSame(5, $this->clone(5));
+    }
+
+    public function testObjectClone(): void
+    {
+        $data = new \stdClass();
+        $result = $this->clone($data);
+
+        $this->assertNotSame($data, $result);
+        $this->assertEquals($data, $result);
+    }
+
+    public function testGeneratorClone(): void
+    {
+        $this->assertNull($this->clone($this->generator()));
+    }
+
+    private function generator(): \Generator
+    {
+        yield 1;
+    }
+}