Uploaded image for project: 'C Driver'
  1. C Driver
  2. CDRIVER-1111

Provide error feedback for invalid newObj arg to update/replace functions

    • Type: Icon: New Feature New Feature
    • Resolution: Duplicate
    • Priority: Icon: Major - P3 Major - P3
    • 1.6.0
    • Affects Version/s: 1.3.3
    • Component/s: Bulk API
    • None

      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.

            Assignee:
            jesse@mongodb.com A. Jesse Jiryu Davis
            Reporter:
            jmikola@mongodb.com Jeremy Mikola
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: