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

This reverts commit f1194464424569250152308e3cae1ecbade7fb71.

10 files changed, 118 insertions, 92 deletions

diff --git a/jstests/core/batch_write_command_insert.js b/jstests/core/batch_write_command_insert.js
index 70acafe3d34..e63632e3c5a 100644
--- a/jstests/core/batch_write_command_insert.js
+++ b/jstests/core/batch_write_command_insert.js
@@ -98,6 +98,22 @@ assert.eq(1, result.n);
 assert.eq(coll.count(), 1);
 
 //
+// Document with illegal key should fail
+coll.drop();
+request = {
+    insert: coll.getName(),
+    documents: [{$set: {a: 1}}],
+    writeConcern: {w: 1},
+    ordered: false
+};
+result = coll.runCommand(request);
+assert(result.ok, tojson(result));
+assert(result.writeErrors != null);
+assert.eq(1, result.writeErrors.length);
+assert.eq(0, result.n);
+assert.eq(coll.count(), 0);
+
+//
 // Document with valid nested key should insert (op log format)
 coll.drop();
 request = {
diff --git a/jstests/core/field_name_validation.js b/jstests/core/field_name_validation.js
index 50182331fdd..3568103e768 100644
--- a/jstests/core/field_name_validation.js
+++ b/jstests/core/field_name_validation.js
@@ -7,7 +7,7 @@
  *
  * contained in a top-level element, embedded element, and within _id.
  *
- * @tags: [assumes_unsharded_collection, requires_fcv_47]
+ * @tags: [assumes_unsharded_collection]
  */
 (function() {
 "use strict";
@@ -31,14 +31,15 @@ assert.writeError(coll.insert({_id: /a/}));
 // Test that _id cannot be an array.
 assert.writeError(coll.insert({_id: [9]}));
 
-// Test that $-prefixed field names are allowed.
+// Test that $-prefixed field names are allowed in embedded objects.
 assert.commandWorked(coll.insert({a: {$b: 1}}));
 assert.eq(1, coll.find({"a.$b": 1}).itcount());
-assert.commandWorked(coll.insert({$a: 1}));
-assert.commandWorked(coll.insert({valid: 1, $a: 1}));
-assert.commandWorked(coll.insert({$a: {$b: 1}}));
 
-// Test that reserved $-prefixed field names are not allowed.
+// Test that $-prefixed field names are not allowed at the top level.
+assert.writeErrorWithCode(coll.insert({$a: 1}), ErrorCodes.BadValue);
+assert.writeErrorWithCode(coll.insert({valid: 1, $a: 1}), ErrorCodes.BadValue);
+
+// Test that reserved $-prefixed field names are also not allowed.
 assert.writeErrorWithCode(coll.insert({$ref: 1}), ErrorCodes.BadValue);
 assert.writeErrorWithCode(coll.insert({$id: 1}), ErrorCodes.BadValue);
 assert.writeErrorWithCode(coll.insert({$db: 1}), ErrorCodes.BadValue);
@@ -52,15 +53,6 @@ assert.commandWorked(coll.insert({a: {_id: [9]}}));
 assert.commandWorked(coll.insert({a: {_id: /a/}}));
 assert.commandWorked(coll.insert({a: {_id: {$b: 1}}}));
 
-// Test that inserting an object with a $-prefixed field name is properly validated.
-assert.commandWorked(coll.insert({_id: 0, $valid: 1, "a": 1}));
-assert.eq([{_id: 0, $valid: 1, "a": 1}], coll.find({_id: 0}).toArray());
-
-assert.writeErrorWithCode(coll.insert({_id: 0, $valid: 1, $id: 1}), ErrorCodes.BadValue);
-assert.writeErrorWithCode(coll.insert({_id: 0, $valid: 1, $db: 1}), ErrorCodes.BadValue);
-assert.writeErrorWithCode(coll.insert({_id: 0, $valid: 1, $ref: 1}), ErrorCodes.BadValue);
-assert.commandWorked(coll.insert({_id: 1, $valid: 1, $alsoValid: 1}));
-
 //
 // Update command field name validation.
 //
@@ -83,37 +75,18 @@ 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());
 
-// $-prefixed field names are allowed.
-assert.commandWorked(coll.update({"a.b": 1}, {$c: 1}, {upsert: true}));
-assert.commandWorked(coll.update({"a.b": 1}, {$set: {$c: 1}}, {upsert: true}));
-assert.commandWorked(coll.update({"a.b": 1}, {$set: {c: {$d: 1}}}, {upsert: true}));
-
-// Reserved $-prefixed field names are not allowed.
-assert.writeErrorWithCode(coll.update({"a.b": 1}, {$ref: 1}), ErrorCodes.InvalidDBRef);
-assert.writeErrorWithCode(coll.update({"a.b": 1}, {$id: 1}), ErrorCodes.InvalidDBRef);
-assert.writeErrorWithCode(coll.update({"a.b": 1}, {$db: 1}), ErrorCodes.InvalidDBRef);
-
-// Test that update docs with recognized operators are treated as modifier-style docs while update
-// docs with non-operator field names are treated as replacement-style docs.
-
-// Test that update documents with non-operators as field names are treated as replacement-style
-// updates.
-coll.update({_id: 1}, {$nonOp: 1}, {upsert: true});
-assert.eq([{_id: 1, $nonOp: 1}], coll.find({_id: 1}).toArray());
-
-// Test that update documents with recognized operators as field names are treated as modifier-style
-// updates.
-coll.update({_id: 2}, {$inc: {x: 1}}, {upsert: true});
-assert.eq([{_id: 2, x: 1}], coll.find({_id: 2}).toArray());
-
-// Test that update documents with a non-operator after an operator will not parse (expects all
-// operators).
-assert.writeErrorWithCode(coll.update({_id: 3}, {$set: {x: 12}, $hello: 1}, {upsert: true}),
+// $-prefixed field names are not allowed.
+assert.writeErrorWithCode(coll.update({"a.b": 1}, {$c: 1}, {upsert: true}),
                           ErrorCodes.FailedToParse);
+assert.writeErrorWithCode(coll.update({"a.b": 1}, {$set: {$c: 1}}, {upsert: true}),
+                          ErrorCodes.DollarPrefixedFieldName);
+assert.writeErrorWithCode(coll.update({"a.b": 1}, {$set: {c: {$d: 1}}}, {upsert: true}),
+                          ErrorCodes.DollarPrefixedFieldName);
 
-// Test that update documents with a non-operator before an operator will expect only non-operators.
-assert.writeErrorWithCode(coll.update({_id: 4}, {$hello: 1, $set: {x: 12}}, {upsert: true}),
-                          ErrorCodes.BadValue);
+// Reserved $-prefixed field names are also not allowed.
+assert.writeErrorWithCode(coll.update({"a.b": 1}, {$ref: 1}), ErrorCodes.FailedToParse);
+assert.writeErrorWithCode(coll.update({"a.b": 1}, {$id: 1}), ErrorCodes.FailedToParse);
+assert.writeErrorWithCode(coll.update({"a.b": 1}, {$db: 1}), ErrorCodes.FailedToParse);
 
 //
 // FindAndModify field name validation.
@@ -135,15 +108,13 @@ assert.eq([{_id: 0, "a.b": 1}], coll.find({_id: 0}).toArray());
 coll.findAndModify({query: {_id: 1, "a.b": 1}, update: {$set: {_id: 1, "a.b": 2}}});
 assert.eq([{_id: 1, a: {b: 2}}], coll.find({_id: 1}).toArray());
 
-// Test that $-prefixed fields are allowed.
-coll.findAndModify({query: {_id: 1}, update: {_id: 1, $valid: 1}});
-assert.eq([{_id: 1, $valid: 1}], coll.find({_id: 1}).toArray());
-
-coll.findAndModify({query: {_id: 1}, update: {_id: 1, $embed: {$a: 1, "b": 2}}});
-assert([{_id: 1, $embed: {$a: 1, "b": 2}}], coll.find({_id: 1}).toArray());
-
-coll.findAndModify({query: {_id: 2}, update: {_id: 2, out: {$in: 1, "x": 2}, upsert: true}});
-assert([{_id: 2, out: {$in: 1, "x": 2}}], coll.find({_id: 2}).toArray());
+// $-prefixed field names are not allowed.
+assert.throws(function() {
+    coll.findAndModify({query: {_id: 1}, update: {_id: 1, $invalid: 1}});
+});
+assert.throws(function() {
+    coll.findAndModify({query: {_id: 1}, update: {$set: {_id: 1, $invalid: 1}}});
+});
 
 // Reserved $-prefixed field names are also not allowed.
 assert.throws(function() {
diff --git a/jstests/core/insert2.js b/jstests/core/insert2.js
new file mode 100644
index 00000000000..6052f1eeb8d
--- /dev/null
+++ b/jstests/core/insert2.js
@@ -0,0 +1,21 @@
+// Cannot implicitly shard accessed collections because of collection existing when none
+// expected.
+// @tags: [
+//   assumes_no_implicit_collection_creation_after_drop,
+//   requires_collstats,
+//   uses_multiple_connections,
+// ]
+
+// Create a new connection object so it won't affect the global connection when we modify
+// it's settings.
+var conn = new Mongo(db.getMongo().host);
+conn.forceWriteMode(db.getMongo().writeMode());
+
+t = conn.getDB(db.getName()).insert2;
+t.drop();
+
+assert.isnull(t.findOne(), "A");
+assert.writeError(t.insert({z: 1, $inc: {x: 1}}, 0, true));
+assert.isnull(t.findOne(), "B");
+// Collection should not exist
+assert.isnull(t.exists());
diff --git a/jstests/core/push_sort.js b/jstests/core/push_sort.js
index 4bd6485b29c..347f230a93d 100644
--- a/jstests/core/push_sort.js
+++ b/jstests/core/push_sort.js
@@ -1,4 +1,4 @@
-// @tags: [requires_non_retryable_writes, requires_fcv_47]
+// @tags: [requires_non_retryable_writes]
 
 //
 //  $push acquired the possibility of sorting the resulting array as part of SERVER-8008. This
@@ -54,13 +54,12 @@ assert.eq([{a: {b: 2}}, {a: {b: 3}}], t.findOne({_id: 7}).x);
 // Invalid Cases
 //
 
-// Test that when given a document with a $sort field that matches the form of a plain document
-// (instead of a $sort modifier document), $push will add that field to the specified array.
+// $push with $sort should not push a "$sort" field
 var doc8 = {_id: 8, x: [{a: 1}, {a: 2}]};
 t.save(doc8);
 var res = t.update({_id: 8}, {$push: {x: {$sort: {a: -1}}}});
-assert.commandWorked(res);
-assert.docEq(t.findOne({_id: 8}), {_id: 8, x: [{a: 1}, {a: 2}, {$sort: {a: -1}}]});
+assert.writeErrorWithCode(res, ErrorCodes.DollarPrefixedFieldName);
+assert.docEq(t.findOne({_id: 8}), doc8);  // ensure doc was not changed
 
 t.save({_id: 100, x: [{a: 1}]});
 
diff --git a/jstests/core/update_addToSet.js b/jstests/core/update_addToSet.js
index 7e9b262591c..5927dd882f8 100644
--- a/jstests/core/update_addToSet.js
+++ b/jstests/core/update_addToSet.js
@@ -71,6 +71,28 @@ t.update({_id: 1}, {$addToSet: {a: {$each: [3, 2, 2, 3, 3]}}});
 o.a.push(3);
 assert.eq(o, t.findOne(), "D4");
 
+// Test that dotted and '$' prefixed field names fail.
+t.drop();
+o = {
+    _id: 1,
+    a: [1, 2]
+};
+assert.commandWorked(t.insert(o));
+
+assert.commandWorked(t.update({}, {$addToSet: {a: {'x.$.y': 'bad'}}}));
+assert.commandWorked(t.update({}, {$addToSet: {a: {b: {'x.$.y': 'bad'}}}}));
+
+assert.writeError(t.update({}, {$addToSet: {a: {"$bad": "bad"}}}));
+assert.writeError(t.update({}, {$addToSet: {a: {b: {"$bad": "bad"}}}}));
+
+assert.commandWorked(t.update({}, {$addToSet: {a: {_id: {"x.y": 2}}}}));
+
+assert.commandWorked(t.update({}, {$addToSet: {a: {$each: [{'x.$.y': 'bad'}]}}}));
+assert.commandWorked(t.update({}, {$addToSet: {a: {$each: [{b: {'x.$.y': 'bad'}}]}}}));
+
+assert.writeError(t.update({}, {$addToSet: {a: {$each: [{'$bad': 'bad'}]}}}));
+assert.writeError(t.update({}, {$addToSet: {a: {$each: [{b: {'$bad': 'bad'}}]}}}));
+
 // Test that nested _id fields are allowed.
 t.drop();
 o = {
diff --git a/jstests/core/update_with_pipeline.js b/jstests/core/update_with_pipeline.js
index 70a6564c92f..4f044a19352 100644
--- a/jstests/core/update_with_pipeline.js
+++ b/jstests/core/update_with_pipeline.js
@@ -4,7 +4,7 @@
  * 'requires_find_command' needed to prevent this test from running with 'compatibility' write mode
  * as pipeline-style update is not supported by OP_UPDATE.
  *
- * @tags: [requires_find_command, requires_non_retryable_writes, requires_fcv_47]
+ * @tags: [requires_find_command, requires_non_retryable_writes]
  */
 (function() {
 "use strict";
@@ -195,9 +195,8 @@ assert.commandFailedWithCode(
         }]),
     ErrorCodes.InvalidOptions);
 
-// Update uses replacement document when supported agg stage is specified outside of pipeline.
-coll.update({_id: 1}, {$addFields: {x: 1}});
-assert.eq([{_id: 1, $addFields: {x: 1}}], coll.find({_id: 1}).toArray());
+// Update fails when supported agg stage is specified outside of pipeline.
+assert.commandFailedWithCode(coll.update({_id: 1}, {$addFields: {x: 1}}), ErrorCodes.FailedToParse);
 
 // The 'arrayFilters' option is not valid for pipeline updates.
 assert.commandFailedWithCode(
@@ -255,12 +254,12 @@ testUpdate({
     nModified: 1
 });
 
-// Test that expressions within constants are treated as field names instead of expressions.
-db.runCommand({
+// Cannot use expressions in constants.
+assert.commandFailedWithCode(db.runCommand({
     update: collName,
     updates: [{q: {_id: 1}, u: [{$set: {x: "$$foo"}}], c: {foo: {$add: [1, 2]}}}]
-});
-assert.eq([{_id: 1, x: {$add: [1, 2]}, foo: "$x"}], coll.find({_id: 1}).toArray());
+}),
+                             ErrorCodes.DollarPrefixedFieldName);
 
 // Cannot use constants with regular updates.
 assert.commandFailedWithCode(
diff --git a/jstests/core/updateh.js b/jstests/core/updateh.js
index 7e3d5ae0a59..1fd3d62750d 100644
--- a/jstests/core/updateh.js
+++ b/jstests/core/updateh.js
@@ -1,9 +1,9 @@
 // Cannot implicitly shard accessed collections because of following errmsg: A single
 // update/delete on a sharded collection must contain an exact match on _id or contain the shard
 // key.
-// @tags: [assumes_unsharded_collection, requires_fcv_47]
+// @tags: [assumes_unsharded_collection]
 
-// Allow $ in field names (as of SERVER-49117)
+// Disallow $ in field names
 var res;
 
 t = db.jstest_updateh;
@@ -11,39 +11,39 @@ t.drop();
 
 t.insert({x: 1});
 
-res = t.update({x: 1}, {$set: {y: 1}});
+res = t.update({x: 1}, {$set: {y: 1}});  // ok
 assert.commandWorked(res);
 
-res = t.update({x: 1}, {$set: {$z: 1}});
-assert.commandWorked(res);
+res = t.update({x: 1}, {$set: {$z: 1}});  // not ok
+assert.writeError(res);
 
-res = t.update({x: 1}, {$set: {'a.$b': 1}});
-assert.commandWorked(res);
+res = t.update({x: 1}, {$set: {'a.$b': 1}});  // not ok
+assert.writeError(res);
 
-res = t.update({x: 1}, {$inc: {$z: 1}});
-assert.commandWorked(res);
+res = t.update({x: 1}, {$inc: {$z: 1}});  // not ok
+assert.writeError(res);
 
 // Second section
 t.drop();
 
 t.save({_id: 0, n: 0});
 
-// Test that '$' can be the first character in a field.
-// (as of SERVER-49117)
+// Test that '$' cannot be the first character in a field.
+// SERVER-7150
 res = t.update({n: 0}, {$set: {$x: 1}});
-assert.commandWorked(res);
+assert.writeError(res);
 
 res = t.update({n: 0}, {$set: {$$$: 1}});
-assert.commandWorked(res);
+assert.writeError(res);
 
 res = t.update({n: 0}, {$set: {"sneaky.$x": 1}});
-assert.commandWorked(res);
+assert.writeError(res);
 
 res = t.update({n: 0}, {$set: {"secret.agent$.$x": 1}});
-assert.commandWorked(res);
+assert.writeError(res);
 
 res = t.update({n: 0}, {$set: {"$secret.agent.x": 1}});
-assert.commandWorked(res);
+assert.writeError(res);
 
 res = t.update({n: 0}, {$set: {"secret.agent$": 1}});
 assert.commandWorked(res);
@@ -60,16 +60,14 @@ t.save({_id: 0, n: 0});
 
 // SERVER-11241: Validation used to allow any DBRef field name as a prefix
 // thus allowing things like $idXXX
-// SERVER-49117: $-prefixed field names are allowed, as long as they are
-// not equal to reserved $-prefixed field names (i.e. $ref/$id/$db)
 res = t.update({n: 0}, {$set: {$reffoo: 1}});
-assert.commandWorked(res);
+assert.writeError(res);
 
 res = t.update({n: 0}, {$set: {$idbar: 1}});
-assert.commandWorked(res);
+assert.writeError(res);
 
 res = t.update({n: 0}, {$set: {$dbbaz: 1}});
-assert.commandWorked(res);
+assert.writeError(res);
 
 // Test that '$id', '$db', and '$ref' are acceptable field names in
 // the correct case ( subdoc)
diff --git a/jstests/core/write_result.js b/jstests/core/write_result.js
index 100322a00e4..49735bee7d6 100644
--- a/jstests/core/write_result.js
+++ b/jstests/core/write_result.js
@@ -119,7 +119,7 @@ assert.eq(coll.count(), 1);
 // Update with error
 coll.remove({});
 coll.insert({foo: "bar"});
-result = coll.update({foo: "bar"}, {_id: /a/});
+printjson(result = coll.update({foo: "bar"}, {$invalid: "expr"}));
 assert.eq(result.nUpserted, 0);
 assert.eq(result.nMatched, 0);
 if (coll.getMongo().writeMode() == "commands")
diff --git a/jstests/gle/gle_sharded_write.js b/jstests/gle/gle_sharded_write.js
index 38ce0f839de..21a42381c96 100644
--- a/jstests/gle/gle_sharded_write.js
+++ b/jstests/gle/gle_sharded_write.js
@@ -111,7 +111,7 @@ assert.eq(coll.count(), 0);
 //
 // Error on one host during update
 coll.remove({});
-coll.update({_id: 1}, {$set: {x: 12}, $hello: 1}, true);
+coll.update({_id: 1}, {$invalid: "xxx"}, true);
 printjson(gle = coll.getDB().runCommand({getLastError: 1}));
 assert(gle.ok);
 assert(gle.err);
@@ -135,7 +135,7 @@ assert.eq(coll.count(), 0);
 //
 // Repeated calls to GLE should work
 coll.remove({});
-coll.update({_id: 1}, {$set: {x: 12}, $hello: 1}, true);
+coll.update({_id: 1}, {$invalid: "xxx"}, true);
 printjson(gle = coll.getDB().runCommand({getLastError: 1}));
 assert(gle.ok);
 assert(gle.err);
diff --git a/jstests/sharding/server_status_crud_metrics.js b/jstests/sharding/server_status_crud_metrics.js
index 4f83d7e4b6e..62a513242ca 100644
--- a/jstests/sharding/server_status_crud_metrics.js
+++ b/jstests/sharding/server_status_crud_metrics.js
@@ -33,8 +33,8 @@ assert.commandWorked(testDB.coll.update({_id: 1}, {$set: {a: 2}}, {multi: false}
 // fails on the individual shard.
 assert.commandFailedWithCode(testDB.coll.update({_id: 1}, {$set: {x: 2}}, {multi: false}), 31025);
 assert.commandFailedWithCode(
-    testDB.coll.update({_id: 1}, {$set: {x: 12}, $hello: 1}, {multi: false}),
-    ErrorCodes.FailedToParse);
+    testDB.coll.update({_id: 1}, {$set: {x: 2, $invalidField: 4}}, {multi: false}),
+    ErrorCodes.DollarPrefixedFieldName);
 
 let mongosServerStatus = testDB.adminCommand({serverStatus: 1});