Reject unusable client ids and look up the entity manager lazily

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;
     }
 }

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();

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
      */

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
      */

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());