[CDRIVER-1932] mongoc_collection_update() omits validation entirely for update documents Created: 21/Nov/16  Updated: 28/Jan/17  Resolved: 28/Jan/17

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

Type: Bug Priority: Minor - P4
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
is related to PHPC-712 Driver should validate BSON documents... Closed
is related to CDRIVER-1341 Driver should validate BSON documents... Closed

 Description   

While looking into PHPC-712, I came across a peculiarity with mongoc_collection_update(), which apparently skips BSON validation entirely for update documents:

if (!((uint32_t)flags & MONGOC_UPDATE_NO_VALIDATE) &&
    bson_iter_init (&iter, update) &&
    bson_iter_next (&iter) &&
    (bson_iter_key (&iter) [0] != '$') &&
    !bson_validate (update, (bson_validate_flags_t)vflags, &err_offset)) {
   bson_set_error (error,
                   MONGOC_ERROR_BSON,
                   MONGOC_ERROR_BSON_INVALID,
                   "update document is corrupt or contains "
                   "invalid keys including $ or .");
   return false;
} else {
   flags = (uint32_t)flags & ~MONGOC_UPDATE_NO_VALIDATE;
}

Validation flags defined earlier are:

int vflags = (BSON_VALIDATE_UTF8 | BSON_VALIDATE_UTF8_ALLOW_NULL
            | BSON_VALIDATE_DOLLAR_KEYS | BSON_VALIDATE_DOT_KEYS);

I understand that BSON_VALIDATE_DOLLAR_KEYS and BSON_VALIDATE_DOT_KEYS should not apply to update arguments with atomic modifiers, but UTF-8 validation still seems relevant. Should libmongoc instead construct vflags based on whether the argument has atomic modifiers or is a replacement document?

Two other observations:

  • The flags assignment in the else condition does not appear to affect the function in any way. The variable is only checked again when building the update options (i.e. "upsert" and "multi"). Is there any reason for this variable, or should the function instead simply read the mongoc_update_flags_t uflags argument?
  • MONGOC_UPDATE_NO_VALIDATE is used no where else in libmongoc; however, it is part of the public API in mongoc-flags.h. This contrasts with the MONGOC_INSERT_NO_VALIDATE, which is at least used internally for legacy index creation.


 Comments   
Comment by A. Jesse Jiryu Davis [ 28/Jan/17 ]

The CDRIVER-1341 work fixed the substantive problem, which is that if the first key starts with "$" all other validation is skipped. Now, if the first key starts with "$" we validate that all keys are valid UTF-8, non-empty, and start with "$". Otherwise we validate the document the same as an insert: no "$" or "." in keys, all valid non-empty UTF-8.

I've deleted the useless assignment to "flags".

Yes, MONGOC_UPDATE_NO_VALIDATE is a public API that the driver doesn't use internally, whereas MONGOC_INSERT_NO_VALIDATE is a public API that the driver also uses internally. This seems to be by design.

Comment by Githook User [ 28/Jan/17 ]

Author:

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

Message: CDRIVER-1932 mongoc_collection_update dead code
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/41a08387cffbf80f43e61aa1bc869247d1039021

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