From 3a20a63dc5e96948131dc0393c07a644f49e8109 Mon Sep 17 00:00:00 2001
From: Haley Connelly <haley.connelly@mongodb.com>
Date: Wed, 1 Jul 2020 18:08:05 +0000
Subject: SERVER-46811 multi=true updates can modify the shard key of orphan
 documents and cause them to become owned

(cherry picked from commit 17e468583ec1a7e9755b34b50eaa7cf017a1c96e)
---
 jstests/sharding/multi_update_orphan_shard_key.js | 60 +++++++++++++++++++++++
 src/mongo/db/exec/update_stage.cpp                | 13 +++--
 2 files changed, 70 insertions(+), 3 deletions(-)
 create mode 100644 jstests/sharding/multi_update_orphan_shard_key.js

diff --git a/jstests/sharding/multi_update_orphan_shard_key.js b/jstests/sharding/multi_update_orphan_shard_key.js
new file mode 100644
index 00000000000..6fec3a1653f
--- /dev/null
+++ b/jstests/sharding/multi_update_orphan_shard_key.js
@@ -0,0 +1,60 @@
+/**
+ * Tests it isn't possible to update an orphan document's shard key. Only multi=true updates skip
+ * shard versioning. They are therefore the only case which skips ownership filtering.
+ * @tags: [requires_fcv_44]
+ */
+(function() {
+"use strict";
+
+load("jstests/libs/fail_point_util.js");
+
+const st = new ShardingTest({mongos: 1, config: 1, shards: 2, rs: {nodes: 1}});
+const dbName = "test";
+const collName = "update_orphan_shard_key";
+const collection = st.s.getDB(dbName).getCollection(collName);
+
+// Create a sharded collection with two chunks on shard0, split at the key {x: -1}.
+assert.commandWorked(st.s.adminCommand({enableSharding: dbName}));
+assert.commandWorked(st.s.adminCommand({movePrimary: dbName, to: st.shard0.shardName}));
+assert.commandWorked(st.s.adminCommand({shardCollection: collection.getFullName(), key: {x: 1}}));
+assert.commandWorked(st.s.adminCommand({split: collection.getFullName(), middle: {x: -1}}));
+
+// Insert some documents into the collection, but only into the higher of the two chunks.
+assert.commandWorked(collection.insert(Array.from({length: 100}, (_, i) => ({_id: i, x: i}))));
+
+// Enable the failpoint to cause range deletion to hang indefinitely.
+const suspendRangeDeletionFailpoint = configureFailPoint(st.shard0, "suspendRangeDeletion");
+
+// Note: Use _waitForDelete=false to ensure the command completes since the test intentionally
+// causes range deletion to hang.
+assert.commandWorked(st.s.adminCommand({
+    moveChunk: collection.getFullName(),
+    find: {x: 1},
+    to: st.shard1.shardName,
+    _waitForDelete: false,
+}));
+
+let res = assert.commandWorked(collection.update({x: 0}, {$set: {y: 1}}));
+assert.eq(1, res.nMatched, res);
+assert.eq(1, res.nModified, res);
+
+// Do a multi=true update that will target both shards but not update any documents on the shard
+// which owns the range [-1, MaxKey].
+res = assert.commandFailedWithCode(
+    collection.update({x: {$lte: 0}, y: {$exists: false}}, {$set: {x: -10, y: 2}}, {multi: true}),
+    31025);
+assert.eq(0, res.nMatched, res);
+assert.eq(0, res.nModified, res);
+assert.eq(0, res.nUpserted, res);
+
+// Wait for range deletion to happen on the donor.
+suspendRangeDeletionFailpoint.off();
+assert.soon(() => {
+    return st.shard0.getCollection(collection.getFullName()).findOne({x: 1}) === null;
+});
+
+// Run a $out aggregation as a simple way to check for duplicate _id values.
+assert.doesNotThrow(() => collection.aggregate([{$out: "output"}]));
+
+st.stop();
+})();
diff --git a/src/mongo/db/exec/update_stage.cpp b/src/mongo/db/exec/update_stage.cpp
index ffd6700c926..1a57e80b856 100644
--- a/src/mongo/db/exec/update_stage.cpp
+++ b/src/mongo/db/exec/update_stage.cpp
@@ -708,9 +708,10 @@ bool UpdateStage::checkUpdateChangesShardKeyFields(ScopedCollectionDescription c
     auto oldShardKey = shardKeyPattern.extractShardKeyFromDoc(oldObj.value());
     auto newShardKey = shardKeyPattern.extractShardKeyFromDoc(newObj);
 
-    // If the shard key fields remain unchanged by this update or if this document is an orphan and
-    // so does not belong to this shard, we can skip the rest of the checks.
-    if ((newShardKey.woCompare(oldShardKey) == 0) || !collDesc->keyBelongsToMe(oldShardKey)) {
+    // If the shard key fields remain unchanged by this update we can skip the rest of the checks.
+    // Using BSONObj::binaryEqual() still allows a missing shard key field to be filled in with an
+    // explicit null value.
+    if (newShardKey.binaryEqual(oldShardKey)) {
         return false;
     }
 
@@ -748,6 +749,12 @@ bool UpdateStage::checkUpdateChangesShardKeyFields(ScopedCollectionDescription c
             "retryWrites: true.",
             opCtx()->getTxnNumber() || !opCtx()->writesAreReplicated());
 
+    // If the shard key of an orphan document is allowed to change, and the document is allowed to
+    // become owned by the shard, the global uniqueness assumption for _id values would be violated.
+    uassert(468110,
+            "Changing the shard key of an orphan document is not allowed",
+            collDesc->keyBelongsToMe(oldShardKey));
+
     if (!collDesc->keyBelongsToMe(newShardKey)) {
         if (MONGO_unlikely(hangBeforeThrowWouldChangeOwningShard.shouldFail())) {
             LOGV2(20605, "Hit hangBeforeThrowWouldChangeOwningShard failpoint");
-- 
cgit v1.2.1