SERVER-49117 Remove storage validation of '$' and '.' in field names for insert and update

12 files changed, 92 insertions, 139 deletions

diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp
index c4013697ebe..1ef996ffed0 100644
--- a/src/mongo/db/matcher/expression_parser.cpp
+++ b/src/mongo/db/matcher/expression_parser.cpp
@@ -66,6 +66,7 @@
 #include "mongo/db/matcher/schema/expression_internal_schema_xor.h"
 #include "mongo/db/matcher/schema/json_schema_parser.h"
 #include "mongo/db/namespace_string.h"
+#include "mongo/db/query/dbref.h"
 #include "mongo/db/query/query_knobs_gen.h"
 #include "mongo/util/str.h"
 #include "mongo/util/string_map.h"
@@ -214,15 +215,15 @@ bool isDBRefDocument(const BSONObj& obj, bool allowIncompleteDBRef) {
         auto element = i.next();
         auto fieldName = element.fieldNameStringData();
         // $ref
-        if (!hasRef && "$ref"_sd == fieldName) {
+        if (!hasRef && dbref::kRefFieldName == fieldName) {
             hasRef = true;
         }
         // $id
-        else if (!hasID && "$id"_sd == fieldName) {
+        else if (!hasID && dbref::kIdFieldName == fieldName) {
             hasID = true;
         }
         // $db
-        else if (!hasDB && "$db"_sd == fieldName) {
+        else if (!hasDB && dbref::kDbFieldName == fieldName) {
             hasDB = true;
         }
     }
diff --git a/src/mongo/db/ops/insert.cpp b/src/mongo/db/ops/insert.cpp
index 383536913e1..07663838aec 100644
--- a/src/mongo/db/ops/insert.cpp
+++ b/src/mongo/db/ops/insert.cpp
@@ -34,6 +34,7 @@
 
 #include "mongo/bson/bson_depth.h"
 #include "mongo/db/commands/feature_compatibility_version_parser.h"
+#include "mongo/db/query/dbref.h"
 #include "mongo/db/vector_clock_mutable.h"
 #include "mongo/db/views/durable_view_catalog.h"
 #include "mongo/util/str.h"
@@ -74,6 +75,11 @@ Status validateDepth(const BSONObj& obj) {
 }
 }  // namespace
 
+bool isReservedDollarPrefixedWord(StringData fieldName) {
+    return (std::find(dbref::kDbRefFieldNames.begin(), dbref::kDbRefFieldNames.end(), fieldName) !=
+            std::end(dbref::kDbRefFieldNames));
+}
+
 StatusWith<BSONObj> fixDocumentForInsert(ServiceContext* service, const BSONObj& doc) {
     if (doc.objsize() > BSONObjMaxUserSize)
         return StatusWith<BSONObj>(ErrorCodes::BadValue,
@@ -102,10 +108,13 @@ StatusWith<BSONObj> fixDocumentForInsert(ServiceContext* service, const BSONObj&
 
             auto fieldName = e.fieldNameStringData();
 
-            if (fieldName[0] == '$') {
-                return StatusWith<BSONObj>(
-                    ErrorCodes::BadValue,
-                    str::stream() << "Document can't have $ prefixed field names: " << fieldName);
+            // Ensure that fieldName is not a reserved $-prefixed field name.
+            if (isReservedDollarPrefixedWord(fieldName)) {
+
+                return StatusWith<BSONObj>(ErrorCodes::BadValue,
+                                           str::stream() << fieldName << " is a reserved fieldName"
+                                                         << " and cannot be used. Please"
+                                                         << " try another field name.");
             }
 
             // check no regexp for _id (SERVER-9502)
diff --git a/src/mongo/db/ops/insert.h b/src/mongo/db/ops/insert.h
index d21e74ceb1c..a0b24f4a271 100644
--- a/src/mongo/db/ops/insert.h
+++ b/src/mongo/db/ops/insert.h
@@ -34,6 +34,12 @@ namespace mongo {
 
 class ServiceContext;
 
+/*
+ * Returns true if fieldName is one of the reserved $-prefixed words
+ * and false if it isn't.
+ */
+bool isReservedDollarPrefixedWord(StringData fieldName);
+
 /**
  * Validates that 'doc' is legal for insertion, possibly with some modifications.
  *
diff --git a/src/mongo/db/query/dbref.h b/src/mongo/db/query/dbref.h
new file mode 100644
index 00000000000..ffa4be06b26
--- /dev/null
+++ b/src/mongo/db/query/dbref.h
@@ -0,0 +1,47 @@
+/**
+ *    Copyright (C) 2018-present MongoDB, Inc.
+ *
+ *    This program is free software: you can redistribute it and/or modify
+ *    it under the terms of the Server Side Public License, version 1,
+ *    as published by MongoDB, Inc.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    Server Side Public License for more details.
+ *
+ *    You should have received a copy of the Server Side Public License
+ *    along with this program. If not, see
+ *    <http://www.mongodb.com/licensing/server-side-public-license>.
+ *
+ *    As a special exception, the copyright holders give permission to link the
+ *    code of portions of this program with the OpenSSL library under certain
+ *    conditions as described in each individual source file and distribute
+ *    linked combinations including the program with the OpenSSL library. You
+ *    must comply with the Server Side Public License in all respects for
+ *    all of the code used other than as permitted herein. If you modify file(s)
+ *    with this exception, you may extend this exception to your version of the
+ *    file(s), but you are not obligated to do so. If you do not wish to do so,
+ *    delete this exception statement from your version. If you delete this
+ *    exception statement from all source files in the program, then also delete
+ *    it in the license file.
+ */
+
+#pragma once
+
+#include <set>
+
+#include "mongo/base/string_data.h"
+
+namespace mongo {
+namespace dbref {
+
+constexpr StringData kDbFieldName = "$db"_sd;
+constexpr StringData kIdFieldName = "$id"_sd;
+constexpr StringData kRefFieldName = "$ref"_sd;
+
+// A list containing all the reserved $-prefixed field names.
+constexpr std::array<StringData, 3> kDbRefFieldNames = {kDbFieldName, kIdFieldName, kRefFieldName};
+
+}  // namespace dbref
+}  // namespace mongo
\ No newline at end of file
diff --git a/src/mongo/db/repl/storage_interface_impl_test.cpp b/src/mongo/db/repl/storage_interface_impl_test.cpp
index 9a121099170..8abf0c66405 100644
--- a/src/mongo/db/repl/storage_interface_impl_test.cpp
+++ b/src/mongo/db/repl/storage_interface_impl_test.cpp
@@ -2220,31 +2220,6 @@ TEST_F(StorageInterfaceImplTest,
               ErrorCodes::IndexNotFound);
 }
 
-TEST_F(StorageInterfaceImplTest,
-       UpsertSingleDocumentReturnsFailedToParseWhenUpdateDocumentContainsUnknownOperator) {
-    auto opCtx = getOperationContext();
-    auto nss = makeNamespace(_agent);
-    auto options = generateOptionsWithUuid();
-
-    StorageInterfaceImpl storage;
-    ASSERT_OK(storage.createCollection(opCtx, nss, options));
-
-    auto unknownUpdateOp = BSON("$unknownUpdateOp" << BSON("x" << 1000));
-    ASSERT_THROWS_CODE_AND_WHAT(
-        storage.upsertById(opCtx, nss, BSON("" << 1).firstElement(), unknownUpdateOp),
-        AssertionException,
-        ErrorCodes::FailedToParse,
-        "Unknown modifier: $unknownUpdateOp. Expected a valid update modifier or pipeline-style "
-        "update specified as an array");
-
-    ASSERT_THROWS_CODE(storage.upsertById(opCtx,
-                                          {nss.db().toString(), *options.uuid},
-                                          BSON("" << 1).firstElement(),
-                                          unknownUpdateOp),
-                       DBException,
-                       ErrorCodes::FailedToParse);
-}
-
 TEST_F(StorageInterfaceImplTest, DeleteByFilterReturnsNamespaceNotFoundWhenDatabaseDoesNotExist) {
     auto opCtx = getOperationContext();
     StorageInterfaceImpl storage;
diff --git a/src/mongo/db/update/SConscript b/src/mongo/db/update/SConscript
index 9a93690dd21..dec70a64686 100644
--- a/src/mongo/db/update/SConscript
+++ b/src/mongo/db/update/SConscript
@@ -9,8 +9,7 @@ env.Library(
     source=[
         'field_checker.cpp',
         'log_builder.cpp',
-        'path_support.cpp',
-        'storage_validation.cpp',
+        'path_support.cpp'
     ],
     LIBDEPS=[
         '$BUILD_DIR/mongo/base',
@@ -37,6 +36,7 @@ env.Library(
         'push_node.cpp',
         'rename_node.cpp',
         'set_node.cpp',
+        'storage_validation.cpp',
         'unset_node.cpp',
         'update_array_node.cpp',
         'update_internal_node.cpp',
diff --git a/src/mongo/db/update/object_replace_executor_test.cpp b/src/mongo/db/update/object_replace_executor_test.cpp
index 0f7adf40df5..284c74639d5 100644
--- a/src/mongo/db/update/object_replace_executor_test.cpp
+++ b/src/mongo/db/update/object_replace_executor_test.cpp
@@ -258,18 +258,6 @@ TEST_F(ObjectReplaceExecutorTest, CanAddImmutableId) {
     ASSERT_BSONOBJ_BINARY_EQ(fromjson("{_id: 0}"), result.oplogEntry);
 }
 
-TEST_F(ObjectReplaceExecutorTest, CannotCreateDollarPrefixedNameWhenValidateForStorageIsTrue) {
-    auto obj = fromjson("{a: {b: 1, $bad: 1}}");
-    ObjectReplaceExecutor node(obj);
-
-    mutablebson::Document doc(fromjson("{}"));
-    ASSERT_THROWS_CODE_AND_WHAT(
-        node.applyUpdate(getApplyParams(doc.root())),
-        AssertionException,
-        ErrorCodes::DollarPrefixedFieldName,
-        "The dollar ($) prefixed field '$bad' in 'a.$bad' is not valid for storage.");
-}
-
 TEST_F(ObjectReplaceExecutorTest, CanCreateDollarPrefixedNameWhenValidateForStorageIsFalse) {
     auto obj = fromjson("{a: {b: 1, $bad: 1}}");
     ObjectReplaceExecutor node(obj);
diff --git a/src/mongo/db/update/rename_node_test.cpp b/src/mongo/db/update/rename_node_test.cpp
index 6eec4d8f498..f263204f708 100644
--- a/src/mongo/db/update/rename_node_test.cpp
+++ b/src/mongo/db/update/rename_node_test.cpp
@@ -550,21 +550,6 @@ TEST_F(RenameNodeTest, ApplyCanRemoveImmutablePathIfNoop) {
     ASSERT_EQUALS(getModifiedPaths(), "{a.b.c, d}");
 }
 
-TEST_F(RenameNodeTest, ApplyCannotCreateDollarPrefixedField) {
-    auto update = fromjson("{$rename: {a: '$bad'}}");
-    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
-    RenameNode node;
-    ASSERT_OK(node.init(update["$rename"]["a"], expCtx));
-
-    mutablebson::Document doc(fromjson("{a: 0}"));
-    setPathToCreate("$bad");
-    ASSERT_THROWS_CODE_AND_WHAT(
-        node.apply(getApplyParams(doc.root()), getUpdateNodeApplyParams()),
-        AssertionException,
-        ErrorCodes::DollarPrefixedFieldName,
-        "The dollar ($) prefixed field '$bad' in '$bad' is not valid for storage.");
-}
-
 TEST_F(RenameNodeTest, ApplyCannotOverwriteImmutablePath) {
     auto update = fromjson("{$rename: {a: 'b'}}");
     boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
diff --git a/src/mongo/db/update/set_node_test.cpp b/src/mongo/db/update/set_node_test.cpp
index 8cd2b60243e..e31edf0b6a6 100644
--- a/src/mongo/db/update/set_node_test.cpp
+++ b/src/mongo/db/update/set_node_test.cpp
@@ -1018,66 +1018,6 @@ TEST_F(SetNodeTest, ApplySetModToEphemeralDocument) {
     ASSERT_FALSE(doc.isInPlaceModeEnabled());
 }
 
-TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldInsideSetElement) {
-    auto update = fromjson("{$set: {a: {$bad: 1}}}");
-    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
-    SetNode node;
-    ASSERT_OK(node.init(update["$set"]["a"], expCtx));
-
-    mutablebson::Document doc(fromjson("{a: 5}"));
-    setPathTaken("a");
-    ASSERT_THROWS_CODE_AND_WHAT(
-        node.apply(getApplyParams(doc.root()["a"]), getUpdateNodeApplyParams()),
-        AssertionException,
-        ErrorCodes::DollarPrefixedFieldName,
-        "The dollar ($) prefixed field '$bad' in 'a.$bad' is not valid for storage.");
-}
-
-TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldAtStartOfPath) {
-    auto update = fromjson("{$set: {'$bad.a': 1}}");
-    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
-    SetNode node;
-    ASSERT_OK(node.init(update["$set"]["$bad.a"], expCtx));
-
-    mutablebson::Document doc(fromjson("{}"));
-    setPathToCreate("$bad.a");
-    ASSERT_THROWS_CODE_AND_WHAT(
-        node.apply(getApplyParams(doc.root()), getUpdateNodeApplyParams()),
-        AssertionException,
-        ErrorCodes::DollarPrefixedFieldName,
-        "The dollar ($) prefixed field '$bad' in '$bad' is not valid for storage.");
-}
-
-TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldInMiddleOfPath) {
-    auto update = fromjson("{$set: {'a.$bad.b': 1}}");
-    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
-    SetNode node;
-    ASSERT_OK(node.init(update["$set"]["a.$bad.b"], expCtx));
-
-    mutablebson::Document doc(fromjson("{}"));
-    setPathToCreate("a.$bad.b");
-    ASSERT_THROWS_CODE_AND_WHAT(
-        node.apply(getApplyParams(doc.root()), getUpdateNodeApplyParams()),
-        AssertionException,
-        ErrorCodes::DollarPrefixedFieldName,
-        "The dollar ($) prefixed field '$bad' in 'a.$bad' is not valid for storage.");
-}
-
-TEST_F(SetNodeTest, ApplyCannotCreateDollarPrefixedFieldAtEndOfPath) {
-    auto update = fromjson("{$set: {'a.$bad': 1}}");
-    boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
-    SetNode node;
-    ASSERT_OK(node.init(update["$set"]["a.$bad"], expCtx));
-
-    mutablebson::Document doc(fromjson("{}"));
-    setPathToCreate("a.$bad");
-    ASSERT_THROWS_CODE_AND_WHAT(
-        node.apply(getApplyParams(doc.root()), getUpdateNodeApplyParams()),
-        AssertionException,
-        ErrorCodes::DollarPrefixedFieldName,
-        "The dollar ($) prefixed field '$bad' in 'a.$bad' is not valid for storage.");
-}
-
 TEST_F(SetNodeTest, ApplyCanCreateDollarPrefixedFieldNameWhenValidateForStorageIsFalse) {
     auto update = fromjson("{$set: {$bad: 1}}");
     boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
diff --git a/src/mongo/db/update/storage_validation.cpp b/src/mongo/db/update/storage_validation.cpp
index 009343776f0..6f2d1e9298b 100644
--- a/src/mongo/db/update/storage_validation.cpp
+++ b/src/mongo/db/update/storage_validation.cpp
@@ -32,6 +32,8 @@
 #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/update/modifier_table.h"
 
 namespace mongo {
 
@@ -58,14 +60,15 @@ void storageValidChildren(mutablebson::ConstElement elem,
 /**
  * Validates an element that has a field name which starts with a dollar sign ($).
  * In the case of a DBRef field ($id, $ref, [$db]) these fields may be valid in
- * the correct order/context only.
+ * the correct order/context only. In other cases, $-prefixed field names are
+ * now allowed (SERVER-49117).
  */
 void validateDollarPrefixElement(mutablebson::ConstElement elem) {
     auto curr = 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 +83,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 +93,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()),
@@ -99,14 +102,13 @@ void validateDollarPrefixElement(mutablebson::ConstElement elem) {
         uassert(ErrorCodes::InvalidDBRef,
                 "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()
-                                << "' in '" << mutablebson::getFullName(elem)
-                                << "' is not valid for storage.");
     }
+
+    // Found a non-reserved $-prefixed field.
+    uassert(ErrorCodes::BadValue,
+            str::stream() << "Field " << currName << " is invalid. "
+                          << "Please use a field name that is not an update modifier",
+            modifiertable::getType(currName) == modifiertable::MOD_UNKNOWN);
 }
 }  // namespace
 
diff --git a/src/mongo/db/update/update_driver.cpp b/src/mongo/db/update/update_driver.cpp
index e0cf8dafb4c..91b625ee3ef 100644
--- a/src/mongo/db/update/update_driver.cpp
+++ b/src/mongo/db/update/update_driver.cpp
@@ -312,7 +312,10 @@ void UpdateDriver::setCollator(const CollatorInterface* collator) {
 
 bool UpdateDriver::isDocReplacement(const write_ops::UpdateModification& updateMod) {
     return (updateMod.type() == write_ops::UpdateModification::Type::kClassic &&
-            *updateMod.getUpdateClassic().firstElementFieldName() != '$') ||
+            (modifiertable::getType(
+                 updateMod.getUpdateClassic().firstElementFieldNameStringData()) ==
+                 modifiertable::MOD_UNKNOWN &&
+             updateMod.getUpdateClassic().firstElementFieldNameStringData() != "$v"_sd)) ||
         updateMod.type() == write_ops::UpdateModification::Type::kPipeline;
 }
 
diff --git a/src/mongo/db/update/update_driver_test.cpp b/src/mongo/db/update/update_driver_test.cpp
index 5c736122c65..c9610841d80 100644
--- a/src/mongo/db/update/update_driver_test.cpp
+++ b/src/mongo/db/update/update_driver_test.cpp
@@ -128,15 +128,12 @@ TEST(Parse, EmptyMod) {
         "'$set' is empty. You must specify a field like so: {$set: {<field>: ...}}");
 }
 
-TEST(Parse, WrongMod) {
+TEST(Parse, UnknownMod) {
     boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
     UpdateDriver driver(expCtx);
     std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters;
-    ASSERT_THROWS_CODE_AND_WHAT(driver.parse(fromjson("{$xyz:{a:1}}"), arrayFilters),
-                                AssertionException,
-                                ErrorCodes::FailedToParse,
-                                "Unknown modifier: $xyz. Expected a valid update modifier or "
-                                "pipeline-style update specified as an array");
+    ASSERT_DOES_NOT_THROW(driver.parse(fromjson("{$xyz:{a:1}}"), arrayFilters));
+    ASSERT_TRUE(driver.type() == UpdateDriver::UpdateType::kReplacement);
 }
 
 TEST(Parse, WrongType) {