diff options
31 files changed, 184 insertions, 290 deletions
diff --git a/buildscripts/build/eslint b/buildscripts/build/eslint Binary files differnew file mode 100755 index 00000000000..3ea705c1289 --- /dev/null +++ b/buildscripts/build/eslint diff --git a/buildscripts/resmokeconfig/suites/replica_sets_kill_primary_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/replica_sets_kill_primary_jscore_passthrough.yml index ee21efb3f14..f1677f377cc 100644 --- a/buildscripts/resmokeconfig/suites/replica_sets_kill_primary_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/replica_sets_kill_primary_jscore_passthrough.yml @@ -59,7 +59,6 @@ selector: - jstests/core/benchrun_pipeline_updates.js # benchRun() used for writes - jstests/core/connection_string_validation.js # Does not expect a replica set connection string. - jstests/core/explain_large_bounds.js # Stepdown can timeout waiting for global lock. - - jstests/core/insert2.js # Creates new mongo connection. - jstests/core/list_collections_filter.js # Temporary collections are dropped on failover. - jstests/core/startup_log.js # Checks pid, which is different on each server. - jstests/core/validate_cmd_ns.js # Calls _exec() directly, not retryable. diff --git a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_kill_primary_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_kill_primary_jscore_passthrough.yml index 6f535d3d154..df0ac0d1b09 100644 --- a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_kill_primary_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_kill_primary_jscore_passthrough.yml @@ -245,7 +245,6 @@ selector: - jstests/core/benchrun_pipeline_updates.js # benchRun() used for writes - jstests/core/connection_string_validation.js # Does not expect a replica set connection string. - jstests/core/explain_large_bounds.js # Stepdown can timeout waiting for global lock. - - jstests/core/insert2.js # Creates new mongo connection. - jstests/core/list_collections_filter.js # Temporary collections are dropped on failover. - jstests/core/startup_log.js # Checks pid, which is different on each server. diff --git a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_stepdown_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_stepdown_jscore_passthrough.yml index c6b41120c4f..49f6c37275a 100644 --- a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_stepdown_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_stepdown_jscore_passthrough.yml @@ -246,7 +246,6 @@ selector: - jstests/core/benchrun_pipeline_updates.js # benchRun() used for writes - jstests/core/connection_string_validation.js # Does not expect a replica set connection string. - jstests/core/explain_large_bounds.js # Stepdown can timeout waiting for global lock. - - jstests/core/insert2.js # Creates new mongo connection. - jstests/core/list_collections_filter.js # Temporary collections are dropped on failover. - jstests/core/startup_log.js # Checks pid, which is different on each server. diff --git a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_terminate_primary_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_terminate_primary_jscore_passthrough.yml index 6a74ccd058c..efb2e96e7e0 100644 --- a/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_terminate_primary_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/replica_sets_multi_stmt_txn_terminate_primary_jscore_passthrough.yml @@ -243,7 +243,6 @@ selector: - jstests/core/benchrun_pipeline_updates.js # benchRun() used for writes - jstests/core/connection_string_validation.js # Does not expect a replica set connection string. - jstests/core/explain_large_bounds.js # Stepdown can timeout waiting for global lock. - - jstests/core/insert2.js # Creates new mongo connection. - jstests/core/list_collections_filter.js # Temporary collections are dropped on failover. - jstests/core/startup_log.js # Checks pid, which is different on each server. diff --git a/buildscripts/resmokeconfig/suites/replica_sets_terminate_primary_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/replica_sets_terminate_primary_jscore_passthrough.yml index 0228ac4f220..e1379646dca 100644 --- a/buildscripts/resmokeconfig/suites/replica_sets_terminate_primary_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/replica_sets_terminate_primary_jscore_passthrough.yml @@ -59,7 +59,6 @@ selector: - jstests/core/benchrun_pipeline_updates.js # benchRun() used for writes - jstests/core/connection_string_validation.js # Does not expect a replica set connection string. - jstests/core/explain_large_bounds.js # Stepdown can timeout waiting for global lock. - - jstests/core/insert2.js # Creates new mongo connection. - jstests/core/list_collections_filter.js # Temporary collections are dropped on failover. - jstests/core/startup_log.js # Checks pid, which is different on each server. - jstests/core/validate_cmd_ns.js # Calls _exec() directly, not retryable. diff --git a/buildscripts/resmokeconfig/suites/retryable_writes_jscore_stepdown_passthrough.yml b/buildscripts/resmokeconfig/suites/retryable_writes_jscore_stepdown_passthrough.yml index d0cb71d09c4..07d0f62743a 100644 --- a/buildscripts/resmokeconfig/suites/retryable_writes_jscore_stepdown_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/retryable_writes_jscore_stepdown_passthrough.yml @@ -63,7 +63,6 @@ selector: - jstests/core/benchrun_pipeline_updates.js # benchRun() used for writes - jstests/core/connection_string_validation.js # Does not expect a replica set connection string. - jstests/core/explain_large_bounds.js # Stepdown can timeout waiting for global lock. - - jstests/core/insert2.js # Creates new mongo connection. - jstests/core/list_collections_filter.js # Temporary collections are dropped on failover. - jstests/core/startup_log.js # Checks pid, which is different on each server. - jstests/core/validate_cmd_ns.js # Calls _exec() directly, not retryable. diff --git a/jstests/core/batch_write_command_insert.js b/jstests/core/batch_write_command_insert.js index e63632e3c5a..70acafe3d34 100644 --- a/jstests/core/batch_write_command_insert.js +++ b/jstests/core/batch_write_command_insert.js @@ -98,22 +98,6 @@ 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 3568103e768..e1c8f390f72 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] + * @tags: [assumes_unsharded_collection, requires_fcv_46] */ (function() { "use strict"; @@ -31,15 +31,14 @@ 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 in embedded objects. +// Test that $-prefixed field names are allowed. 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 $-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. +// Test that reserved $-prefixed field names are 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); @@ -53,6 +52,15 @@ 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. // @@ -75,18 +83,37 @@ 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 not allowed. -assert.writeErrorWithCode(coll.update({"a.b": 1}, {$c: 1}, {upsert: true}), +// $-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}), 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); -// 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); +// 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); // // FindAndModify field name validation. @@ -108,13 +135,15 @@ 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()); -// $-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}}}); -}); +// 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()); // Reserved $-prefixed field names are also not allowed. assert.throws(function() { diff --git a/jstests/core/insert2.js b/jstests/core/insert2.js deleted file mode 100644 index 6052f1eeb8d..00000000000 --- a/jstests/core/insert2.js +++ /dev/null @@ -1,21 +0,0 @@ -// 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 347f230a93d..05a64b88f12 100644 --- a/jstests/core/push_sort.js +++ b/jstests/core/push_sort.js @@ -1,4 +1,4 @@ -// @tags: [requires_non_retryable_writes] +// @tags: [requires_non_retryable_writes, requires_fcv_46] // // $push acquired the possibility of sorting the resulting array as part of SERVER-8008. This @@ -54,12 +54,13 @@ assert.eq([{a: {b: 2}}, {a: {b: 3}}], t.findOne({_id: 7}).x); // Invalid Cases // -// $push with $sort should not push a "$sort" field +// 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. var doc8 = {_id: 8, x: [{a: 1}, {a: 2}]}; t.save(doc8); var res = t.update({_id: 8}, {$push: {x: {$sort: {a: -1}}}}); -assert.writeErrorWithCode(res, ErrorCodes.DollarPrefixedFieldName); -assert.docEq(t.findOne({_id: 8}), doc8); // ensure doc was not changed +assert.commandWorked(res); +assert.docEq(t.findOne({_id: 8}), {_id: 8, x: [{a: 1}, {a: 2}, {$sort: {a: -1}}]}); t.save({_id: 100, x: [{a: 1}]}); diff --git a/jstests/core/update_addToSet.js b/jstests/core/update_addToSet.js index 5927dd882f8..7e9b262591c 100644 --- a/jstests/core/update_addToSet.js +++ b/jstests/core/update_addToSet.js @@ -71,28 +71,6 @@ 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 4f044a19352..891d9feb83e 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] + * @tags: [requires_find_command, requires_non_retryable_writes, requires_fcv_46] */ (function() { "use strict"; @@ -195,8 +195,9 @@ assert.commandFailedWithCode( }]), ErrorCodes.InvalidOptions); -// Update fails when supported agg stage is specified outside of pipeline. -assert.commandFailedWithCode(coll.update({_id: 1}, {$addFields: {x: 1}}), ErrorCodes.FailedToParse); +// 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()); // The 'arrayFilters' option is not valid for pipeline updates. assert.commandFailedWithCode( @@ -254,12 +255,12 @@ testUpdate({ nModified: 1 }); -// Cannot use expressions in constants. -assert.commandFailedWithCode(db.runCommand({ +// Test that expressions within constants are treated as field names instead of expressions. +db.runCommand({ update: collName, updates: [{q: {_id: 1}, u: [{$set: {x: "$$foo"}}], c: {foo: {$add: [1, 2]}}}] -}), - ErrorCodes.DollarPrefixedFieldName); +}); +assert.eq([{_id: 1, x: {$add: [1, 2]}, foo: "$x"}], coll.find({_id: 1}).toArray()); // Cannot use constants with regular updates. assert.commandFailedWithCode( diff --git a/jstests/core/updateh.js b/jstests/core/updateh.js index 1fd3d62750d..e647fb4bfb8 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] +// @tags: [assumes_unsharded_collection, requires_fcv_46] -// Disallow $ in field names +// Allow $ in field names (as of SERVER-49117) var res; t = db.jstest_updateh; @@ -11,39 +11,39 @@ t.drop(); t.insert({x: 1}); -res = t.update({x: 1}, {$set: {y: 1}}); // ok +res = t.update({x: 1}, {$set: {y: 1}}); assert.commandWorked(res); -res = t.update({x: 1}, {$set: {$z: 1}}); // not ok -assert.writeError(res); +res = t.update({x: 1}, {$set: {$z: 1}}); +assert.commandWorked(res); -res = t.update({x: 1}, {$set: {'a.$b': 1}}); // not ok -assert.writeError(res); +res = t.update({x: 1}, {$set: {'a.$b': 1}}); +assert.commandWorked(res); -res = t.update({x: 1}, {$inc: {$z: 1}}); // not ok -assert.writeError(res); +res = t.update({x: 1}, {$inc: {$z: 1}}); +assert.commandWorked(res); // Second section t.drop(); t.save({_id: 0, n: 0}); -// Test that '$' cannot be the first character in a field. -// SERVER-7150 +// Test that '$' can be the first character in a field. +// (as of SERVER-49117) res = t.update({n: 0}, {$set: {$x: 1}}); -assert.writeError(res); +assert.commandWorked(res); res = t.update({n: 0}, {$set: {$$$: 1}}); -assert.writeError(res); +assert.commandWorked(res); res = t.update({n: 0}, {$set: {"sneaky.$x": 1}}); -assert.writeError(res); +assert.commandWorked(res); res = t.update({n: 0}, {$set: {"secret.agent$.$x": 1}}); -assert.writeError(res); +assert.commandWorked(res); res = t.update({n: 0}, {$set: {"$secret.agent.x": 1}}); -assert.writeError(res); +assert.commandWorked(res); res = t.update({n: 0}, {$set: {"secret.agent$": 1}}); assert.commandWorked(res); @@ -60,14 +60,16 @@ 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.writeError(res); +assert.commandWorked(res); res = t.update({n: 0}, {$set: {$idbar: 1}}); -assert.writeError(res); +assert.commandWorked(res); res = t.update({n: 0}, {$set: {$dbbaz: 1}}); -assert.writeError(res); +assert.commandWorked(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 49735bee7d6..100322a00e4 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"}); -printjson(result = coll.update({foo: "bar"}, {$invalid: "expr"})); +result = coll.update({foo: "bar"}, {_id: /a/}); 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 21a42381c96..38ce0f839de 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}, {$invalid: "xxx"}, true); +coll.update({_id: 1}, {$set: {x: 12}, $hello: 1}, 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}, {$invalid: "xxx"}, true); +coll.update({_id: 1}, {$set: {x: 12}, $hello: 1}, 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 62a513242ca..4f83d7e4b6e 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: 2, $invalidField: 4}}, {multi: false}), - ErrorCodes.DollarPrefixedFieldName); + testDB.coll.update({_id: 1}, {$set: {x: 12}, $hello: 1}, {multi: false}), + ErrorCodes.FailedToParse); let mongosServerStatus = testDB.adminCommand({serverStatus: 1}); 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) { diff --git a/src/mongo/dbtests/updatetests.cpp b/src/mongo/dbtests/updatetests.cpp index f37dc9adb9b..cfb0ba85311 100644 --- a/src/mongo/dbtests/updatetests.cpp +++ b/src/mongo/dbtests/updatetests.cpp @@ -112,12 +112,6 @@ class ModNonmodMix : public Fail { } }; -class InvalidMod : public Fail { - void doIt() { - update(ns(), BSONObj(), fromjson("{$awk:{a:4}}")); - } -}; - class ModNotFirst : public Fail { void doIt() { update(ns(), BSONObj(), fromjson("{z:3,$set:{a:4}}")); @@ -136,20 +130,6 @@ class IncNonNumber : public Fail { } }; -class PushAllNonArray : public Fail { - void doIt() { - insert(ns(), fromjson("{a:[1]}")); - update(ns(), BSONObj(), fromjson("{$pushAll:{a:'d'}}")); - } -}; - -class PullAllNonArray : public Fail { - void doIt() { - insert(ns(), fromjson("{a:[1]}")); - update(ns(), BSONObj(), fromjson("{$pullAll:{a:'d'}}")); - } -}; - class IncTargetNonNumber : public Fail { void doIt() { insert(ns(), @@ -2004,12 +1984,9 @@ public: void setupTests() { add<ModId>(); add<ModNonmodMix>(); - add<InvalidMod>(); add<ModNotFirst>(); add<ModDuplicateFieldSpec>(); add<IncNonNumber>(); - add<PushAllNonArray>(); - add<PullAllNonArray>(); add<IncTargetNonNumber>(); add<SetNum>(); add<SetString>(); diff --git a/src/mongo/embedded/stitch_support/stitch_support_test.cpp b/src/mongo/embedded/stitch_support/stitch_support_test.cpp index d8984f43367..7c6dc94cb62 100644 --- a/src/mongo/embedded/stitch_support/stitch_support_test.cpp +++ b/src/mongo/embedded/stitch_support/stitch_support_test.cpp @@ -575,10 +575,6 @@ TEST_F(StitchSupportTest, TestUpdateWithSetOnInsert) { } TEST_F(StitchSupportTest, TestUpdateProducesProperStatus) { - ASSERT_EQ( - "Unknown modifier: $bogus. Expected a valid update modifier or pipeline-style update " - "specified as an array", - checkUpdateStatus("{$bogus: {a: 2}}", "{a: 1}")); ASSERT_EQ("Updating the path 'a' would create a conflict at 'a'", checkUpdateStatus("{$set: {a: 2, a: 3}}", "{a: 1}")); ASSERT_EQ("No array filter found for identifier 'i' in path 'a.$[i]'", |