SERVER-31953 Target secondary application of updates and deletes by just _id

(cherry picked from commit b6235c8418faf541af93e9caacc22b7ed9aed817)

2 files changed, 44 insertions, 6 deletions

diff --git a/jstests/sharding/replication_with_undefined_shard_key.js b/jstests/sharding/replication_with_undefined_shard_key.js
new file mode 100644
index 00000000000..8e37c171735
--- /dev/null
+++ b/jstests/sharding/replication_with_undefined_shard_key.js
@@ -0,0 +1,30 @@
+// Test for SERVER-31953 where secondaries crash when replicating an oplog entry where the document
+// identifier in the oplog entry contains a shard key value that contains an undefined value.
+(function() {
+    "use strict";
+
+    const st = new ShardingTest({mongos: 1, config: 1, shard: 1, rs: {nodes: 2}});
+    const mongosDB = st.s.getDB("test");
+    const mongosColl = mongosDB.mycoll;
+
+    // Shard the test collection on the "x" field.
+    assert.commandWorked(mongosDB.adminCommand({enableSharding: mongosDB.getName()}));
+    assert.commandWorked(mongosDB.adminCommand({
+        shardCollection: mongosColl.getFullName(),
+        key: {x: 1},
+    }));
+
+    // Insert a document with a literal undefined value.
+    assert.writeOK(mongosColl.insert({x: undefined}));
+
+    jsTestLog("Doing writes that generate oplog entries including undefined document key");
+
+    assert.writeOK(mongosColl.update(
+        {},
+        {$set: {a: 1}},
+        {multi: true, writeConcern: {w: 2, wtimeout: ReplSetTest.kDefaultTimeoutMs}}));
+    assert.writeOK(
+        mongosColl.remove({}, {writeConcern: {w: 2, wtimeout: ReplSetTest.kDefaultTimeoutMs}}));
+
+    st.stop();
+})();
\ No newline at end of file
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 909cf659fa8..c849a450f26 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -1297,12 +1297,15 @@ Status applyOperation_inlock(OperationContext* opCtx,
     } else if (*opType == 'u') {
         opCounters->gotUpdate();
 
-        BSONObj updateCriteria = o2;
-        const bool upsert = valueB || alwaysUpsert;
-
+        auto idField = o2["_id"];
         uassert(ErrorCodes::NoSuchKey,
                 str::stream() << "Failed to apply update due to missing _id: " << op.toString(),
-                updateCriteria.hasField("_id"));
+                !idField.eoo());
+
+        // The o2 field may contain additional fields besides the _id (like the shard key fields),
+        // but we want to do the update by just _id so we can take advantage of the IDHACK.
+        BSONObj updateCriteria = idField.wrap();
+        const bool upsert = valueB || alwaysUpsert;
 
         UpdateRequest request(requestNss);
         request.setQuery(updateCriteria);
@@ -1379,9 +1382,14 @@ Status applyOperation_inlock(OperationContext* opCtx,
     } else if (*opType == 'd') {
         opCounters->gotDelete();
 
+        auto idField = o["_id"];
         uassert(ErrorCodes::NoSuchKey,
                 str::stream() << "Failed to apply delete due to missing _id: " << op.toString(),
-                o.hasField("_id"));
+                !idField.eoo());
+
+        // The o field may contain additional fields besides the _id (like the shard key fields),
+        // but we want to do the delete by just _id so we can take advantage of the IDHACK.
+        BSONObj deleteCriteria = idField.wrap();
 
         SnapshotName timestamp;
         if (assignOperationTimestamp) {
@@ -1396,7 +1404,7 @@ Status applyOperation_inlock(OperationContext* opCtx,
             }
 
             if (opType[1] == 0) {
-                deleteObjects(opCtx, collection, requestNss, o, /*justOne*/ valueB);
+                deleteObjects(opCtx, collection, requestNss, deleteCriteria, /*justOne*/ valueB);
             } else
                 verify(opType[1] == 'b');  // "db" advertisement
             wuow.commit();