Discovered while working on an edge case in PHPC-550, where the driver would sometimes inject a regular field name into a atomic modifier object. While that's certainly a bug to fix, it's possible that users may trigger this (from the driver, not our high-level library, where we check for it) by providing their own malformed newObj documents.
If BulkWrite always encodes the ODS field, bsonSerialize() is now unable to return atomic updates. Worse yet, the __pclass field gets added into the document with atomic modifiers, which causes BulkWrite::update() to detect the document as a replacement due to:
while (bson_iter_next (&iter)) { if (!strchr (bson_iter_key (&iter), '$')) { mongoc_bulk_operation_replace_one(intern->bulk, bquery, bupdate, !!(flags & MONGOC_UPDATE_UPSERT)); replaced = 1; break; } }In turn, mongoc_bulk_operation_replace_one() actually fails silently (apart from a MONGOC_WARNING() which only surfaces in PHP with mongodb.debug enabled) and refuses to add the update to its command document. I noticed this when executing the BulkWrite failed due to being empty. If there were other writes scheduled, that update would simply be ignored, which would be much harder to catch. Independent of this ticket, we should certainly add some better error reporting around this, even if it means validating the document on our own before libmongoc does so again.
MONGOC_WARNING() does not seem suitable for relaying these errors (at least if a wrapping driver wishes to capture them and throw an exception). Since the functions are void and take no output parameters, there likely isn't a good way to introduce error reporting without an ABI change.
As a temporary work-around, I suppose the PHP driver can manually validate the BSON itself, as is done in the libmongoc functions before the warning might be logged.
- duplicates
-
CDRIVER-1341 Driver should validate BSON documents before insert and update
- Closed
- is related to
-
PHPC-550 Always encode ODS field when serializing Persistable documents
- Closed
- related to
-
PHPC-579 Throw exception for invalid BulkWrite::update() newObj argument
- Closed