CDRIVER-3895: Remove dots/dollars validation for insert/replace docs (#801)

* CDRIVER-3895: Update CRUD, transactions, and unified spec tests

Also replaces CRUD legacy/v2 tests with unified equivalents for CDRIVER-3960.

Synced with mongodb/specifications@a124e21

* Skip unified CRUD tests related to CDRIVER-3630

* Ensure upsertedIds is always present in BulkWriteResult

* Allow array or document for updateOne/Many update arg

* Remove dot/dollar restrictions for insert/replace validation

This adds top-level key validation to _mongoc_validate_replace, similar to what existed in _mongoc_validate_update.

* Remove redundant replace doc validation for legacy updates

Replacement docs will already be validated by _mongoc_validate_replace before this function is reached.

This adds logic to ensure "q" and "u" documents are present, similar to what existed in _mongoc_write_command_delete_legacy.

* Test success and failure for legacy update code path

The failure code path is testing that arguments are still validated by mongoc-util.c, despite replacement validation being removed in a previous commit.

* Revert "Skip tests with dots and dollars in field names on 5.0+ (#795)"

This reverts commit bd014ee.

* Revise bulk and collection tests for allowing dots/dollars

Empty keys can be used as a reliable BSON validation error, since insert, replace, and update all specify BSON_VALIDATE_EMPTY_KEYS in their vflags. Error message expectations are adjusted accordingly.

An _id document with a dollar-prefixed key is also used as a reliable server-side validation error.

* Allow methods in bulk_op_append to fail so expectError can be evaluated

* Skip unified tests due to schema version and outstanding issues

Allow entire files to be skipped to preempt schema version errors.

* Ensure WC is set on database and collection entities

Co-authored-by: Kevin Albertson <[email protected]>

Author: jmikola

src/libmongoc/src/mongoc/mongoc-util.c

@@ -28,13 +28,11 @@
 
 const bson_validate_flags_t _mongoc_default_insert_vflags =
    BSON_VALIDATE_UTF8 | BSON_VALIDATE_UTF8_ALLOW_NULL |
-   BSON_VALIDATE_EMPTY_KEYS | BSON_VALIDATE_DOT_KEYS |
-   BSON_VALIDATE_DOLLAR_KEYS;
+   BSON_VALIDATE_EMPTY_KEYS;
 
 const bson_validate_flags_t _mongoc_default_replace_vflags =
    BSON_VALIDATE_UTF8 | BSON_VALIDATE_UTF8_ALLOW_NULL |
-   BSON_VALIDATE_EMPTY_KEYS | BSON_VALIDATE_DOT_KEYS |
-   BSON_VALIDATE_DOLLAR_KEYS;
+   BSON_VALIDATE_EMPTY_KEYS;
 
 const bson_validate_flags_t _mongoc_default_update_vflags =
    BSON_VALIDATE_UTF8 | BSON_VALIDATE_UTF8_ALLOW_NULL |
@@ -324,6 +322,8 @@ _mongoc_validate_replace (const bson_t *doc,
                           bson_error_t *error)
 {
    bson_error_t validate_err;
+   bson_iter_t iter;
+   const char *key;
 
    if (vflags == BSON_VALIDATE_NONE) {
       return true;
@@ -338,6 +338,27 @@ _mongoc_validate_replace (const bson_t *doc,
       return false;
    }
 
+   if (!bson_iter_init (&iter, doc)) {
+      bson_set_error (error,
+                      MONGOC_ERROR_BSON,
+                      MONGOC_ERROR_BSON_INVALID,
+                      "replace document is corrupt");
+      return false;
+   }
+
+   while (bson_iter_next (&iter)) {
+      key = bson_iter_key (&iter);
+      if (key[0] == '$') {
+         bson_set_error (error,
+                         MONGOC_ERROR_COMMAND,
+                         MONGOC_ERROR_COMMAND_INVALID_ARG,
+                         "Invalid key '%s': replace prohibits $ operators",
+                         key);
+
+         return false;
+      }
+   }
+
    return true;
 }
 

src/libmongoc/src/mongoc/mongoc-write-command-legacy.c

@@ -380,16 +380,13 @@ _mongoc_write_command_update_legacy (mongoc_write_command_t *command,
    int32_t max_bson_obj_size;
    mongoc_rpc_t rpc;
    uint32_t request_id = 0;
-   bson_iter_t subiter, subsubiter;
-   bson_t doc;
+   bson_iter_t subiter;
    bson_t update, selector;
    const uint8_t *data = NULL;
    uint32_t len = 0;
-   size_t err_offset;
    bool val = false;
    char *ns;
-   int vflags = (BSON_VALIDATE_UTF8 | BSON_VALIDATE_UTF8_ALLOW_NULL |
-                 BSON_VALIDATE_DOLLAR_KEYS | BSON_VALIDATE_DOT_KEYS);
+   bool r;
    bson_reader_t *reader;
    const bson_t *bson;
    bool eof;
@@ -406,45 +403,19 @@ _mongoc_write_command_update_legacy (mongoc_write_command_t *command,
 
    max_bson_obj_size = mongoc_server_stream_max_bson_obj_size (server_stream);
 
-   reader =
-      bson_reader_new_from_data (command->payload.data, command->payload.len);
-   while ((bson = bson_reader_read (reader, &eof))) {
-      if (bson_iter_init (&subiter, bson) && bson_iter_find (&subiter, "u") &&
-          BSON_ITER_HOLDS_DOCUMENT (&subiter)) {
-         bson_iter_document (&subiter, &len, &data);
-         BSON_ASSERT (bson_init_static (&doc, data, len));
-
-         if (bson_iter_init (&subsubiter, &doc) &&
-             bson_iter_next (&subsubiter) &&
-             (bson_iter_key (&subsubiter)[0] != '$') &&
-             !bson_validate (
-                &doc, (bson_validate_flags_t) vflags, &err_offset)) {
-            result->failed = true;
-            bson_set_error (error,
-                            MONGOC_ERROR_BSON,
-                            MONGOC_ERROR_BSON_INVALID,
-                            "update document is corrupt or contains "
-                            "invalid keys including $ or .");
-            bson_reader_destroy (reader);
-            EXIT;
-         }
-      } else {
-         result->failed = true;
-         bson_set_error (error,
-                         MONGOC_ERROR_BSON,
-                         MONGOC_ERROR_BSON_INVALID,
-                         "updates is malformed.");
-         bson_reader_destroy (reader);
-         EXIT;
-      }
-   }
-
    ns = bson_strdup_printf ("%s.%s", database, collection);
 
-   bson_reader_destroy (reader);
    reader =
       bson_reader_new_from_data (command->payload.data, command->payload.len);
    while ((bson = bson_reader_read (reader, &eof))) {
+      /* ensure the document has "q" and "u" document fields in that order */
+      r = (bson_iter_init (&subiter, bson) && bson_iter_find (&subiter, "q") &&
+           BSON_ITER_HOLDS_DOCUMENT (&subiter) &&
+           bson_iter_find (&subiter, "u") &&
+           BSON_ITER_HOLDS_DOCUMENT (&subiter));
+
+      BSON_ASSERT (r);
+
       request_id = ++client->cluster.request_id;
 
       rpc.header.msg_len = 0;
@@ -459,6 +430,10 @@ _mongoc_write_command_update_legacy (mongoc_write_command_t *command,
       while (bson_iter_next (&subiter)) {
          if (strcmp (bson_iter_key (&subiter), "u") == 0) {
             bson_iter_document (&subiter, &len, &data);
+
+            BSON_ASSERT (data);
+            BSON_ASSERT (len >= 5);
+
             if (len > max_bson_obj_size) {
                _mongoc_write_command_too_large_error (
                   error, 0, len, max_bson_obj_size);
@@ -472,6 +447,10 @@ _mongoc_write_command_update_legacy (mongoc_write_command_t *command,
             BSON_ASSERT (bson_init_static (&update, data, len));
          } else if (strcmp (bson_iter_key (&subiter), "q") == 0) {
             bson_iter_document (&subiter, &len, &data);
+
+            BSON_ASSERT (data);
+            BSON_ASSERT (len >= 5);
+
             if (len > max_bson_obj_size) {
                _mongoc_write_command_too_large_error (
                   error, 0, len, max_bson_obj_size);

src/libmongoc/tests/bsonutil/bson-parser.c

@@ -27,6 +27,7 @@ typedef enum {
    BSON_PARSER_BOOL,
    BSON_PARSER_DOC,
    BSON_PARSER_ARRAY,
+   BSON_PARSER_ARRAY_OR_DOC,
    BSON_PARSER_ANY,
    BSON_PARSER_WRITE_CONCERN,
    BSON_PARSER_READ_CONCERN,
@@ -62,6 +63,8 @@ parser_type_to_string (bson_parser_type_t ptype)
       return "DOC";
    case BSON_PARSER_ARRAY:
       return "ARRAY";
+   case BSON_PARSER_ARRAY_OR_DOC:
+      return "ARRAY or DOC";
    case BSON_PARSER_ANY:
       return "ANY";
    case BSON_PARSER_WRITE_CONCERN:
@@ -240,7 +243,8 @@ bson_parser_entry_destroy (bson_parser_entry_t *entry, bool with_parsed_fields)
 {
    if (with_parsed_fields) {
       if (entry->ptype == BSON_PARSER_DOC ||
-          entry->ptype == BSON_PARSER_ARRAY) {
+          entry->ptype == BSON_PARSER_ARRAY ||
+          entry->ptype == BSON_PARSER_ARRAY_OR_DOC) {
          bson_t **out;
 
          out = (bson_t **) entry->out;
@@ -399,6 +403,23 @@ bson_parser_array_optional (bson_parser_t *parser,
    bson_parser_add_entry (parser, key, (void *) out, BSON_PARSER_ARRAY, true);
 }
 
+void
+bson_parser_array_or_doc (bson_parser_t *parser, const char *key, bson_t **out)
+{
+   *out = NULL;
+   bson_parser_add_entry (
+      parser, key, (void *) out, BSON_PARSER_ARRAY_OR_DOC, false);
+}
+void
+bson_parser_array_or_doc_optional (bson_parser_t *parser,
+                                   const char *key,
+                                   bson_t **out)
+{
+   *out = NULL;
+   bson_parser_add_entry (
+      parser, key, (void *) out, BSON_PARSER_ARRAY_OR_DOC, true);
+}
+
 void
 bson_parser_bool (bson_parser_t *parser, const char *key, bool **out)
 {
@@ -579,6 +600,19 @@ entry_marshal (bson_parser_entry_t *entry,
       *out = bson_copy (&tmp);
    }
 
+   else if (ptype == BSON_PARSER_ARRAY_OR_DOC) {
+      bson_t tmp;
+      bson_t **out = (bson_t **) entry->out;
+
+      if (btype != BSON_TYPE_ARRAY && btype != BSON_TYPE_DOCUMENT) {
+         marshal_error (key, btype, ptype, error);
+         goto done;
+      }
+
+      bson_iter_bson (iter, &tmp);
+      *out = bson_copy (&tmp);
+   }
+
    else if (ptype == BSON_PARSER_ANY) {
       bson_val_t **out = (bson_val_t **) entry->out;
 

src/libmongoc/tests/bsonutil/bson-parser.h

@@ -81,6 +81,13 @@ bson_parser_array (bson_parser_t *bp, const char *key, bson_t **out);
 void
 bson_parser_array_optional (bson_parser_t *bp, const char *key, bson_t **out);
 
+void
+bson_parser_array_or_doc (bson_parser_t *bp, const char *key, bson_t **out);
+void
+bson_parser_array_or_doc_optional (bson_parser_t *bp,
+                                   const char *key,
+                                   bson_t **out);
+
 void
 bson_parser_bool (bson_parser_t *bp, const char *key, bool **out);
 void