SERVER-28739 make the js scripting engine correctly verify that accessed collections are unsharded (uassert rather than invariant)

3 files changed, 15 insertions, 12 deletions

diff --git a/src/mongo/db/commands/eval.cpp b/src/mongo/db/commands/eval.cpp
index caef4ff25d3..1fd7c72342e 100644
--- a/src/mongo/db/commands/eval.cpp
+++ b/src/mongo/db/commands/eval.cpp
@@ -181,17 +181,18 @@ public:
         // Note: 'eval' is not allowed to touch sharded namespaces, but we can't check the
         // shardVersions of the namespaces accessed in the script until the script is evaluated.
         // Instead, we enforce that the script does not access sharded namespaces by ensuring the
-        // shardVersion is set to UNSHARDED on the OperationContext here. If a shardVersion is set
-        // on the OperationContext, a check for a different namespace will default to UNSHARDED.
+        // shardVersion is set to UNSHARDED on the OperationContext before sending the script to be
+        // evaluated.
         auto& oss = OperationShardingState::get(opCtx);
-        if (oss.hasShardVersion()) {
-            // Can't set the shardVersion if it's already been set, so just verify it.
-            invariant(oss.getShardVersion(NamespaceString(dbname))
-                          .isStrictlyEqualTo(ChunkVersion::UNSHARDED()));
-        } else {
-            // Set the shardVersion to UNSHARDED. The "namespace" used does not matter.
-            oss.setShardVersion(NamespaceString(dbname), ChunkVersion::UNSHARDED());
-        }
+        uassert(ErrorCodes::IllegalOperation,
+                "can't send a shardVersion with the 'eval' command, since you can't use sharded "
+                "collections from 'eval'",
+                !oss.hasShardVersion());
+
+        // Set the shardVersion to UNSHARDED. The "namespace" used does not matter, because if a
+        // shardVersion is set on the OperationContext, a check for a different namespace will
+        // default to UNSHARDED.
+        oss.setShardVersion(NamespaceString(dbname), ChunkVersion::UNSHARDED());
 
         try {
             if (cmdObj["nolock"].trueValue()) {
diff --git a/src/mongo/db/s/collection_sharding_state_test.cpp b/src/mongo/db/s/collection_sharding_state_test.cpp
index a77937d35f0..f1cecac1979 100644
--- a/src/mongo/db/s/collection_sharding_state_test.cpp
+++ b/src/mongo/db/s/collection_sharding_state_test.cpp
@@ -63,8 +63,6 @@ public:
         // TODO(esha): remove once the Safe Secondary Reads (PM-256) project is complete.
         auto svCtx = getServiceContext();
         repl::ReplSettings replSettings;
-        replSettings.setReplSetString(
-            ConnectionString::forReplicaSet(_setName, _servers).toString());
         replSettings.setMaster(true);
         repl::ReplicationCoordinator::set(
             svCtx, stdx::make_unique<repl::ReplicationCoordinatorMock>(svCtx, replSettings));
diff --git a/src/mongo/db/s/set_shard_version_command.cpp b/src/mongo/db/s/set_shard_version_command.cpp
index e7078cea3d3..178a22596d2 100644
--- a/src/mongo/db/s/set_shard_version_command.cpp
+++ b/src/mongo/db/s/set_shard_version_command.cpp
@@ -93,6 +93,10 @@ public:
              BSONObj& cmdObj,
              string& errmsg,
              BSONObjBuilder& result) {
+        uassert(ErrorCodes::IllegalOperation,
+                "can't issue setShardVersion from 'eval'",
+                !opCtx->getClient()->isInDirectClient());
+
         auto shardingState = ShardingState::get(opCtx);
         uassertStatusOK(shardingState->canAcceptShardedCommands());