From ee838559115734dcff2a9f0ac0b2858b1d4bd3db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 11:33:00 +0100 Subject: [PATCH 1/2] Reject cookies with an empty client id or one longer than 255 characters --- .../RequestBasedCookieProvider.php | 11 ++++++- tests/Functional/ClientCookieTest.php | 14 ++++++++ .../RequestBasedCookieProviderTest.php | 33 +++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/CookieProvider/RequestBasedCookieProvider.php b/src/CookieProvider/RequestBasedCookieProvider.php index 4b9292d..0b0efb3 100644 --- a/src/CookieProvider/RequestBasedCookieProvider.php +++ b/src/CookieProvider/RequestBasedCookieProvider.php @@ -10,6 +10,8 @@ final class RequestBasedCookieProvider implements CookieProviderInterface { + private const MAX_CLIENT_ID_LENGTH = 255; + public function __construct( private readonly RequestStack $requestStack, private readonly string $cookieName, @@ -24,9 +26,16 @@ public function getCookie(?Request $request = null): ?Cookie } try { - return Cookie::fromString($cookieValue); + $cookie = Cookie::fromString($cookieValue); } catch (\InvalidArgumentException) { return null; } + + // All visitors with an empty client id would share one client, and the database column holds 255 characters + if ('' === $cookie->clientId || strlen($cookie->clientId) > self::MAX_CLIENT_ID_LENGTH) { + return null; + } + + return $cookie; } } diff --git a/tests/Functional/ClientCookieTest.php b/tests/Functional/ClientCookieTest.php index 53967df..4aca8c9 100644 --- a/tests/Functional/ClientCookieTest.php +++ b/tests/Functional/ClientCookieTest.php @@ -101,6 +101,20 @@ public function it_treats_a_visitor_with_an_invalid_cookie_as_a_new_visitor(): v self::assertSame($clientId, $this->browser->getResponse()->getContent()); } + /** + * @test + */ + public function it_treats_a_visitor_with_an_empty_client_id_as_a_new_visitor(): void + { + $this->setCookie('2.1.1.'); + + $this->browser->request('GET', '/'); + + $clientId = $this->getClientId(); + self::assertTrue(Uuid::isValid($clientId)); + self::assertSame($clientId, $this->browser->getResponse()->getContent()); + } + /** * @test */ diff --git a/tests/Unit/CookieProvider/RequestBasedCookieProviderTest.php b/tests/Unit/CookieProvider/RequestBasedCookieProviderTest.php index a24cab7..d749510 100644 --- a/tests/Unit/CookieProvider/RequestBasedCookieProviderTest.php +++ b/tests/Unit/CookieProvider/RequestBasedCookieProviderTest.php @@ -56,6 +56,39 @@ public function it_returns_null_for_an_empty_cookie_value(): void self::assertNull($provider->getCookie($request)); } + /** + * @test + */ + public function it_returns_null_when_the_client_id_is_empty(): void + { + $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); + $request = new Request([], [], [], [self::COOKIE_NAME => '2.1.1.']); + + self::assertNull($provider->getCookie($request)); + } + + /** + * @test + */ + public function it_returns_null_when_the_client_id_is_longer_than_255_characters(): void + { + $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); + $request = new Request([], [], [], [self::COOKIE_NAME => (new Cookie(str_repeat('a', 256)))->toString()]); + + self::assertNull($provider->getCookie($request)); + } + + /** + * @test + */ + public function it_accepts_a_client_id_of_255_characters(): void + { + $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); + $request = new Request([], [], [], [self::COOKIE_NAME => (new Cookie(str_repeat('a', 255)))->toString()]); + + self::assertSame(str_repeat('a', 255), $provider->getCookie($request)?->clientId); + } + /** * @test */ From c702a125f239898021e4809b1587c0c1f63bc453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 11:33:12 +0100 Subject: [PATCH 2/2] Look up the entity manager only when the metadata is used --- src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php | 7 +++---- .../DoctrineOrmBasedMetadataProviderTest.php | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php b/src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php index e5aa281..5d966db 100644 --- a/src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php +++ b/src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php @@ -27,11 +27,10 @@ public function __construct( public function getMetadata(string $clientId): Metadata { - $manager = $this->getManager($this->metadataEntityClass); - - return LazyChangeAwareMetadata::createLazyGhost(function (Metadata $instance) use ($clientId, $manager): void { + // The manager is looked up when the metadata is used, so a request that doesn't use it doesn't start the entity manager + return LazyChangeAwareMetadata::createLazyGhost(function (Metadata $instance) use ($clientId): void { /** @var MetadataInterface|null $entity */ - $entity = $manager->find($this->metadataEntityClass, $clientId); + $entity = $this->getManager($this->metadataEntityClass)->find($this->metadataEntityClass, $clientId); if (null === $entity) { $metadata = $this->decorated->getMetadata($clientId)->toArray(); diff --git a/tests/Unit/MetadataProvider/DoctrineOrmBasedMetadataProviderTest.php b/tests/Unit/MetadataProvider/DoctrineOrmBasedMetadataProviderTest.php index 9fefe8e..f9810ac 100644 --- a/tests/Unit/MetadataProvider/DoctrineOrmBasedMetadataProviderTest.php +++ b/tests/Unit/MetadataProvider/DoctrineOrmBasedMetadataProviderTest.php @@ -25,12 +25,12 @@ final class DoctrineOrmBasedMetadataProviderTest extends TestCase */ public function it_returns_lazy_metadata(): void { - $manager = $this->prophesize(EntityManagerInterface::class); - $manager->find(Argument::cetera())->shouldNotBeCalled(); + $managerRegistry = $this->prophesize(ManagerRegistry::class); + $managerRegistry->getManagerForClass(Argument::cetera())->shouldNotBeCalled(); $decorated = $this->prophesize(MetadataProviderInterface::class); - $metadata = $this->createProvider($this->createManagerRegistry($manager), $decorated)->getMetadata('some-client-id'); + $metadata = $this->createProvider($managerRegistry, $decorated)->getMetadata('some-client-id'); self::assertInstanceOf(LazyChangeAwareMetadata::class, $metadata); self::assertFalse($metadata->isLazyObjectInitialized());