diff --git a/README.md b/README.md index bca7047..795afd8 100644 --- a/README.md +++ b/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: [] ``` diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index cf2783f..1095a9f 100644 --- a/src/DependencyInjection/Configuration.php +++ b/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() diff --git a/src/EventSubscriber/FilterConfiguredUserAgentsSubscriber.php b/src/EventSubscriber/FilterConfiguredUserAgentsSubscriber.php index ef909aa..6ba48b2 100644 --- a/src/EventSubscriber/FilterConfiguredUserAgentsSubscriber.php +++ b/src/EventSubscriber/FilterConfiguredUserAgentsSubscriber.php @@ -10,10 +10,35 @@ final class FilterConfiguredUserAgentsSubscriber implements EventSubscriberInterface { /** - * @param list $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 $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(); } } diff --git a/tests/Integration/DependencyInjection/SetonoMetaConversionsApiExtensionTest.php b/tests/Integration/DependencyInjection/SetonoMetaConversionsApiExtensionTest.php index fe4399d..068d9d6 100644 --- a/tests/Integration/DependencyInjection/SetonoMetaConversionsApiExtensionTest.php +++ b/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 { diff --git a/tests/Unit/EventSubscriber/FilterConfiguredUserAgentsSubscriberTest.php b/tests/Unit/EventSubscriber/FilterConfiguredUserAgentsSubscriberTest.php index 79f5102..8dfb110 100644 --- a/tests/Unit/EventSubscriber/FilterConfiguredUserAgentsSubscriberTest.php +++ b/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']); + } }