SERVER-49117 Remove storage validation of '$' prefixes in insert and update

7 files changed, 65 insertions, 17 deletions

diff --git a/src/mongo/db/update/object_replace_executor.cpp b/src/mongo/db/update/object_replace_executor.cpp
index 4bd38575dba..92fc0023dcd 100644
--- a/src/mongo/db/update/object_replace_executor.cpp
+++ b/src/mongo/db/update/object_replace_executor.cpp
@@ -72,7 +72,10 @@ ObjectReplaceExecutor::ObjectReplaceExecutor(BSONObj replacement)
 }
 
 UpdateExecutor::ApplyResult ObjectReplaceExecutor::applyReplacementUpdate(
-    ApplyParams applyParams, const BSONObj& replacementDoc, bool replacementDocContainsIdField) {
+    ApplyParams applyParams,
+    const BSONObj& replacementDoc,
+    bool replacementDocContainsIdField,
+    bool allowTopLevelDollarPrefixedFields) {
     auto originalDoc = applyParams.element.getDocument().getObject();
 
     // Check for noop.
@@ -102,7 +105,8 @@ UpdateExecutor::ApplyResult ObjectReplaceExecutor::applyReplacementUpdate(
 
     // Validate for storage.
     if (applyParams.validateForStorage) {
-        storage_validation::storageValid(applyParams.element.getDocument());
+        storage_validation::storageValid(applyParams.element.getDocument(),
+                                         allowTopLevelDollarPrefixedFields);
     }
 
     // Check immutable paths.
diff --git a/src/mongo/db/update/object_replace_executor.h b/src/mongo/db/update/object_replace_executor.h
index 4b7d8d87acb..d965b9a6283 100644
--- a/src/mongo/db/update/object_replace_executor.h
+++ b/src/mongo/db/update/object_replace_executor.h
@@ -51,12 +51,18 @@ public:
      * If 'replacementDocContainsIdField' is false then the _id field from the original document
      * will be preserved.
      *
+     * If 'allowTopLevelDollarPrefixedFields' is true, then when the dots and dollars feature flag
+     * is enabled, top-level dollar-prefixed fields will be permitted in document updates. This is
+     * only set to true in pipeline-style updates, when we can be sure that there are no collisions
+     * between '$'-prefixed fieldnames and update modifiers like $set.
+     *
      * This function will ignore the log mode provided in 'applyParams'. The 'oplogEntry' field
      * of the returned ApplyResult is always empty.
      */
     static ApplyResult applyReplacementUpdate(ApplyParams applyParams,
                                               const BSONObj& replacementDoc,
-                                              bool replacementDocContainsIdField);
+                                              bool replacementDocContainsIdField,
+                                              bool allowTopLevelDollarPrefixedFields = false);
 
     /**
      * Initializes the node with the document to replace with. Any zero-valued timestamps (except
diff --git a/src/mongo/db/update/object_replace_executor_test.cpp b/src/mongo/db/update/object_replace_executor_test.cpp
index 811c8eb0e06..f2b53003666 100644
--- a/src/mongo/db/update/object_replace_executor_test.cpp
+++ b/src/mongo/db/update/object_replace_executor_test.cpp
@@ -35,6 +35,7 @@
 #include "mongo/bson/mutable/mutable_bson_test_utils.h"
 #include "mongo/db/json.h"
 #include "mongo/db/update/update_node_test_fixture.h"
+#include "mongo/idl/server_parameter_test_util.h"
 #include "mongo/unittest/unittest.h"
 
 namespace mongo {
@@ -258,6 +259,8 @@ TEST_F(ObjectReplaceExecutorTest, CanAddImmutableId) {
 }
 
 TEST_F(ObjectReplaceExecutorTest, CannotCreateDollarPrefixedNameWhenValidateForStorageIsTrue) {
+    RAIIServerParameterControllerForTest controller("featureFlagDotsAndDollars", false);
+
     auto obj = fromjson("{a: {b: 1, $bad: 1}}");
     ObjectReplaceExecutor node(obj);
 
diff --git a/src/mongo/db/update/pipeline_executor.cpp b/src/mongo/db/update/pipeline_executor.cpp
index d5d62bcee23..2c1f900190a 100644
--- a/src/mongo/db/update/pipeline_executor.cpp
+++ b/src/mongo/db/update/pipeline_executor.cpp
@@ -36,6 +36,7 @@
 #include "mongo/db/pipeline/document_source_queue.h"
 #include "mongo/db/pipeline/lite_parsed_pipeline.h"
 #include "mongo/db/pipeline/variable_validation.h"
+#include "mongo/db/query/query_feature_flags_gen.h"
 #include "mongo/db/update/document_diff_calculator.h"
 #include "mongo/db/update/object_replace_executor.h"
 #include "mongo/db/update/storage_validation.h"
@@ -101,7 +102,10 @@ UpdateExecutor::ApplyResult PipelineExecutor::applyUpdate(ApplyParams applyParam
     // Replace the pre-image document in applyParams with the post image we got from running the
     // post image.
     auto ret = ObjectReplaceExecutor::applyReplacementUpdate(
-        applyParams, transformedDoc, transformedDocHasIdField);
+        applyParams,
+        transformedDoc,
+        transformedDocHasIdField,
+        feature_flags::gFeatureFlagDotsAndDollars.isEnabledAndIgnoreFCV());
 
     // The oplog entry should not have been populated yet.
     invariant(ret.oplogEntry.isEmpty());
diff --git a/src/mongo/db/update/set_node_test.cpp b/src/mongo/db/update/set_node_test.cpp
index 55a7f54027d..e1b7598e07b 100644
--- a/src/mongo/db/update/set_node_test.cpp
+++ b/src/mongo/db/update/set_node_test.cpp
@@ -36,6 +36,7 @@
 #include "mongo/db/json.h"
 #include "mongo/db/pipeline/expression_context_for_test.h"
 #include "mongo/db/update/update_node_test_fixture.h"
+#include "mongo/idl/server_parameter_test_util.h"
 #include "mongo/unittest/death_test.h"
 #include "mongo/unittest/unittest.h"
 
@@ -1035,6 +1036,8 @@ TEST_F(SetNodeTest, ApplySetModToEphemeralDocument) {
 }
 
 TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldInsideSetElement) {
+    RAIIServerParameterControllerForTest controller("featureFlagDotsAndDollars", false);
+
     auto update = fromjson("{$set: {a: {$bad: 1}}}");
     boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
     SetNode node;
@@ -1065,6 +1068,8 @@ TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldAtStartOfPath) {
 }
 
 TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldInMiddleOfPath) {
+    RAIIServerParameterControllerForTest controller("featureFlagDotsAndDollars", false);
+
     auto update = fromjson("{$set: {'a.$bad.b': 1}}");
     boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
     SetNode node;
@@ -1080,6 +1085,8 @@ TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldInMiddleOfPath) {
 }
 
 TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldAtEndOfPath) {
+    RAIIServerParameterControllerForTest controller("featureFlagDotsAndDollars", false);
+
     auto update = fromjson("{$set: {'a.$bad': 1}}");
     boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
     SetNode node;
diff --git a/src/mongo/db/update/storage_validation.cpp b/src/mongo/db/update/storage_validation.cpp
index 009343776f0..2199109e450 100644
--- a/src/mongo/db/update/storage_validation.cpp
+++ b/src/mongo/db/update/storage_validation.cpp
@@ -32,6 +32,9 @@
 #include "mongo/bson/bson_depth.h"
 #include "mongo/bson/mutable/algorithm.h"
 #include "mongo/bson/mutable/document.h"
+#include "mongo/db/query/dbref.h"
+#include "mongo/db/query/query_feature_flags_gen.h"
+#include "mongo/db/update/modifier_table.h"
 
 namespace mongo {
 
@@ -43,14 +46,15 @@ const StringData idFieldName = "_id"_sd;
 
 void storageValidChildren(mutablebson::ConstElement elem,
                           const bool deep,
-                          std::uint32_t recursionLevel) {
+                          std::uint32_t recursionLevel,
+                          const bool allowTopLevelDollarPrefixes) {
     if (!elem.hasChildren()) {
         return;
     }
 
     auto curr = elem.leftChild();
     while (curr.ok()) {
-        storageValid(curr, deep, recursionLevel + 1);
+        storageValid(curr, deep, recursionLevel + 1, allowTopLevelDollarPrefixes);
         curr = curr.rightSibling();
     }
 }
@@ -65,7 +69,7 @@ void validateDollarPrefixElement(mutablebson::ConstElement elem) {
     auto currName = elem.getFieldName();
 
     // Found a $db field.
-    if (currName == "$db") {
+    if (currName == dbref::kDbFieldName) {
         uassert(ErrorCodes::InvalidDBRef,
                 str::stream() << "The DBRef $db field must be a String, not a "
                               << typeName(curr.getType()),
@@ -80,7 +84,7 @@ void validateDollarPrefixElement(mutablebson::ConstElement elem) {
     }
 
     // Found a $id field.
-    if (currName == "$id") {
+    if (currName == dbref::kIdFieldName) {
         curr = curr.leftSibling();
         uassert(ErrorCodes::InvalidDBRef,
                 "Found $id field without a $ref before it, which is invalid.",
@@ -90,7 +94,7 @@ void validateDollarPrefixElement(mutablebson::ConstElement elem) {
     }
 
     // Found a $ref field.
-    if (currName == "$ref") {
+    if (currName == dbref::kRefFieldName) {
         uassert(ErrorCodes::InvalidDBRef,
                 str::stream() << "The DBRef $ref field must be a String, not a "
                               << typeName(curr.getType()),
@@ -100,7 +104,6 @@ void validateDollarPrefixElement(mutablebson::ConstElement elem) {
                 "The DBRef $ref field must be followed by a $id field",
                 curr.rightSibling().ok() && curr.rightSibling().getFieldName() == "$id");
     } else {
-
         // Not an okay, $ prefixed field name.
         uasserted(ErrorCodes::DollarPrefixedFieldName,
                   str::stream() << "The dollar ($) prefixed field '" << elem.getFieldName()
@@ -110,7 +113,7 @@ void validateDollarPrefixElement(mutablebson::ConstElement elem) {
 }
 }  // namespace
 
-void storageValid(const mutablebson::Document& doc) {
+void storageValid(const mutablebson::Document& doc, const bool allowTopLevelDollarPrefixes) {
     auto currElem = doc.root().leftChild();
     while (currElem.ok()) {
         if (currElem.getFieldName() == idFieldName) {
@@ -129,13 +132,16 @@ void storageValid(const mutablebson::Document& doc) {
         // Validate this child element.
         const auto deep = true;
         const uint32_t recursionLevel = 1;
-        storageValid(currElem, deep, recursionLevel);
+        storageValid(currElem, deep, recursionLevel, allowTopLevelDollarPrefixes);
 
         currElem = currElem.rightSibling();
     }
 }
 
-void storageValid(mutablebson::ConstElement elem, const bool deep, std::uint32_t recursionLevel) {
+void storageValid(mutablebson::ConstElement elem,
+                  const bool deep,
+                  std::uint32_t recursionLevel,
+                  const bool allowTopLevelDollarPrefixes) {
     uassert(ErrorCodes::BadValue, "Invalid elements cannot be stored.", elem.ok());
 
     uassert(ErrorCodes::Overflow,
@@ -148,7 +154,14 @@ void storageValid(mutablebson::ConstElement elem, const bool deep, std::uint32_t
     const mutablebson::ConstElement& parent = elem.parent();
     const bool childOfArray = parent.ok() ? (parent.getType() == BSONType::Array) : false;
 
-    if (!childOfArray) {
+    // If the feature flag is disabled, always validate fields for '$'-prefixes. Otherwise, only
+    // check top-level fields if 'allowTopLevelDollarPrefixes' is false, and don't validate any
+    // fields for '$'-prefixes if 'allowTopLevelDollarPrefixes' is true.
+    const bool checkTopLevelFields = !allowTopLevelDollarPrefixes && (recursionLevel == 1);
+    const bool checkFields =
+        !feature_flags::gFeatureFlagDotsAndDollars.isEnabledAndIgnoreFCV() || checkTopLevelFields;
+
+    if (!childOfArray && checkFields) {
         auto fieldName = elem.getFieldName();
 
         // Cannot start with "$", unless dbref.
@@ -160,7 +173,7 @@ void storageValid(mutablebson::ConstElement elem, const bool deep, std::uint32_t
     if (deep) {
 
         // Check children if there are any.
-        storageValidChildren(elem, deep, recursionLevel);
+        storageValidChildren(elem, deep, recursionLevel, allowTopLevelDollarPrefixes);
     }
 }
 
diff --git a/src/mongo/db/update/storage_validation.h b/src/mongo/db/update/storage_validation.h
index ee85f8ac024..a16cb8a81d0 100644
--- a/src/mongo/db/update/storage_validation.h
+++ b/src/mongo/db/update/storage_validation.h
@@ -39,15 +39,26 @@ namespace storage_validation {
  * 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.
+ *
+ * When the dots and dollars feature flag is off, always reject $-prefixed fields. Otherwise, reject
+ * only $-prefixed fields at the top-level of a document. If 'allowTopLevelDollarPrefixes' is set to
+ * true, do not reject $-prefixed fields at the top-level of a document.
  */
-void storageValid(const mutablebson::Document& doc);
+void storageValid(const mutablebson::Document& doc, const bool allowTopLevelDollarPrefixes = false);
 
 /**
  * Validates that the MutableBSON element 'elem' is acceptable for storage in a collection. If
  * 'deep' is true, the check is performed recursively on subdocuments. Uasserts if the validation
  * fails or if 'recursionLevel' exceeds the maximum allowable depth.
+ *
+ * When the dots and dollars feature flag is off, always reject $-prefixed fields. Otherwise, reject
+ * only $-prefixed fields at the top-level of a document. If 'allowTopLevelDollarPrefixes' is set to
+ * true, do not reject $-prefixed fields at the top-level of a document.
  */
-void storageValid(mutablebson::ConstElement elem, const bool deep, std::uint32_t recursionLevel);
+void storageValid(mutablebson::ConstElement elem,
+                  const bool deep,
+                  std::uint32_t recursionLevel,
+                  const bool allowTopLevelDollarPrefixes = false);
 
 }  // namespace storage_validation