diff --git a/phpunit.xml.dist b/phpunit.xml.dist index dd0876323e3..c4c10132275 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -11,6 +11,7 @@ + diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 9e7f9aa21f8..837f67ceae7 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -630,9 +630,10 @@ protected function getFactoryOptions(array $context): array $options['serializer_groups'] = (array) $context[self::GROUPS]; } - $operationCacheKey = ($context['resource_class'] ?? '').($context['operation_name'] ?? '').($context['api_normalize'] ?? ''); - if ($operationCacheKey && isset($this->localFactoryOptionsCache[$operationCacheKey])) { - return $options + $this->localFactoryOptionsCache[$operationCacheKey]; + $operationCacheKey = ($context['resource_class'] ?? '').($context['operation_name'] ?? '').($context['root_operation_name'] ?? ''); + $suffix = ($context['api_normalize'] ?? '') ? 'n' : ''; + if ($operationCacheKey && isset($this->localFactoryOptionsCache[$operationCacheKey.$suffix])) { + return $options + $this->localFactoryOptionsCache[$operationCacheKey.$suffix]; } // This is a hot spot @@ -645,7 +646,7 @@ protected function getFactoryOptions(array $context): array } } - return $options + $this->localFactoryOptionsCache[$operationCacheKey] = $options; + return $options + $this->localFactoryOptionsCache[$operationCacheKey.$suffix] = $options; } /** diff --git a/src/Serializer/Tests/AbstractItemNormalizerTest.php b/src/Serializer/Tests/AbstractItemNormalizerTest.php index 9766c01ee07..f873a49bfe7 100644 --- a/src/Serializer/Tests/AbstractItemNormalizerTest.php +++ b/src/Serializer/Tests/AbstractItemNormalizerTest.php @@ -1545,6 +1545,84 @@ public function testDenormalizeObjectWithNullDisabledTypeEnforcement(): void $this->assertInstanceOf(DtoWithNullValue::class, $actual); $this->assertEquals(new DtoWithNullValue(), $actual); } + + public function testCacheKey(): void + { + $relatedDummy = new RelatedDummy(); + + $dummy = new Dummy(); + $dummy->setName('foo'); + $dummy->setAlias('ignored'); + $dummy->setRelatedDummy($relatedDummy); + $dummy->relatedDummies->add(new RelatedDummy()); + + $relatedDummies = new ArrayCollection([$relatedDummy]); + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(Dummy::class, Argument::type('array'))->willReturn(new PropertyNameCollection(['name', 'alias', 'relatedDummy', 'relatedDummies'])); + + $relatedDummyType = new Type(Type::BUILTIN_TYPE_OBJECT, false, RelatedDummy::class); + $relatedDummiesType = new Type(Type::BUILTIN_TYPE_OBJECT, false, ArrayCollection::class, true, new Type(Type::BUILTIN_TYPE_INT), $relatedDummyType); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', Argument::type('array'))->willReturn((new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_STRING)])->withDescription('')->withReadable(true)); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'alias', Argument::type('array'))->willReturn((new ApiProperty())->withBuiltinTypes([new Type(Type::BUILTIN_TYPE_STRING)])->withDescription('')->withReadable(true)); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy', Argument::type('array'))->willReturn((new ApiProperty())->withBuiltinTypes([$relatedDummyType])->withDescription('')->withReadable(true)->withWritable(false)->withReadableLink(false)); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummies', Argument::type('array'))->willReturn((new ApiProperty())->withBuiltinTypes([$relatedDummiesType])->withReadable(true)->withWritable(false)->withReadableLink(false)); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy->getIriFromResource($dummy, Argument::cetera())->willReturn('/dummies/1'); + $iriConverterProphecy->getIriFromResource($relatedDummy, Argument::cetera())->willReturn('/dummies/2'); + + $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); + $propertyAccessorProphecy->getValue($dummy, 'name')->willReturn('foo'); + $propertyAccessorProphecy->getValue($dummy, 'relatedDummy')->willReturn($relatedDummy); + $propertyAccessorProphecy->getValue($dummy, 'relatedDummies')->willReturn($relatedDummies); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass(null, Dummy::class)->willReturn(Dummy::class); + $resourceClassResolverProphecy->getResourceClass($dummy, null)->willReturn(Dummy::class); + $resourceClassResolverProphecy->getResourceClass($relatedDummy, RelatedDummy::class)->willReturn(RelatedDummy::class); + $resourceClassResolverProphecy->getResourceClass($relatedDummies, RelatedDummy::class)->willReturn(RelatedDummy::class); + $resourceClassResolverProphecy->isResourceClass(Dummy::class)->willReturn(true); + $resourceClassResolverProphecy->isResourceClass(RelatedDummy::class)->willReturn(true); + + $serializerProphecy = $this->prophesize(SerializerInterface::class); + $serializerProphecy->willImplement(NormalizerInterface::class); + $serializerProphecy->normalize('foo', null, Argument::type('array'))->willReturn('foo'); + $serializerProphecy->normalize(['/dummies/2'], null, Argument::type('array'))->willReturn(['/dummies/2']); + + $normalizer = $this->getMockForAbstractClass(AbstractItemNormalizer::class, [ + $propertyNameCollectionFactoryProphecy->reveal(), + $propertyMetadataFactoryProphecy->reveal(), + $iriConverterProphecy->reveal(), + $resourceClassResolverProphecy->reveal(), + $propertyAccessorProphecy->reveal(), + null, + null, + [], + null, + null, + ]); + $normalizer->setSerializer($serializerProphecy->reveal()); + + $expected = [ + 'name' => 'foo', + 'relatedDummy' => '/dummies/2', + 'relatedDummies' => ['/dummies/2'], + ]; + $this->assertSame($expected, $normalizer->normalize($dummy, null, [ + 'resources' => [], + 'groups' => ['group'], + 'ignored_attributes' => ['alias'], + 'operation_name' => 'operation_name', + 'root_operation_name' => 'root_operation_name', + ])); + + $operationCacheKey = (new \ReflectionClass($normalizer))->getProperty('localFactoryOptionsCache')->getValue($normalizer); + $this->assertEquals(array_keys($operationCacheKey), [sprintf('%s%s%s%s', Dummy::class, 'operation_name', 'root_operation_name', 'n')]); + $this->assertEquals(current($operationCacheKey), ['serializer_groups' => ['group']]); + } } class ObjectWithBasicProperties diff --git a/src/State/Provider/ContentNegotiationProvider.php b/src/State/Provider/ContentNegotiationProvider.php index 8b3cb533ff8..05da2a02b65 100644 --- a/src/State/Provider/ContentNegotiationProvider.php +++ b/src/State/Provider/ContentNegotiationProvider.php @@ -97,7 +97,8 @@ private function flattenMimeTypes(array $formats): array */ private function getInputFormat(HttpOperation $operation, Request $request): ?string { - if (null === ($contentType = $request->headers->get('CONTENT_TYPE'))) { + $contentType = $request->headers->get('CONTENT_TYPE'); + if (null === $contentType || '' === $contentType) { return null; } diff --git a/src/State/Provider/DeserializeProvider.php b/src/State/Provider/DeserializeProvider.php index b2217275051..09a3f7b7f51 100644 --- a/src/State/Provider/DeserializeProvider.php +++ b/src/State/Provider/DeserializeProvider.php @@ -56,7 +56,7 @@ public function provide(Operation $operation, array $uriVariables = [], array $c } $contentType = $request->headers->get('CONTENT_TYPE'); - if (null === $contentType) { + if (null === $contentType || '' === $contentType) { throw new UnsupportedMediaTypeHttpException('The "Content-Type" header must exist.'); } diff --git a/src/Symfony/EventListener/DeserializeListener.php b/src/Symfony/EventListener/DeserializeListener.php index a246ad9fef3..00b53ec42eb 100644 --- a/src/Symfony/EventListener/DeserializeListener.php +++ b/src/Symfony/EventListener/DeserializeListener.php @@ -132,7 +132,7 @@ private function getFormat(Request $request, array $formats): string { /** @var ?string $contentType */ $contentType = $request->headers->get('CONTENT_TYPE'); - if (null === $contentType) { + if (null === $contentType || '' === $contentType) { throw new UnsupportedMediaTypeHttpException('The "Content-Type" header must exist.'); } diff --git a/tests/State/Provider/ContentNegotiationProviderTest.php b/tests/State/Provider/ContentNegotiationProviderTest.php new file mode 100644 index 00000000000..e4eb99ba904 --- /dev/null +++ b/tests/State/Provider/ContentNegotiationProviderTest.php @@ -0,0 +1,63 @@ + + * + * 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\Tests\State\Provider; + +use ApiPlatform\Metadata\Post; +use ApiPlatform\State\Provider\ContentNegotiationProvider; +use ApiPlatform\State\ProviderInterface; +use Negotiation\Negotiator; +use PHPUnit\Framework\TestCase; +use Prophecy\Argument; +use Prophecy\PhpUnit\ProphecyTrait; +use Symfony\Component\HttpFoundation\Request; + +class ContentNegotiationProviderTest extends TestCase +{ + use ProphecyTrait; + + public function testRequestWithEmptyContentType(): void + { + $expectedResult = new \stdClass(); + + $decorated = $this->prophesize(ProviderInterface::class); + $decorated->provide(Argument::cetera())->willReturn($expectedResult); + + $negotiator = new Negotiator(); + $formats = ['jsonld' => ['application/ld+json']]; + $errorFormats = ['jsonld' => ['application/ld+json']]; + + $provider = new ContentNegotiationProvider($decorated->reveal(), $negotiator, $formats, $errorFormats); + + // in Symfony (at least up to 7.0.2, 6.4.2, 6.3.11, 5.4.34), a request + // without a content-type and content-length header will result in the + // variables set to an empty string, not null + + $request = new Request( + server: [ + 'REQUEST_METHOD' => 'POST', + 'REQUEST_URI' => '/', + 'CONTENT_TYPE' => '', + 'CONTENT_LENGTH' => '', + ], + content: '' + ); + + $operation = new Post(); + $context = ['request' => $request]; + + $result = $provider->provide($operation, [], $context); + + $this->assertSame($expectedResult, $result); + } +} diff --git a/tests/State/Provider/DeserializeProviderTest.php b/tests/State/Provider/DeserializeProviderTest.php new file mode 100644 index 00000000000..b94c7473ca9 --- /dev/null +++ b/tests/State/Provider/DeserializeProviderTest.php @@ -0,0 +1,63 @@ + + * + * 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\Tests\State\Provider; + +use ApiPlatform\Metadata\Post; +use ApiPlatform\Serializer\SerializerContextBuilderInterface; +use ApiPlatform\State\Provider\DeserializeProvider; +use ApiPlatform\State\ProviderInterface; +use PHPUnit\Framework\TestCase; +use Prophecy\Argument; +use Prophecy\PhpUnit\ProphecyTrait; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\UnsupportedMediaTypeHttpException; +use Symfony\Component\Serializer\SerializerInterface; + +class DeserializeProviderTest extends TestCase +{ + use ProphecyTrait; + + public function testRequestWithEmptyContentType(): void + { + $expectedResult = new \stdClass(); + + $decorated = $this->prophesize(ProviderInterface::class); + $decorated->provide(Argument::cetera())->willReturn($expectedResult); + + $serializer = $this->prophesize(SerializerInterface::class); + $serializerContextBuilder = $this->prophesize(SerializerContextBuilderInterface::class); + + $provider = new DeserializeProvider($decorated->reveal(), $serializer->reveal(), $serializerContextBuilder->reveal()); + + // in Symfony (at least up to 7.0.2, 6.4.2, 6.3.11, 5.4.34), a request + // without a content-type and content-length header will result in the + // variables set to an empty string, not null + + $request = new Request( + server: [ + 'REQUEST_METHOD' => 'POST', + 'REQUEST_URI' => '/', + 'CONTENT_TYPE' => '', + 'CONTENT_LENGTH' => '', + ], + content: '' + ); + + $operation = new Post(deserialize: true); + $context = ['request' => $request]; + + $this->expectException(UnsupportedMediaTypeHttpException::class); + $result = $provider->provide($operation, [], $context); + } +} diff --git a/tests/Symfony/EventListener/DeserializeListenerTest.php b/tests/Symfony/EventListener/DeserializeListenerTest.php index 8290d47b801..f12489bb6e0 100644 --- a/tests/Symfony/EventListener/DeserializeListenerTest.php +++ b/tests/Symfony/EventListener/DeserializeListenerTest.php @@ -28,6 +28,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\Exception\UnsupportedMediaTypeHttpException; +use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Serializer\Exception\NotNormalizableValueException; use Symfony\Component\Serializer\Exception\PartialDenormalizationException; use Symfony\Component\Serializer\Normalizer\AbstractNormalizer; @@ -369,4 +370,56 @@ public function testTurnPartialDenormalizationExceptionIntoValidationException() $this->assertSame($violation->getCode(), 'ba785a8c-82cb-4283-967c-3cf342181b40'); } } + + public function testRequestWithEmptyContentType(): void + { + $serializerProphecy = $this->prophesize(SerializerInterface::class); + $serializerProphecy->deserialize(Argument::cetera())->shouldNotBeCalled(); + + $serializerContextBuilderProphecy = $this->prophesize(SerializerContextBuilderInterface::class); + $serializerContextBuilderProphecy->createFromRequest(Argument::cetera())->willReturn([]); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataFactoryProphecy->create(Argument::cetera())->willReturn(new ResourceMetadataCollection(Dummy::class, [ + new ApiResource(operations: [ + 'post' => new Post(inputFormats: self::FORMATS), + ]), + ]))->shouldBeCalled(); + + $listener = new DeserializeListener( + $serializerProphecy->reveal(), + $serializerContextBuilderProphecy->reveal(), + $resourceMetadataFactoryProphecy->reveal() + ); + + // in Symfony (at least up to 7.0.2, 6.4.2, 6.3.11, 5.4.34), a request + // without a content-type and content-length header will result in the + // variables set to an empty string, not null + + $request = new Request( + server: [ + 'REQUEST_METHOD' => 'POST', + 'REQUEST_URI' => '/', + 'CONTENT_TYPE' => '', + 'CONTENT_LENGTH' => '', + ], + attributes: [ + '_api_resource_class' => Dummy::class, + '_api_operation_name' => 'post', + '_api_receive' => true, + ], + content: '' + ); + + $event = new RequestEvent( + $this->prophesize(HttpKernelInterface::class)->reveal(), + $request, + \defined(HttpKernelInterface::class.'::MAIN_REQUEST') ? HttpKernelInterface::MAIN_REQUEST : HttpKernelInterface::MASTER_REQUEST, + ); + + $this->expectException(UnsupportedMediaTypeHttpException::class); + $this->expectExceptionMessage('The "Content-Type" header must exist.'); + + $listener->onKernelRequest($event); + } }