SERVER-57382 Add _id $-prefix validation to storage validation

4 files changed, 44 insertions, 37 deletions

diff --git a/jstests/core/field_name_validation.js b/jstests/core/field_name_validation.js
index 936da4147de..1dc9219f74e 100644
--- a/jstests/core/field_name_validation.js
+++ b/jstests/core/field_name_validation.js
@@ -109,6 +109,15 @@ assert.eq(1, coll.find({a: {b: 2}}).itcount());
 assert.commandWorked(coll.update({"a.b": 2}, {"a.b": 3}));
 assert.eq(0, coll.find({"a.b": 3}).itcount());
 
+// Upserting _id fields containing $-prefixed fields is not allowed.
+assert.writeErrorWithCode(coll.update({"a.b": 1}, {_id: {$invalid: 1}}, {upsert: true}),
+                          ErrorCodes.DollarPrefixedFieldName);
+assert.writeErrorWithCode(coll.update({"a.b": 1}, {$set: {_id: {$invalid: 1}}}, {upsert: true}),
+                          ErrorCodes.DollarPrefixedFieldName);
+assert.writeErrorWithCode(
+    coll.update({"a.b": 1}, {$setOnInsert: {_id: {$invalid: 1}}}, {upsert: true}),
+    ErrorCodes.DollarPrefixedFieldName);
+
 if (isDotsAndDollarsEnabled) {
     // Replacement-style updates can contain nested $-prefixed fields.
     assert.commandWorked(coll.update({"a.b": 1}, {a: {$c: 1}}));
diff --git a/src/mongo/db/ops/insert.cpp b/src/mongo/db/ops/insert.cpp
index 1a7d6027c0a..c508f95d0c6 100644
--- a/src/mongo/db/ops/insert.cpp
+++ b/src/mongo/db/ops/insert.cpp
@@ -38,6 +38,7 @@
 #include "mongo/db/query/dbref.h"
 #include "mongo/db/query/query_feature_flags_gen.h"
 #include "mongo/db/repl/replication_coordinator.h"
+#include "mongo/db/update/storage_validation.h"
 #include "mongo/db/vector_clock_mutable.h"
 #include "mongo/db/views/durable_view_catalog.h"
 #include "mongo/util/fail_point.h"
@@ -134,33 +135,9 @@ StatusWith<BSONObj> fixDocumentForInsert(OperationContext* opCtx,
                 // also, disallow undefined and arrays
                 // Make sure _id isn't duplicated (SERVER-19361).
                 if (fieldName == "_id") {
-                    if (e.type() == RegEx) {
-                        return StatusWith<BSONObj>(ErrorCodes::BadValue,
-                                                   "can't use a regex for _id");
-                    }
-                    if (e.type() == Undefined) {
-                        return StatusWith<BSONObj>(ErrorCodes::BadValue,
-                                                   "can't use a undefined for _id");
-                    }
-                    if (e.type() == Array) {
-                        return StatusWith<BSONObj>(ErrorCodes::BadValue,
-                                                   "can't use an array for _id");
-                    }
-                    if (e.type() == Object) {
-                        BSONObj o = e.Obj();
-                        Status s = o.storageValidEmbedded();
-                        if (!s.isOK()) {
-                            if (feature_flags::gFeatureFlagDotsAndDollars.isEnabledAndIgnoreFCV() &&
-                                s.code() == ErrorCodes::DollarPrefixedFieldName) {
-                                return StatusWith<BSONObj>(
-                                    s.code(),
-                                    str::stream()
-                                        << "_id fields may not contain '$'-prefixed fields: "
-                                        << s.reason());
-                            } else {
-                                return StatusWith<BSONObj>(s);
-                            }
-                        }
+                    auto status = storage_validation::storageValidIdField(e);
+                    if (!status.isOK()) {
+                        return StatusWith<BSONObj>(status);
                     }
                     if (hadId) {
                         return StatusWith<BSONObj>(
diff --git a/src/mongo/db/update/storage_validation.cpp b/src/mongo/db/update/storage_validation.cpp
index cac577f414c..a314e8c3065 100644
--- a/src/mongo/db/update/storage_validation.cpp
+++ b/src/mongo/db/update/storage_validation.cpp
@@ -125,6 +125,30 @@ void validateDollarPrefixElement(mutablebson::ConstElement elem) {
 }
 }  // namespace
 
+Status storageValidIdField(const mongo::BSONElement& element) {
+    switch (element.type()) {
+        case BSONType::RegEx:
+        case BSONType::Array:
+        case BSONType::Undefined:
+            return Status(ErrorCodes::InvalidIdField,
+                          str::stream()
+                              << "The '_id' value cannot be of type " << typeName(element.type()));
+        case BSONType::Object: {
+            auto status = element.Obj().storageValidEmbedded();
+            if (!status.isOK() && status.code() == ErrorCodes::DollarPrefixedFieldName &&
+                feature_flags::gFeatureFlagDotsAndDollars.isEnabledAndIgnoreFCV()) {
+                return Status(status.code(),
+                              str::stream() << "_id fields may not contain '$'-prefixed fields: "
+                                            << status.reason());
+            }
+            return status;
+        }
+        default:
+            break;
+    }
+    return Status::OK();
+}
+
 void storageValid(const mutablebson::Document& doc,
                   const bool allowTopLevelDollarPrefixes,
                   const bool shouldValidate,
@@ -132,16 +156,7 @@ void storageValid(const mutablebson::Document& doc,
     auto currElem = doc.root().leftChild();
     while (currElem.ok()) {
         if (currElem.getFieldName() == idFieldName && shouldValidate) {
-            switch (currElem.getType()) {
-                case BSONType::RegEx:
-                case BSONType::Array:
-                case BSONType::Undefined:
-                    uasserted(ErrorCodes::InvalidIdField,
-                              str::stream() << "The '_id' value cannot be of type "
-                                            << typeName(currElem.getType()));
-                default:
-                    break;
-            }
+            uassertStatusOK(storageValidIdField(currElem.getValue()));
         }
 
         // Validate this child element.
diff --git a/src/mongo/db/update/storage_validation.h b/src/mongo/db/update/storage_validation.h
index 9e3ae50ee84..951f5fe7e95 100644
--- a/src/mongo/db/update/storage_validation.h
+++ b/src/mongo/db/update/storage_validation.h
@@ -36,6 +36,12 @@ namespace mongo {
 namespace storage_validation {
 
 /**
+ * Returns a status to indicate whether or not 'element' is a valid _id field for storage in a
+ * collection.
+ */
+Status storageValidIdField(const mongo::BSONElement& element);
+
+/**
  * Validates that the MutableBSON document 'doc' is acceptable for storage in a collection. The
  * check is performed recursively on subdocuments. Uasserts if the validation fails or if the depth
  * exceeds the maximum allowable depth.