[CDRIVER-2017] Inconsistent error reporting for insert, update, and replace BSON validation Created: 02/Feb/17  Updated: 18/Feb/17  Resolved: 18/Feb/17

Status: Closed
Project: C Driver
Component/s: libmongoc
Affects Version/s: 1.6.0
Fix Version/s: 1.6.1

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

Issue Links:
Related
related to CDRIVER-2018 Introduce bulk write options to disab... Closed
is related to PHPC-712 Driver should validate BSON documents... Closed
is related to CDRIVER-1341 Driver should validate BSON documents... Closed

 Description   

The functions that append a write statement to the bulk (e.g. mongoc_bulk_operation_insert(), mongoc_bulk_operation_replace_one()) inconsistently handle error reporting.

If we look at mongoc_bulk_operation_insert(), _mongoc_validate_new_document() is only called if a document is being appended to a newly initialized insert command, and even then the validation function's boolean return value is ignored and a potentially invalid document is still added to the command. Validation is never performed at all before appending to an existing insert command (i.e. _mongoc_write_command_insert_append()).

mongoc_bulk_operation_replace_one() delegates to _mongoc_bulk_operation_replace_one_with_opts(), which starts with the following logic:

if (bulk->result.error.domain) {
   /* already failed e.g. a bad call to mongoc_bulk_operation_insert */
   RETURN (true);
}

This logic prevents a later _mongoc_validate_replace() call from overwriting a previous error, which is good, but it effectively makes mongoc_bulk_operation_replace_one() a NOP with no way to indicate that to the user. It would be better to return false if there is a pre-existing error. Also, the insert and update paths are missing equivalent logic (i.e. NOP on a pre-existing error) entirely, which means they may overwrite a previous error.



 Comments   
Comment by Githook User [ 18/Feb/17 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-2017 fix bulk-op validation logic
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/4a0945f722d5c58ae5b5e25ac3d0860202729c3b

Comment by Jeremy Mikola [ 02/Feb/17 ]

I realize that any new API would need to wait until 1.7.0. PHP may be able to work around the BulkWrite::insert() issue of a deferred exception by validating the document ourselves. This duplicates validation, but would ensure that we can catch the error before libmongoc.

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