From fabee6faf52639d9a16d0009381daefe14de3595 Mon Sep 17 00:00:00 2001 From: Aleksey Polyvanyi Date: Sat, 7 Dec 2024 00:20:11 +0100 Subject: [PATCH 1/3] fix(doctrine): handle invalid uuid in ORM SearchFilter --- .../Common/Filter/SearchFilterTrait.php | 37 ++++- src/Doctrine/Odm/Filter/SearchFilter.php | 1 + src/Doctrine/Orm/Filter/SearchFilter.php | 6 + .../Orm/Tests/DoctrineOrmFilterTestCase.php | 2 +- .../Tests/Filter/SearchFilterWithUuidTest.php | 129 ++++++++++++++++++ .../Orm/Tests/Fixtures/Entity/DummyCar.php | 6 +- .../Fixtures/Entity/GuidIdentifierDummy.php | 51 +++++++ .../Entity/RelatedUuidIdentifierDummy.php | 37 +++++ .../Fixtures/Entity/UuidIdentifierDummy.php | 45 ++++-- src/Doctrine/Orm/Tests/config.yml | 3 - 10 files changed, 295 insertions(+), 22 deletions(-) create mode 100644 src/Doctrine/Orm/Tests/Filter/SearchFilterWithUuidTest.php create mode 100644 src/Doctrine/Orm/Tests/Fixtures/Entity/GuidIdentifierDummy.php create mode 100644 src/Doctrine/Orm/Tests/Fixtures/Entity/RelatedUuidIdentifierDummy.php diff --git a/src/Doctrine/Common/Filter/SearchFilterTrait.php b/src/Doctrine/Common/Filter/SearchFilterTrait.php index d04fe78ba19..9b66280bdac 100644 --- a/src/Doctrine/Common/Filter/SearchFilterTrait.php +++ b/src/Doctrine/Common/Filter/SearchFilterTrait.php @@ -122,6 +122,14 @@ abstract protected function normalizePropertyName(string $property): string; */ protected function getIdFromValue(string $value): mixed { + if (is_numeric($value)) { + return $value; + } + + if ($this->isValidUuid($value)) { + return $value; + } + try { $iriConverter = $this->getIriConverter(); $item = $iriConverter->getResourceFromIri($value, ['fetch_data' => false]); @@ -163,16 +171,41 @@ protected function normalizeValues(array $values, string $property): ?array } /** - * When the field should be an integer, check that the given value is a valid one. + * Check if the values are valid for the given Doctrine type. */ protected function hasValidValues(array $values, ?string $type = null): bool { foreach ($values as $value) { - if (null !== $value && \in_array($type, (array) self::DOCTRINE_INTEGER_TYPE, true) && false === filter_var($value, \FILTER_VALIDATE_INT)) { + if (null === $value) { + continue; + } + + if (\in_array($type, (array) self::DOCTRINE_INTEGER_TYPE, true) && false === filter_var($value, \FILTER_VALIDATE_INT)) { + return false; + } + + if (\in_array($type, (array) self::DOCTRINE_UUID_TYPE, true) && false === $this->isValidUuid($value)) { return false; } } return true; } + + protected function isValidUuid(mixed $value): bool + { + if (!\is_string($value)) { + return false; + } + + if (class_exists('\Symfony\Component\Uid\Uuid')) { + return \Symfony\Component\Uid\Uuid::isValid($value); + } + + if (class_exists('\Ramsey\Uuid\Uuid')) { + return \Ramsey\Uuid\Uuid::isValid($value); + } + + return 1 === preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i', $value); + } } diff --git a/src/Doctrine/Odm/Filter/SearchFilter.php b/src/Doctrine/Odm/Filter/SearchFilter.php index fdb8c9d271c..14308c6dec6 100644 --- a/src/Doctrine/Odm/Filter/SearchFilter.php +++ b/src/Doctrine/Odm/Filter/SearchFilter.php @@ -141,6 +141,7 @@ final class SearchFilter extends AbstractFilter implements SearchFilterInterface use SearchFilterTrait; public const DOCTRINE_INTEGER_TYPE = [MongoDbType::INTEGER, MongoDbType::INT]; + public const DOCTRINE_UUID_TYPE = []; public function __construct(ManagerRegistry $managerRegistry, IriConverterInterface|LegacyIriConverterInterface $iriConverter, IdentifiersExtractorInterface|LegacyIdentifiersExtractorInterface|null $identifiersExtractor, ?PropertyAccessorInterface $propertyAccessor = null, ?LoggerInterface $logger = null, ?array $properties = null, ?NameConverterInterface $nameConverter = null) { diff --git a/src/Doctrine/Orm/Filter/SearchFilter.php b/src/Doctrine/Orm/Filter/SearchFilter.php index a94c8627ec0..92df1eb71f1 100644 --- a/src/Doctrine/Orm/Filter/SearchFilter.php +++ b/src/Doctrine/Orm/Filter/SearchFilter.php @@ -140,6 +140,7 @@ final class SearchFilter extends AbstractFilter implements SearchFilterInterface use SearchFilterTrait; public const DOCTRINE_INTEGER_TYPE = Types::INTEGER; + public const DOCTRINE_UUID_TYPE = 'uuid'; public function __construct(ManagerRegistry $managerRegistry, IriConverterInterface|LegacyIriConverterInterface $iriConverter, ?PropertyAccessorInterface $propertyAccessor = null, ?LoggerInterface $logger = null, ?array $properties = null, IdentifiersExtractorInterface|LegacyIdentifiersExtractorInterface|null $identifiersExtractor = null, ?NameConverterInterface $nameConverter = null) { @@ -231,6 +232,11 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB if (is_numeric($value)) { return $value; } + + if ($this->isValidUuid($value)) { + return $value; + } + try { $item = $this->getIriConverter()->getResourceFromIri($value, ['fetch_data' => false]); diff --git a/src/Doctrine/Orm/Tests/DoctrineOrmFilterTestCase.php b/src/Doctrine/Orm/Tests/DoctrineOrmFilterTestCase.php index f091da00923..df16707cf0d 100644 --- a/src/Doctrine/Orm/Tests/DoctrineOrmFilterTestCase.php +++ b/src/Doctrine/Orm/Tests/DoctrineOrmFilterTestCase.php @@ -42,7 +42,7 @@ protected function setUp(): void self::bootKernel(); $this->managerRegistry = self::$kernel->getContainer()->get('doctrine'); - $this->repository = $this->managerRegistry->getManagerForClass(Dummy::class)->getRepository(Dummy::class); + $this->repository = $this->managerRegistry->getManagerForClass($this->resourceClass)->getRepository($this->resourceClass); } /** diff --git a/src/Doctrine/Orm/Tests/Filter/SearchFilterWithUuidTest.php b/src/Doctrine/Orm/Tests/Filter/SearchFilterWithUuidTest.php new file mode 100644 index 00000000000..2f9b369cae0 --- /dev/null +++ b/src/Doctrine/Orm/Tests/Filter/SearchFilterWithUuidTest.php @@ -0,0 +1,129 @@ + + * + * 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\Doctrine\Orm\Tests\Filter; + +use ApiPlatform\Doctrine\Orm\Filter\SearchFilter; +use ApiPlatform\Doctrine\Orm\Tests\DoctrineOrmFilterTestCase; +use ApiPlatform\Doctrine\Orm\Tests\Fixtures\CustomConverter; +use ApiPlatform\Doctrine\Orm\Tests\Fixtures\Entity\UuidIdentifierDummy; +use ApiPlatform\Metadata\Exception\InvalidArgumentException; +use ApiPlatform\Metadata\IriConverterInterface; +use Doctrine\Persistence\ManagerRegistry; +use Prophecy\Argument; +use Prophecy\PhpUnit\ProphecyTrait; + +/** + * @author Oleksii Polyvanyi + */ +class SearchFilterWithUuidTest extends DoctrineOrmFilterTestCase +{ + use ProphecyTrait; + + protected string $resourceClass = UuidIdentifierDummy::class; + protected string $filterClass = SearchFilter::class; + + public static function provideApplyTestData(): array + { + $filterFactory = self::buildSearchFilter(...); + $validUuid = '9584fbef-e849-41e3-912b-f2c509874a70'; + + return [ + 'invalid uuid for id' => [ + [ + 'id' => 'exact', + ], + [ + 'id' => 'some-invalid-uuid', + ], + 'SELECT o FROM ApiPlatform\Doctrine\Orm\Tests\Fixtures\Entity\UuidIdentifierDummy o', + [], + $filterFactory, + ], + + 'valid uuid for id' => [ + [ + 'id' => 'exact', + ], + [ + 'id' => $validUuid, + ], + 'SELECT o FROM ApiPlatform\Doctrine\Orm\Tests\Fixtures\Entity\UuidIdentifierDummy o WHERE o.id = :id_p1', + ['id_p1' => $validUuid], + $filterFactory, + ], + + 'invalid uuid for uuidField' => [ + [ + 'uuidField' => 'exact', + ], + [ + 'uuidField' => 'some-invalid-uuid', + ], + 'SELECT o FROM ApiPlatform\Doctrine\Orm\Tests\Fixtures\Entity\UuidIdentifierDummy o', + [], + $filterFactory, + ], + + 'valid uuid for uuidField' => [ + [ + 'uuidField' => 'exact', + ], + [ + 'uuidField' => $validUuid, + ], + 'SELECT o FROM ApiPlatform\Doctrine\Orm\Tests\Fixtures\Entity\UuidIdentifierDummy o WHERE o.uuidField = :uuidField_p1', + ['uuidField_p1' => $validUuid], + $filterFactory, + ], + + 'invalid uuid for relatedUuidIdentifierDummy' => [ + [ + 'relatedUuidIdentifierDummy' => 'exact', + ], + [ + 'relatedUuidIdentifierDummy' => 'some-invalid-uuid', + ], + 'SELECT o FROM ApiPlatform\Doctrine\Orm\Tests\Fixtures\Entity\UuidIdentifierDummy o', + [], + $filterFactory, + ], + + 'valid uuid for relatedUuidIdentifierDummy' => [ + [ + 'relatedUuidIdentifierDummy' => 'exact', + ], + [ + 'relatedUuidIdentifierDummy' => $validUuid, + ], + 'SELECT o FROM ApiPlatform\Doctrine\Orm\Tests\Fixtures\Entity\UuidIdentifierDummy o WHERE o.relatedUuidIdentifierDummy = :relatedUuidIdentifierDummy_p1', + ['relatedUuidIdentifierDummy_p1' => $validUuid], + $filterFactory, + ], + ]; + } + + protected static function buildSearchFilter(self $that, ManagerRegistry $managerRegistry, ?array $properties = null): SearchFilter + { + $iriConverterProphecy = $that->prophesize(IriConverterInterface::class); + + $iriConverterProphecy->getResourceFromIri(Argument::type('string'), ['fetch_data' => false])->will(function (): void { + throw new InvalidArgumentException(); + }); + + $iriConverter = $iriConverterProphecy->reveal(); + $propertyAccessor = static::$kernel->getContainer()->get('test.property_accessor'); + + return new SearchFilter($managerRegistry, $iriConverter, $propertyAccessor, null, $properties, null, new CustomConverter()); + } +} diff --git a/src/Doctrine/Orm/Tests/Fixtures/Entity/DummyCar.php b/src/Doctrine/Orm/Tests/Fixtures/Entity/DummyCar.php index ba9fd297690..5af770c83f9 100644 --- a/src/Doctrine/Orm/Tests/Fixtures/Entity/DummyCar.php +++ b/src/Doctrine/Orm/Tests/Fixtures/Entity/DummyCar.php @@ -49,10 +49,10 @@ class DummyCar #[ORM\OneToMany(targetEntity: DummyCarColor::class, mappedBy: 'car')] private Collection|iterable|null $thirdColors = null; #[ApiFilter(SearchFilter::class, strategy: 'exact')] - #[ORM\ManyToMany(targetEntity: UuidIdentifierDummy::class, indexBy: 'uuid')] + #[ORM\ManyToMany(targetEntity: GuidIdentifierDummy::class, indexBy: 'guid')] #[ORM\JoinColumn(name: 'car_id', referencedColumnName: 'id_id')] - #[ORM\InverseJoinColumn(name: 'uuid_uuid', referencedColumnName: 'uuid')] - #[ORM\JoinTable(name: 'uuid_cars')] + #[ORM\InverseJoinColumn(name: 'guid_guid', referencedColumnName: 'guid')] + #[ORM\JoinTable(name: 'guid_cars')] private Collection|iterable|null $uuid = null; #[ApiFilter(SearchFilter::class, strategy: 'partial')] diff --git a/src/Doctrine/Orm/Tests/Fixtures/Entity/GuidIdentifierDummy.php b/src/Doctrine/Orm/Tests/Fixtures/Entity/GuidIdentifierDummy.php new file mode 100644 index 00000000000..d1d1969dd68 --- /dev/null +++ b/src/Doctrine/Orm/Tests/Fixtures/Entity/GuidIdentifierDummy.php @@ -0,0 +1,51 @@ + + * + * 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\Doctrine\Orm\Tests\Fixtures\Entity; + +use ApiPlatform\Metadata\ApiResource; +use Doctrine\ORM\Mapping as ORM; + +/** + * Custom identifier dummy. + */ +#[ApiResource] +#[ORM\Entity] +class GuidIdentifierDummy +{ + #[ORM\Column(type: 'guid')] + #[ORM\Id] + private ?string $guid = null; + #[ORM\Column(length: 30)] + private ?string $name = null; + + public function getGuid(): ?string + { + return $this->guid; + } + + public function setGuid(string $guid): void + { + $this->guid = $guid; + } + + public function getName(): ?string + { + return $this->name; + } + + public function setName(string $name): void + { + $this->name = $name; + } +} diff --git a/src/Doctrine/Orm/Tests/Fixtures/Entity/RelatedUuidIdentifierDummy.php b/src/Doctrine/Orm/Tests/Fixtures/Entity/RelatedUuidIdentifierDummy.php new file mode 100644 index 00000000000..d925e189017 --- /dev/null +++ b/src/Doctrine/Orm/Tests/Fixtures/Entity/RelatedUuidIdentifierDummy.php @@ -0,0 +1,37 @@ + + * + * 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\Doctrine\Orm\Tests\Fixtures\Entity; + +use ApiPlatform\Metadata\ApiResource; +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Uid\Uuid; + +#[ApiResource] +#[ORM\Entity] +class RelatedUuidIdentifierDummy +{ + #[ORM\Column(type: 'uuid')] + #[ORM\Id] + private Uuid $id; + + public function getId(): Uuid + { + return $this->id; + } + + public function setId(Uuid $id): void + { + $this->id = $id; + } +} diff --git a/src/Doctrine/Orm/Tests/Fixtures/Entity/UuidIdentifierDummy.php b/src/Doctrine/Orm/Tests/Fixtures/Entity/UuidIdentifierDummy.php index 12535642a5b..1abdba63e31 100644 --- a/src/Doctrine/Orm/Tests/Fixtures/Entity/UuidIdentifierDummy.php +++ b/src/Doctrine/Orm/Tests/Fixtures/Entity/UuidIdentifierDummy.php @@ -13,39 +13,58 @@ namespace ApiPlatform\Doctrine\Orm\Tests\Fixtures\Entity; +use ApiPlatform\Doctrine\Orm\Filter\SearchFilter; +use ApiPlatform\Metadata\ApiFilter; use ApiPlatform\Metadata\ApiResource; use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Uid\Uuid; /** - * Custom identifier dummy. + * UUID identifier dummy. */ #[ApiResource] +#[ApiFilter(SearchFilter::class, properties: ['id' => 'exact', 'uuidField' => 'exact', 'relatedUuidIdentifierDummy' => 'exact'])] #[ORM\Entity] class UuidIdentifierDummy { - #[ORM\Column(type: 'guid')] + #[ORM\Column(type: 'uuid')] #[ORM\Id] - private ?string $uuid = null; - #[ORM\Column(length: 30)] - private ?string $name = null; + private Uuid $id; - public function getUuid(): ?string + #[ORM\Column(type: 'uuid')] + private Uuid $uuidField; + + #[ORM\JoinColumn(nullable: false)] + #[ORM\ManyToOne(targetEntity: RelatedUuidIdentifierDummy::class)] + private RelatedUuidIdentifierDummy $relatedUuidIdentifierDummy; + + public function getId(): Uuid + { + return $this->id; + } + + public function setId(Uuid $id): void + { + $this->id = $id; + } + + public function getUuidField(): Uuid { - return $this->uuid; + return $this->uuidField; } - public function setUuid(string $uuid): void + public function setUuidField(Uuid $uuidField): void { - $this->uuid = $uuid; + $this->uuidField = $uuidField; } - public function getName(): ?string + public function getRelatedUuidIdentifierDummy(): RelatedUuidIdentifierDummy { - return $this->name; + return $this->relatedUuidIdentifierDummy; } - public function setName(string $name): void + public function setRelatedUuidIdentifierDummy(RelatedUuidIdentifierDummy $relatedUuidIdentifierDummy): void { - $this->name = $name; + $this->relatedUuidIdentifierDummy = $relatedUuidIdentifierDummy; } } diff --git a/src/Doctrine/Orm/Tests/config.yml b/src/Doctrine/Orm/Tests/config.yml index f89f33e7d0f..8dbc7bfbd5a 100644 --- a/src/Doctrine/Orm/Tests/config.yml +++ b/src/Doctrine/Orm/Tests/config.yml @@ -2,9 +2,6 @@ doctrine: dbal: driver: 'pdo_sqlite' charset: 'UTF8' - types: - uuid: Ramsey\Uuid\Doctrine\UuidType - symfony_uuid: Symfony\Bridge\Doctrine\Types\UuidType orm: auto_generate_proxy_classes: '%kernel.debug%' From 313d3e9d465c1c3575c002d4a1d2774347059b30 Mon Sep 17 00:00:00 2001 From: Aleksey Polyvanyi Date: Sat, 7 Dec 2024 00:34:56 +0100 Subject: [PATCH 2/3] -refactoring --- src/Doctrine/Common/Filter/SearchFilterTrait.php | 2 +- src/Doctrine/Odm/Filter/SearchFilter.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Doctrine/Common/Filter/SearchFilterTrait.php b/src/Doctrine/Common/Filter/SearchFilterTrait.php index 9b66280bdac..5456f3a8815 100644 --- a/src/Doctrine/Common/Filter/SearchFilterTrait.php +++ b/src/Doctrine/Common/Filter/SearchFilterTrait.php @@ -184,7 +184,7 @@ protected function hasValidValues(array $values, ?string $type = null): bool return false; } - if (\in_array($type, (array) self::DOCTRINE_UUID_TYPE, true) && false === $this->isValidUuid($value)) { + if ($type === self::DOCTRINE_UUID_TYPE && false === $this->isValidUuid($value)) { return false; } } diff --git a/src/Doctrine/Odm/Filter/SearchFilter.php b/src/Doctrine/Odm/Filter/SearchFilter.php index 14308c6dec6..920bd04a4d5 100644 --- a/src/Doctrine/Odm/Filter/SearchFilter.php +++ b/src/Doctrine/Odm/Filter/SearchFilter.php @@ -141,7 +141,7 @@ final class SearchFilter extends AbstractFilter implements SearchFilterInterface use SearchFilterTrait; public const DOCTRINE_INTEGER_TYPE = [MongoDbType::INTEGER, MongoDbType::INT]; - public const DOCTRINE_UUID_TYPE = []; + public const DOCTRINE_UUID_TYPE = null; public function __construct(ManagerRegistry $managerRegistry, IriConverterInterface|LegacyIriConverterInterface $iriConverter, IdentifiersExtractorInterface|LegacyIdentifiersExtractorInterface|null $identifiersExtractor, ?PropertyAccessorInterface $propertyAccessor = null, ?LoggerInterface $logger = null, ?array $properties = null, ?NameConverterInterface $nameConverter = null) { From 9bdd539d2ef2c896dcb98d435a645c31a7c1b893 Mon Sep 17 00:00:00 2001 From: Aleksey Polyvanyi Date: Sat, 7 Dec 2024 00:37:57 +0100 Subject: [PATCH 3/3] -refactoring --- src/Doctrine/Common/Filter/SearchFilterTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Doctrine/Common/Filter/SearchFilterTrait.php b/src/Doctrine/Common/Filter/SearchFilterTrait.php index 5456f3a8815..ac815bed8fe 100644 --- a/src/Doctrine/Common/Filter/SearchFilterTrait.php +++ b/src/Doctrine/Common/Filter/SearchFilterTrait.php @@ -184,7 +184,7 @@ protected function hasValidValues(array $values, ?string $type = null): bool return false; } - if ($type === self::DOCTRINE_UUID_TYPE && false === $this->isValidUuid($value)) { + if (self::DOCTRINE_UUID_TYPE === $type && false === $this->isValidUuid($value)) { return false; } }