Fix invalid BSON regex flags in DatabasePresenceVerifier::getCount() - #3563
Conversation
GromNaN
left a comment
There was a problem hiding this comment.
The fix is correct. MongoDB\BSON\Regex expects flag characters only (i, m, x, ...) without PCRE delimiters, so replacing '/i' with 'i' is the right change, consistent with what getMultiCount() already does.
Tests: The new testGetCountBuildsRegexWithValidFlags test correctly validates the flags using a lightweight stub, and the assertion would fail with the old code (getFlags() would return '/i'). The existing testUnique and testExists tests cover the end-to-end behavior.
Security review: No issues. preg_quote() properly escapes user input, anchors ^/$ prevent partial matches, and the regex has no quantifiers that could cause ReDoS.
Performance: The invalid / flag was stripped by the BSON encoder before reaching the server (as noted in the PR description), so no query behavior change is expected.
Thanks for spotting this inconsistency and for the thorough PR description explaining the observable impact!
MongoDB\BSON\Regex expects PCRE flag characters only (e.g. "i"), not a delimiter-style string like "/i". getMultiCount() already passed the flags correctly; getCount() did not.
Regression test for the flags fix in the previous commit. Asserts against the raw Regex object passed to where(), since MongoDB's BSON encoder silently strips invalid flag characters before a query ever reaches the server, making the bug unobservable end-to-end.
10c395b to
ea858bc
Compare
DatabasePresenceVerifierDatabasePresenceVerifier::getCount()
Summary
Description
DatabasePresenceVerifier::getCount()was passing'/i'as the flags argument toMongoDB\BSON\Regex, instead of the plain PCRE flag character'i'.Regexflags must only contain flag characters (i,m,x,s, ...), not a delimiter-style string.The sibling method
getMultiCount()already used the correct'i'.This PR has no observable effect against MongoDB's current PHP driver/server: the BSON encoder silently strips invalid flag characters before a query ever reaches the server (verified by inspecting the actual command sent via
CommandSubscriber).Still, constructing an invalid
Regexvalue is incorrect and worth fixing for correctness / forward-compatibility.Test Plans
Added
testGetCountBuildsRegexWithValidFlags()toValidationTest.php.Since the invalid flags can't be observed end-to-end (see above), the test asserts against the raw
Regexobject passed towhere(), using a lightweight stub in place of the query builder/connection.Checklist
Test Results