Skip to content

fix(flags): match regex filters like the flags service - #261

Merged
marandaneto merged 1 commit into
PostHog:mainfrom
quotentiroler:fix/regex-filter-patterns
Oct 16, 2026
Merged

marandaneto merged 1 commit into
PostHog:mainfrom
quotentiroler:fix/regex-filter-patterns

Conversation

@quotentiroler

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Local evaluation of regex and not_regex filters gave different answers from the flags service. The SDK treated the filter value as a PHP delimited pattern whenever it happened to compile as one, and otherwise wrapped it in / without escaping. As a result:

  • ^https://example\.com/ (any pattern with an unescaped /) failed to compile and always returned false
  • [abc] used the brackets as delimiters, so the character class was lost
  • /abc/i was matched case-insensitively instead of literally
  • an empty pattern returned false instead of matching everything
  • the property value was passed to preg_match as-is: null short-circuited to false, true became "1", and an array property threw a TypeError out of getFeatureFlag because Client only catches Exception

The flags service (rust/feature-flags/src/properties/property_matching.rs) compiles the filter value as the pattern and converts the property value to a string with the same rules used for the other string operators. The regex requirement in PostHog/sdk-specs local-feature-flag-evaluator says the same. This change makes the PHP SDK do that:

  • the pattern and property are converted to strings with valueToString
  • the pattern is wrapped in /.../u, with any slash that isn't already escaped escaped
  • invalid patterns still return false for both operators
  • a preg_match execution failure is now inconclusive, so not_regex cannot match on an error

testMatchPropertyWithNones expected regex "null" against a null property to be false. The flags service compares against the string "null", so it is now true.

💚 How did you test it?

  • Added a data-provider test covering slashes, escaped slashes, bracket classes, slash-wrapped values, empty and invalid patterns, case sensitivity, Unicode, and non-string property values (null, bool, float, array). Every expected value was checked against the flags service matcher, compiled from rust/feature-flags and run on the same inputs.
  • Added a test that a regex execution failure (invalid UTF-8 input) is inconclusive for both operators.
  • Added a Client-level test with only_evaluate_locally for a URL regex flag and an array property; it returned false before this change.
  • Ran the full PHPUnit suite on PHP 8.2, 8.3 and 8.5 (758 tests), plus phpcs, composer api:check, and the sdk_compliance_adapter unit tests.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm change to generate a change intent file

🤖 Agent context

Autonomy: Human-driven (agent-assisted). DRI: @quotentiroler (an outside contributor can't be set as assignee).

  • Found and fixed with Claude Code; I reviewed and tested the change.
  • The divergence came out of running one set of property-matching cases through every server SDK and the flags service's own matcher (compiled from rust/feature-flags). PHP was the only SDK that failed the slash and bracket cases, which the other six SDKs and the service agree on.
  • The changeset was written by hand in the pnpm change format.

@quotentiroler
quotentiroler requested a review from a team as a code owner October 15, 2026 17:22
@marandaneto
marandaneto requested a review from a team October 15, 2026 17:23
@marandaneto

Copy link
Copy Markdown
Member

thanks @quotentiroler

The regex branch treated the filter value as a PHP delimited pattern when
it happened to compile as one, and otherwise wrapped it in "/" without
escaping. Patterns with an unescaped slash (for example
^https://example\.com/) failed to compile and always returned false, a
pattern like [abc] used the brackets as delimiters and lost its character
class, and a value like /abc/i got PHP flags instead of being matched
literally. The flags service and the other SDKs compile the value as-is.

The property value was passed to preg_match without stringification:
null short-circuited to false for both operators, booleans became "1",
and an array property raised a TypeError that escapes getFeatureFlag
because Client only catches Exception.

Now the pattern and the property are stringified with valueToString, the
pattern is wrapped in slashes with any unescaped slash escaped, and it runs
in Unicode mode like the flags service. Invalid patterns still return false
for both operators, and a preg_match execution failure raises
InconclusiveMatchException so not_regex can no longer match on an error.
These follow the regex requirement in PostHog/sdk-specs
local-feature-flag-evaluator and match rust/feature-flags
property_matching.rs.

testMatchPropertyWithNones expected regex "null" against a null property to
be false; the flags service stringifies null to "null", so it is now true.
@marandaneto
marandaneto force-pushed the fix/regex-filter-patterns branch from a63da17 to d136d11 Compare October 16, 2026 18:25
@marandaneto
marandaneto enabled auto-merge (squash) October 16, 2026 18:49
@marandaneto
marandaneto merged commit aa30867 into PostHog:main Oct 16, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants