Repository navigation
Conversation
…alidation filter() keeps underscores, so with validation skipped a document key such as `_uid` or `_u!id` reached the column the storage keeps for the document's own id and rewrote it on create or update, `_id` chose the sequence, and a bulk update with `_permissions` replaced the document's permissions. An upsert also let `_ten!ant` move a new row to another tenant, since the filtered key overwrote the tenant the write hooks set. Every SQL and Memory write now takes the document's attributes through writableAttributes(), which drops a key whose filtered name is a reserved storage column before operators are extracted, matching how Mongo drops storage keys rather than refusing the write. An upsert splits each change's writable attributes into operators and updates once and hands the result to executeUpsertBatch, rather than extracting them up to three times per change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collection, relationship and analyze DDL joined several statements with ';' into one prepare, which only ran on MySQL and MariaDB because multi-statement execution was left enabled on every connection, and on PostgreSQL because emulated prepares send a simple query. Keeping multi-statement execution on widens what an injected fragment can do. executeStatements() now runs each statement through its own prepare, so transforms, events and the profiler still see every statement, and a later statement's error fails the call without MariaDB having to walk its rowsets. No transaction is opened, since createCollection may run inside a caller's transaction. The MySQL-family e2e connections turn multi-statement execution off so any regression fails there. PostgreSQL ran a multi-statement simple query as one implicit transaction, so DDL loses that atomicity here. createCollection marks a collection created once its table exists and discards both tables on any failure in its internal phase (permissions table, internal indexes); only a duplicate declared index keeps them. deleteCollection converges on retry: when the main table is already gone it reports NotFound after still dropping the permissions table. Relationship create and delete converge through the executor added in a following commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without PCRE_DOLLAR_ENDONLY, `$` also matches before a final "\n", so a join alias, aggregate alias, joined column, UUIDv7 sequence, SQL hook identifier, collation or integer string carrying one passed its allowlist and reached the query. The D modifier anchors `$` to the end of the subject, as Rounding and ObjectPath already do. BigInt::integralValue and the Postgres hex-WKB check get it too: "5.0\n" is not a whole decimal and "ab\n" is not hex. Parsers whose input cannot carry a trailing newline (collapsed or trimmed whitespace, first error line) or where D would loosen a deny-list are left unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
setTimeout() and clearTimeout() ignored their event, so a timeout meant for one event (as Pool::syncTimeouts() forwards each one) bounded every find, count and sum, and clearing any event dropped the global timeout too. The adapter now keeps per-event state through the Timeout trait like the SQL adapters and Pool, find/count/sum read DocumentFind/DocumentCount/DocumentSum, and a non-positive timeout clears that event as Pool does. count() also loses option assignments that getTransactionOptions() always overwrote. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With DDL run one statement per prepare, PostgreSQL no longer applies a two-way relationship's columns atomically. The database recovers from a partial create by catching DuplicateException and from a partial delete by catching NotFoundException, but the SQL adapters threw the raw PDOException, so neither recovery ever ran, and a retry stopped at the first column a previous attempt had already handled. Relationship create and delete now map engine errors and carry on past a statement the schema already reflects, then report that refusal, so a retry applies the rest and the database writes or removes the metadata. deleteCollection shares the same path. Inside a transaction the first refusal is thrown at once, as PostgreSQL rejects the rest of an aborted transaction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A pooled connection keeps the per-event timeouts its last holder set, and an adapter prefers a per-event entry over the global one. The global-only shortcut set the global timeout without clearing them, so a handle asking for 1000ms everywhere ran finds under another request's 50ms. Each sync now clears every event the pool holds no entry for, which also avoids a full clear and its extra SET round trip on MariaDB when only the global timeout is set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The adapter drops a key whose filtered name is a reserved column, but the validator checked the raw key, so `_u!id` or `_ten.ant` passed validation and, on schemaless adapters, was accepted and then silently dropped on write. Filtering through the same static helper the adapter uses keeps the two checks in lockstep; keys Validator\Key accepts never start with an underscore, so legitimate attributes are unaffected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
increaseDocumentAttribute filtered the attribute name itself, so an increment of a reserved column such as `_tenant` or `_ten.ant` rewrote the document's own tenant, sequence or timestamps. It now resolves the column through writableColumn() like the SQL and Mongo adapters, so the increment is refused with NotFound before the document is touched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The adapters now send one statement per call, so a connection with multi-statements enabled (the MySQL default) only widens what an injected semicolon can do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A converging DDL run takes a refused statement as one an earlier failed attempt already applied, runs the rest, then rethrows so the caller knows the schema held part of it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review: 2
Please check the settings in the CodeRabbit UI or the
 |
2 tasks done
The pool now clears each per-event timeout a borrowed connection kept, not only the global one, so the read adapter sees one clear per event. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Follow-ups from #1054 that don't depend on a new utopia-php/query release. Stacked on
fix/injection-hardening.Changes
_uid,_idor another storage column could overwrite it on create, update or upsert. SQL and Memory write paths now drop those keys, like Mongo'swithoutStorageKeys(). The Structure validator filters a key before its reserved check, so_u!idis refused too. Memory refusesincreaseDocumentAttribute()on a storage column.;, so connections no longer need multi-statement execution. The README recommends turning it off (PDO::MYSQL_ATTR_MULTI_STATEMENTS => false).createCollectionnow drops both tables if an internal step fails. A duplicate declared index keeps them.D, so a trailing newline no longer matches$.maxTimeMSis scoped per event.find,countandsumread the timeout set for their own event, falling back to the global one.Notes
SQL::executeUpsertBatch()gained an$extractionsparameter. Subclasses that override it need updating.LIKE ... ESCAPE, andcontainsAll()literal escaping (its own PR, since it changes behaviour).Every fix has a regression test.
🤖 Generated with Claude Code