Validate and compile the user agent filter pattern once

README.md

@@ -92,7 +92,8 @@ setono_meta_conversions_api:
         value: null
 
     filters:
-        # Regular expression fragments (no delimiters). Events with a matching user agent are not tracked
+        # Regular expression fragments without delimiters, matched case insensitively. Events with a matching user
+        # agent are not tracked. Invalid fragments, and fragments containing an unescaped '#', fail at compile time
         user_agent: []
 ```
 

src/DependencyInjection/Configuration.php

@@ -83,7 +83,14 @@ public function getConfigTreeBuilder(): TreeBuilder
                 ->addDefaultsIfNotSet()
                 ->children()
                     ->arrayNode('user_agent')
-                        ->scalarPrototype()->end()
+                        ->info('Regular expression fragments without delimiters. Events with a matching user agent are not tracked. Matching is case insensitive')
+                        ->scalarPrototype()
+                            ->cannotBeEmpty()
+                            ->validate()
+                                ->ifTrue(static fn (mixed $fragment): bool => !is_string($fragment) || false === @preg_match('#' . $fragment . '#i', ''))
+                                ->thenInvalid('%s is not a valid regular expression fragment. Remember to escape the "#" delimiter')
+                            ->end()
+                        ->end()
                     ->end()
                 ->end()
             ->end()

src/EventSubscriber/FilterConfiguredUserAgentsSubscriber.php

@@ -10,10 +10,35 @@
 final class FilterConfiguredUserAgentsSubscriber implements EventSubscriberInterface
 {
     /**
-     * @param list<string> $userAgents
+     * The compiled pattern, or null when no user agents are configured
      */
-    public function __construct(private readonly array $userAgents)
+    private readonly ?string $pattern;
+
+    /**
+     * @param list<string> $userAgents Regular expression fragments without delimiters
+     *
+     * @throws \InvalidArgumentException if the fragments do not compile into a valid regular expression
+     */
+    public function __construct(array $userAgents)
     {
+        if ([] === $userAgents) {
+            $this->pattern = null;
+
+            return;
+        }
+
+        $pattern = '#' . implode('|', $userAgents) . '#i';
+
+        // Compiling once up front turns a typo into a boot failure instead of a filter that silently stops matching:
+        // preg_match() returns false (not 1) for an invalid pattern, so the previous code just never filtered again
+        if (false === @preg_match($pattern, '')) {
+            throw new \InvalidArgumentException(sprintf(
+                'The configured user agent filters do not compile into a valid regular expression: "%s". Remember to escape the "#" delimiter inside a fragment',
+                $pattern,
+            ));
+        }
+
+        $this->pattern = $pattern;
     }
 
     public static function getSubscribedEvents(): array
@@ -25,16 +50,16 @@ public static function getSubscribedEvents(): array
 
     public function filter(ConversionsApiEventRaised $event): void
     {
-        if ([] === $this->userAgents) {
+        if (null === $this->pattern) {
             return;
         }
 
-        $ua = $event->event->userData->clientUserAgent;
-        if (null === $ua) {
+        $userAgent = $event->event->userData->clientUserAgent;
+        if (null === $userAgent) {
             return;
         }
-        $regex = '#' . implode('|', $this->userAgents) . '#';
-        if (preg_match($regex, $ua) === 1) {
+
+        if (1 === preg_match($this->pattern, $userAgent)) {
             $event->stopPropagation();
         }
     }

tests/Integration/DependencyInjection/SetonoMetaConversionsApiExtensionTest.php

@@ -15,6 +15,7 @@
 use Setono\MetaConversionsApiBundle\EventSubscriber\AddLibraryToTagBagSubscriber;
 use Setono\MetaConversionsApiBundle\EventSubscriber\StoreTestEventCodeSubscriber;
 use Setono\TagBagBundle\SetonoTagBagBundle;
+use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 
 #[CoversClass(SetonoMetaConversionsApiExtension::class)]
 final class SetonoMetaConversionsApiExtensionTest extends AbstractExtensionTestCase
@@ -68,6 +69,30 @@ public function it_does_not_load_client_side_event_subscribers_when_client_side_
         $this->assertContainerBuilderNotHasService(AddLibraryToTagBagSubscriber::class);
     }
 
+    #[Test]
+    public function it_rejects_a_user_agent_filter_that_is_not_a_valid_regular_expression(): void
+    {
+        $this->expectException(InvalidConfigurationException::class);
+
+        $this->load([
+            'filters' => [
+                'user_agent' => ['(unbalanced'],
+            ],
+        ]);
+    }
+
+    #[Test]
+    public function it_rejects_a_user_agent_filter_with_an_unescaped_delimiter(): void
+    {
+        $this->expectException(InvalidConfigurationException::class);
+
+        $this->load([
+            'filters' => [
+                'user_agent' => ['foo#bar'],
+            ],
+        ]);
+    }
+
     #[Test]
     public function it_tags_the_cached_contexts_as_resettable(): void
     {

tests/Unit/EventSubscriber/FilterConfiguredUserAgentsSubscriberTest.php

@@ -39,4 +39,58 @@ public function it_does_not_stop_when_user_agent_does_not_match(): void
 
         self::assertFalse($event->isPropagationStopped());
     }
+
+    #[Test]
+    public function it_matches_case_insensitively(): void
+    {
+        $metaEvent = new Event(Event::EVENT_VIEW_CONTENT);
+        $metaEvent->userData->clientUserAgent = 'I_AM_A_BOT/1.0';
+
+        $event = new ConversionsApiEventRaised($metaEvent);
+        $subscriber = new FilterConfiguredUserAgentsSubscriber(['i_am_a_bot']);
+        $subscriber->filter($event);
+
+        self::assertTrue($event->isPropagationStopped());
+    }
+
+    #[Test]
+    public function it_does_not_stop_without_a_user_agent(): void
+    {
+        $event = new ConversionsApiEventRaised(new Event(Event::EVENT_VIEW_CONTENT));
+        $subscriber = new FilterConfiguredUserAgentsSubscriber(['i_am_a_bot']);
+        $subscriber->filter($event);
+
+        self::assertFalse($event->isPropagationStopped());
+    }
+
+    #[Test]
+    public function it_does_not_stop_when_no_user_agents_are_configured(): void
+    {
+        $metaEvent = new Event(Event::EVENT_VIEW_CONTENT);
+        $metaEvent->userData->clientUserAgent = 'i_am_a_bot';
+
+        $event = new ConversionsApiEventRaised($metaEvent);
+        $subscriber = new FilterConfiguredUserAgentsSubscriber([]);
+        $subscriber->filter($event);
+
+        self::assertFalse($event->isPropagationStopped());
+    }
+
+    #[Test]
+    public function it_throws_when_the_fragments_do_not_compile(): void
+    {
+        $this->expectException(\InvalidArgumentException::class);
+
+        // An unescaped delimiter turns the rest of the fragment into modifiers. Previously preg_match() returned
+        // false here and the filter silently stopped matching anything
+        new FilterConfiguredUserAgentsSubscriber(['foo#bar']);
+    }
+
+    #[Test]
+    public function it_throws_for_an_invalid_fragment(): void
+    {
+        $this->expectException(\InvalidArgumentException::class);
+
+        new FilterConfiguredUserAgentsSubscriber(['(unbalanced']);
+    }
 }