Repository navigation
fix(adapters): refuse a regex the engine cannot compile as a query error - #1030
Merged
Merged
Conversation
A regex filter with a pattern such as `((` or `[` failed with an unmapped PDOException on MariaDB (1139), MySQL (3685 to 3700, 3887) and PostgreSQL (2201B), and a raw driver error on MongoDB (51091), which consumers report as a server error. SQLite, Memory and Redis match with PCRE and read such a pattern as matching nothing. 7.4.2 maps none of them either. Each engine's invalid pattern errors map to Exception\Query, and the PCRE adapters refuse a pattern that does not compile with the same error. The pattern is not checked up front: each engine has its own dialect (PCRE2, ICU, ARE), so a pattern valid on one may not compile on another. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
testCountReportsDriverErrors pinned the raw driver exception to prove count() reports the error rather than answering 0. It now arrives as Exception\Query with the driver error as its previous, which still proves that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 13 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tern # Conflicts: # src/Database/Adapter/MariaDB.php # src/Database/Adapter/MySQL.php # tests/unit/EngineErrorMappingTest.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Staging bug C:
regex("s", ["(("])gave a 500,PDOException 3691 Mismatched parenthesis in regular expression.On origin/main a regex pattern that the engine cannot compile fails as follows:
PDOException1139.PDOException3685–3700 or 3887.PDOException2201B.Each engine's invalid-pattern errors now map to
Exception\Query("Invalid regular expression"). The PCRE adapters refuse a pattern that does not compile with the same error. The pattern is not pre-validated in PHP, because each engine has its own dialect (PCRE2, ICU, ARE), so a pattern valid on one engine may not compile on another.This is ported from the unpushed
qfix-overflowwork (83aae640d) without the spatial error mapping, which is in the spatial PR. The E2E test was extended to covercount()and more patterns.Regression tests
DocumentTests::testInvalidRegexIsAQueryError:((,[,a{2,1},),*aonfind()andcount(); a valid^tarstill matchesPDOException 3691 Mismatched parenthesis in regular expressionPDOException 1139 Regex error 'missing closing parenthesis at offset 2'PDOException 2201B invalid regular expression: parentheses () not balancedUtopia\Mongo\Exception E51091 Regular expression is invalidUnit\EngineErrorMappingTest(regex errors per engine),Unit\MemoryRegexTest,Unit\SQLiteRegexTestMongoReadFilterTests::testCountReportsDriverErrorspinned the raw driver exception to show thatcount()reports the error instead of answering 0. It now expectsException\Querywith the driver error as its previous exception, which still shows that.Run locally against Docker: the full adapter suites for MySQL, MariaDB, Postgres, SQLite, MongoDB, Memory and Redis pass, as do the full unit suite, Pint and PHPStan.
API repro (Appwrite)
GET /v1/tablesdb/{db}/tables/{t}/rows?queries[]={"method":"regex","attribute":"s","values":["(("]}→400 general_query_invalid, messageInvalid regular expression(was 500). The same holds withtotal=true(the count path).🤖 Generated with Claude Code