Uploaded image for project: 'PHP Driver: Library'
  1. PHP Driver: Library
  2. PHPLIB-1129

Replace operations may inadvertently execute pipeline updates

      PHPLIB already implements update and replacement validation, and additionally allows pipelines for update arguments.

      Replace operations should not allow pipelines, but this is inconsistent. ReplaceOne prohibits pipelines but FindOneAndReplace and BulkWrite replaceOne do not (likely an oversight in PHPLIB-418). This potentially allows replace operations to execute an update.

      Additionally, the is_pipeline() utility function has historically returned false for empty pipelines. Technically, empty pipelines are valid NOPs for update and findAndModify commands; however, they are rejected by libmongoc for updates because empty arrays and documents have identical bson_t representations (for BC, libmongoc considers such a bson_t as an empty replacement document). Comprehensive validation for update and replace operations may require us to consider empty arrays as pipelines, so is_pipeline() will need to be enhanced. This is also relevant to PHPLIB-881, which will start using the function to validate aggregation pipelines (empty arrays are completely valid there).

      CRUD spec tests generally do not test invalid types for operation arguments. Since PHP arrays can express both a document or pipeline array, PHPLIB must be sensitive to replacement documents that resemble pipelines.

      Notable edge cases

      MongoDB 5.0+ relaxed restrictions on dots and dollars in field names. This is not problematic for replacement documents since those require special syntax (see: Document replacing updates).

      CDRIVER-4658 discusses an edge case where a replacement document might be serialized as an update pipeline. This is due to internal logic in libmongoc's code for assembling an update command, whereby bson_t structs resembling a pipeline (i.e. numeric keys and document with a dollar-prefixed key) are encoded as a BSON array instead of a document. It's very unlikely for a user to run into that, but it'd be prudent to add test coverage for it so long as libmongoc maintains this behavior.

            Assignee:
            jmikola@mongodb.com Jeremy Mikola
            Reporter:
            jmikola@mongodb.com Jeremy Mikola
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:
              Resolved: