[CDRIVER-1111] Provide error feedback for invalid newObj arg to update/replace functions Created: 11/Feb/16  Updated: 30/Jan/17  Resolved: 30/Jan/17

Status: Closed
Project: C Driver
Component/s: Bulk API
Affects Version/s: 1.3.3
Fix Version/s: 1.6.0

Type: New Feature Priority: Major - P3
Reporter: Jeremy Mikola Assignee: A. Jesse Jiryu Davis
Resolution: Duplicate Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
duplicates CDRIVER-1341 Driver should validate BSON documents... Closed
Related
related to PHPC-579 Throw exception for invalid BulkWrite... Closed
is related to PHPC-550 Always encode ODS field when serializ... Closed

 Description   

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.



 Comments   
Comment by A. Jesse Jiryu Davis [ 12/Feb/16 ]

Yeah, PyMongo raises an exception directly from BulkOperation.replace_one(replacement) if the "replacement" document has any fields that contain $-signs. We can't communicate errors from mongoc_bulk_operation_replace_one, though.

It might be appropriate in a 1.x release to set an internal mongoc_bulk_operation_t.error if we get an invalid input. mongoc_bulk_operation_execute to fails with a useful error about the first invalid operation added to the bulk op.

Generated at Wed Feb 07 21:11:36 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.