SERVER-28739 make the js scripting engine correctly verify that accessed collections are unsharded

22 files changed, 89 insertions, 193 deletions

diff --git a/jstests/core/eval1.js b/jstests/core/eval1.js
index 8b139cae02a..237bd4bb020 100644
--- a/jstests/core/eval1.js
+++ b/jstests/core/eval1.js
@@ -1,3 +1,6 @@
+// Cannot implicitly shard accessed collections because unsupported use of sharded collection
+// from db.eval.
+// @tags: [assumes_unsharded_collection]
 
 t = db.eval1;
 t.drop();
diff --git a/jstests/core/eval5.js b/jstests/core/eval5.js
index 46bd679dd77..3b367f00fe3 100644
--- a/jstests/core/eval5.js
+++ b/jstests/core/eval5.js
@@ -1,3 +1,6 @@
+// Cannot implicitly shard accessed collections because unsupported use of sharded collection
+// from db.eval.
+// @tags: [assumes_unsharded_collection]
 
 t = db.eval5;
 t.drop();
diff --git a/jstests/core/eval6.js b/jstests/core/eval6.js
index 31258f6917b..fdb72d96a9a 100644
--- a/jstests/core/eval6.js
+++ b/jstests/core/eval6.js
@@ -1,3 +1,6 @@
+// Cannot implicitly shard accessed collections because unsupported use of sharded collection
+// from db.eval.
+// @tags: [assumes_unsharded_collection]
 
 t = db.eval6;
 t.drop();
diff --git a/jstests/core/eval9.js b/jstests/core/eval9.js
index 1480ff6519c..3338a34fc60 100644
--- a/jstests/core/eval9.js
+++ b/jstests/core/eval9.js
@@ -1,3 +1,7 @@
+// Cannot implicitly shard accessed collections because unsupported use of sharded collection
+// from db.eval.
+// @tags: [assumes_unsharded_collection]
+
 assert.writeOK(db.evalprep.insert({}), "db must exist for eval to succeed");
 db.evalprep.drop();
 
diff --git a/jstests/core/eval_nolock.js b/jstests/core/eval_nolock.js
index 9511784becb..979a6ddc6dc 100644
--- a/jstests/core/eval_nolock.js
+++ b/jstests/core/eval_nolock.js
@@ -1,3 +1,6 @@
+// Cannot implicitly shard accessed collections because unsupported use of sharded collection
+// from db.eval.
+// @tags: [assumes_unsharded_collection]
 
 t = db.eval_nolock;
 t.drop();
diff --git a/jstests/core/evala.js b/jstests/core/evala.js
index 7ccf33ac754..8ee0a3bc05f 100644
--- a/jstests/core/evala.js
+++ b/jstests/core/evala.js
@@ -1,3 +1,6 @@
+// Cannot implicitly shard accessed collections because unsupported use of sharded collection
+// from db.eval.
+// @tags: [assumes_unsharded_collection]
 
 t = db.evala;
 t.drop();
diff --git a/jstests/core/evalc.js b/jstests/core/evalc.js
index 0d55790afe3..60511295b87 100644
--- a/jstests/core/evalc.js
+++ b/jstests/core/evalc.js
@@ -1,3 +1,7 @@
+// Cannot implicitly shard accessed collections because unsupported use of sharded collection
+// from db.eval.
+// @tags: [assumes_unsharded_collection]
+
 t = db.jstests_evalc;
 t.drop();
 
diff --git a/jstests/core/evale.js b/jstests/core/evale.js
index 1ddc8519fc6..59dce866935 100644
--- a/jstests/core/evale.js
+++ b/jstests/core/evale.js
@@ -1,3 +1,7 @@
+// Cannot implicitly shard accessed collections because unsupported use of sharded collection
+// from db.eval.
+// @tags: [assumes_unsharded_collection]
+
 t = db.jstests_evale;
 t.drop();
 
@@ -8,4 +12,4 @@ db.eval(function() {
         }
     });
 });
-db.eval("db.jstests_evale.count( { $where:function() { return true; } } )");
\ No newline at end of file
+db.eval("db.jstests_evale.count( { $where:function() { return true; } } )");
diff --git a/jstests/core/evalg.js b/jstests/core/evalg.js
index 18503659217..0ee0e230264 100644
--- a/jstests/core/evalg.js
+++ b/jstests/core/evalg.js
@@ -1,3 +1,7 @@
+// Cannot implicitly shard accessed collections because unsupported use of sharded collection
+// from db.eval.
+// @tags: [assumes_unsharded_collection]
+
 // SERVER-17499: Test behavior of getMore on aggregation cursor under eval command.
 db.evalg.drop();
 for (var i = 0; i < 102; ++i) {
diff --git a/src/mongo/db/assemble_response.cpp b/src/mongo/db/assemble_response.cpp
index 7aed5ce0f01..1c4444a2ce4 100644
--- a/src/mongo/db/assemble_response.cpp
+++ b/src/mongo/db/assemble_response.cpp
@@ -315,7 +315,7 @@ void receivedQuery(OperationContext* opCtx,
         dbResponse.exhaustNS = runQuery(opCtx, q, nss, dbResponse.response);
     } catch (const AssertionException& e) {
         // If we got a stale config, wait in case the operation is stuck in a critical section
-        if (e.getCode() == ErrorCodes::SendStaleConfig) {
+        if (!opCtx->getClient()->isInDirectClient() && e.getCode() == ErrorCodes::SendStaleConfig) {
             auto& sce = static_cast<const StaleConfigException&>(e);
             ShardingState::get(opCtx)->onStaleShardVersion(
                 opCtx, NamespaceString(sce.getns()), sce.getVersionReceived());
diff --git a/src/mongo/db/commands/dbcommands.cpp b/src/mongo/db/commands/dbcommands.cpp
index 5e6fef242fc..f3dfa5856fd 100644
--- a/src/mongo/db/commands/dbcommands.cpp
+++ b/src/mongo/db/commands/dbcommands.cpp
@@ -1850,8 +1850,10 @@ void mongo::execCommandDatabase(OperationContext* opCtx,
             auto sce = dynamic_cast<const StaleConfigException*>(&e);
             invariant(sce);  // do not upcasts from DBException created by uassert variants.
 
-            ShardingState::get(opCtx)->onStaleShardVersion(
-                opCtx, NamespaceString(sce->getns()), sce->getVersionReceived());
+            if (!opCtx->getClient()->isInDirectClient()) {
+                ShardingState::get(opCtx)->onStaleShardVersion(
+                    opCtx, NamespaceString(sce->getns()), sce->getVersionReceived());
+            }
         }
 
         BSONObjBuilder metadataBob;
diff --git a/src/mongo/db/commands/eval.cpp b/src/mongo/db/commands/eval.cpp
index fd9cb29f940..caef4ff25d3 100644
--- a/src/mongo/db/commands/eval.cpp
+++ b/src/mongo/db/commands/eval.cpp
@@ -43,6 +43,8 @@
 #include "mongo/db/jsobj.h"
 #include "mongo/db/json.h"
 #include "mongo/db/operation_context.h"
+#include "mongo/db/s/operation_sharding_state.h"
+#include "mongo/s/stale_exception.h"
 #include "mongo/scripting/engine.h"
 #include "mongo/util/log.h"
 
@@ -176,15 +178,40 @@ public:
              BSONObj& cmdObj,
              string& errmsg,
              BSONObjBuilder& result) {
-        if (cmdObj["nolock"].trueValue()) {
-            return dbEval(opCtx, dbname, cmdObj, result, errmsg);
+        // 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.
+        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());
         }
 
-        Lock::GlobalWrite lk(opCtx);
+        try {
+            if (cmdObj["nolock"].trueValue()) {
+                return dbEval(opCtx, dbname, cmdObj, result, errmsg);
+            }
+
+            Lock::GlobalWrite lk(opCtx);
 
-        OldClientContext ctx(opCtx, dbname, false /* no shard version checking */);
+            OldClientContext ctx(opCtx, dbname, false /* no shard version checking here */);
 
-        return dbEval(opCtx, dbname, cmdObj, result, errmsg);
+            return dbEval(opCtx, dbname, cmdObj, result, errmsg);
+        } catch (const UserException& ex) {
+            // Convert a stale shardVersion error to a stronger error to prevent this node or the
+            // sending node from believing it needs to refresh its routing table.
+            if (ex.getCode() == ErrorCodes::RecvStaleConfig) {
+                uasserted(ErrorCodes::BadValue,
+                          str::stream() << "can't use sharded collection from db.eval");
+            }
+            throw ex;
+        }
     }
 
 } cmdeval;
diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp
index d30992c3f8e..1e55790361a 100644
--- a/src/mongo/db/ops/write_ops_exec.cpp
+++ b/src/mongo/db/ops/write_ops_exec.cpp
@@ -221,8 +221,10 @@ bool handleError(OperationContext* opCtx,
                                    << demangleName(typeid(ex)));
         }
 
-        ShardingState::get(opCtx)->onStaleShardVersion(
-            opCtx, wholeOp.ns, staleConfigException->getVersionReceived());
+        if (!opCtx->getClient()->isInDirectClient()) {
+            ShardingState::get(opCtx)->onStaleShardVersion(
+                opCtx, wholeOp.ns, staleConfigException->getVersionReceived());
+        }
         out->staleConfigException =
             stdx::make_unique<SendStaleConfigException>(*staleConfigException);
         return false;
diff --git a/src/mongo/db/s/collection_sharding_state.cpp b/src/mongo/db/s/collection_sharding_state.cpp
index 24746d7880e..5b4b39cb33f 100644
--- a/src/mongo/db/s/collection_sharding_state.cpp
+++ b/src/mongo/db/s/collection_sharding_state.cpp
@@ -296,11 +296,6 @@ bool CollectionShardingState::_checkShardVersionOk(OperationContext* opCtx,
                                                    ChunkVersion* actualShardVersion) {
     Client* client = opCtx->getClient();
 
-    // Operations using the DBDirectClient are unversioned.
-    if (client->isInDirectClient()) {
-        return true;
-    }
-
     if (!repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesForDatabase(opCtx, _nss.db())) {
         // Right now connections to secondaries aren't versioned at all.
         return true;
diff --git a/src/mongo/db/s/collection_sharding_state_test.cpp b/src/mongo/db/s/collection_sharding_state_test.cpp
index 51666a7800f..a77937d35f0 100644
--- a/src/mongo/db/s/collection_sharding_state_test.cpp
+++ b/src/mongo/db/s/collection_sharding_state_test.cpp
@@ -34,6 +34,8 @@
 #include "mongo/db/jsobj.h"
 #include "mongo/db/namespace_string.h"
 #include "mongo/db/operation_context_noop.h"
+#include "mongo/db/repl/replication_coordinator.h"
+#include "mongo/db/repl/replication_coordinator_mock.h"
 #include "mongo/db/s/collection_metadata.h"
 #include "mongo/db/s/collection_sharding_state.h"
 #include "mongo/db/s/sharding_state.h"
@@ -55,10 +57,18 @@ public:
 
         serverGlobalParams.clusterRole = ClusterRole::ShardServer;
         _client = _service.makeClient("ShardingStateTest");
-        // This skips version checking to avoid accessing a null ReplicationCoordinator.
-        _client->setInDirectClient(true);
         _opCtx = _client->makeOperationContext();
 
+        // Set a ReplicationCoordinator, since it is accessed as part of shardVersion checks.
+        // 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));
+
         // Note: this assumes that globalInit will always be called on the same thread as the main
         // test thread.
         ShardingState::get(opCtx())->setGlobalInitMethodForTest(
@@ -90,6 +100,9 @@ private:
     ServiceContext::UniqueOperationContext _opCtx;
 
     int _initCallCount = 0;
+    const HostAndPort _host{"node1:12345"};
+    const std::string _setName = "mySet";
+    const std::vector<HostAndPort> _servers{_host};
 };
 
 TEST_F(CollShardingStateTest, GlobalInitGetsCalledAfterWriteCommits) {
diff --git a/src/mongo/db/s/sharding_state.cpp b/src/mongo/db/s/sharding_state.cpp
index 2646af582dd..0b3bc8bd9fb 100644
--- a/src/mongo/db/s/sharding_state.cpp
+++ b/src/mongo/db/s/sharding_state.cpp
@@ -63,7 +63,6 @@
 #include "mongo/s/client/shard_registry.h"
 #include "mongo/s/client/sharding_network_connection_hook.h"
 #include "mongo/s/grid.h"
-#include "mongo/s/local_sharding_info.h"
 #include "mongo/s/sharding_initialization.h"
 #include "mongo/util/log.h"
 #include "mongo/util/mongoutils/str.h"
@@ -111,30 +110,6 @@ void updateShardIdentityConfigStringCB(const string& setName, const string& newC
     }
 }
 
-bool haveLocalShardingInfo(OperationContext* opCtx, const string& ns) {
-    if (!ShardingState::get(opCtx)->enabled()) {
-        return false;
-    }
-
-    const auto& oss = OperationShardingState::get(opCtx);
-    if (oss.hasShardVersion()) {
-        return true;
-    }
-
-    const auto& sci = ShardedConnectionInfo::get(opCtx->getClient(), false);
-    if (sci && !sci->getVersion(ns).isStrictlyEqualTo(ChunkVersion::UNSHARDED())) {
-        return true;
-    }
-
-    return false;
-}
-
-MONGO_INITIALIZER_WITH_PREREQUISITES(MongoDLocalShardingInfo, ("SetGlobalEnvironment"))
-(InitializerContext* context) {
-    enableLocalShardingInfo(getGlobalServiceContext(), &haveLocalShardingInfo);
-    return Status::OK();
-}
-
 }  // namespace
 
 ShardingState::ShardingState()
@@ -248,6 +223,7 @@ void ShardingState::scheduleCleanup(const NamespaceString& nss) {
 Status ShardingState::onStaleShardVersion(OperationContext* opCtx,
                                           const NamespaceString& nss,
                                           const ChunkVersion& expectedVersion) {
+    invariant(!opCtx->getClient()->isInDirectClient());
     invariant(!opCtx->lockState()->isLocked());
     invariant(enabled());
 
diff --git a/src/mongo/s/SConscript b/src/mongo/s/SConscript
index 70633d8e6ca..c5c7e32c73d 100644
--- a/src/mongo/s/SConscript
+++ b/src/mongo/s/SConscript
@@ -330,13 +330,3 @@ env.CppUnitTest(
         'sharding_test_fixture',
     ]
 )
-
-env.Library(
-    target='local_sharding_info',
-    source=[
-        'local_sharding_info.cpp',
-    ],
-    LIBDEPS=[
-        '$BUILD_DIR/mongo/db/service_context',
-    ],
-)
diff --git a/src/mongo/s/local_sharding_info.cpp b/src/mongo/s/local_sharding_info.cpp
deleted file mode 100644
index 7b8926bac65..00000000000
--- a/src/mongo/s/local_sharding_info.cpp
+++ /dev/null
@@ -1,71 +0,0 @@
-/*
- *    Copyright (C) 2016 10gen Inc.
- *
- *    This program is free software: you can redistribute it and/or  modify
- *    it under the terms of the GNU Affero General Public License, version 3,
- *    as published by the Free Software Foundation.
- *
- *    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
- *    GNU Affero General Public License for more details.
- *
- *    You should have received a copy of the GNU Affero General Public License
- *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- *    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 GNU Affero General 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.
- */
-
-#include "mongo/platform/basic.h"
-
-#include "mongo/s/local_sharding_info.h"
-
-#include "mongo/db/operation_context.h"
-#include "mongo/db/service_context.h"
-
-namespace mongo {
-namespace {
-
-using Handler = stdx::function<bool(OperationContext*, const std::string&)>;
-
-class LocalShardingInfo {
-public:
-    void registerHandler(Handler handler) {
-        _handler = handler;
-    }
-
-    const Handler& getHandler() const {
-        return _handler;
-    }
-
-private:
-    Handler _handler;
-};
-
-const ServiceContext::Decoration<LocalShardingInfo> forService =
-    ServiceContext::declareDecoration<LocalShardingInfo>();
-
-}  // namespace
-
-void enableLocalShardingInfo(ServiceContext* context, Handler handler) {
-    forService(context).registerHandler(handler);
-}
-
-bool haveLocalShardingInfo(OperationContext* opCtx, const std::string& ns) {
-    auto handler = forService(opCtx->getServiceContext()).getHandler();
-    if (handler)
-        return handler(opCtx, ns);
-    return false;
-}
-
-}  // namespace mongo
diff --git a/src/mongo/s/local_sharding_info.h b/src/mongo/s/local_sharding_info.h
deleted file mode 100644
index 9be721de42b..00000000000
--- a/src/mongo/s/local_sharding_info.h
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- *    Copyright (C) 2016 10gen Inc.
- *
- *    This program is free software: you can redistribute it and/or  modify
- *    it under the terms of the GNU Affero General Public License, version 3,
- *    as published by the Free Software Foundation.
- *
- *    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
- *    GNU Affero General Public License for more details.
- *
- *    You should have received a copy of the GNU Affero General Public License
- *    along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- *    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 GNU Affero General 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 <string>
-
-#include "mongo/stdx/functional.h"
-
-namespace mongo {
-
-class ServiceContext;
-class OperationContext;
-
-/**
- * Register a hook function that will implement
- * 'haveLocalShardingInfo' below. If no hook is registered,
- * haveLocalShardingInfo will always return 'false'.
- */
-void enableLocalShardingInfo(ServiceContext* context,
-                             stdx::function<bool(OperationContext*, const std::string&)> handler);
-
-/**
- * @return true if we have any shard info for the ns
- */
-bool haveLocalShardingInfo(OperationContext* opCtx, const std::string& ns);
-
-}  // namespace mongo
diff --git a/src/mongo/scripting/SConscript b/src/mongo/scripting/SConscript
index f8fd64d0c91..0f6ccad52f0 100644
--- a/src/mongo/scripting/SConscript
+++ b/src/mongo/scripting/SConscript
@@ -148,7 +148,6 @@ if usemozjs:
             '$BUILD_DIR/third_party/shim_mozjs',
             '$BUILD_DIR/mongo/shell/mongojs',
             '$BUILD_DIR/mongo/db/service_context',
-            '$BUILD_DIR/mongo/s/local_sharding_info',
         ],
     )
 else:
diff --git a/src/mongo/scripting/mozjs/db.cpp b/src/mongo/scripting/mozjs/db.cpp
index 7fa2f179241..aa886830556 100644
--- a/src/mongo/scripting/mozjs/db.cpp
+++ b/src/mongo/scripting/mozjs/db.cpp
@@ -32,7 +32,6 @@
 
 #include "mongo/db/namespace_string.h"
 #include "mongo/db/operation_context.h"
-#include "mongo/s/local_sharding_info.h"
 #include "mongo/scripting/mozjs/idwrapper.h"
 #include "mongo/scripting/mozjs/implscope.h"
 #include "mongo/scripting/mozjs/internedstring.h"
@@ -56,12 +55,6 @@ void DBInfo::getProperty(JSContext* cx,
 
         if (opContext && vp.isObject()) {
             ObjectWrapper o(cx, vp);
-
-            if (o.hasOwnField(InternedString::_fullName)) {
-                // need to check every time that the collection did not get sharded
-                if (haveLocalShardingInfo(opContext, o.getString(InternedString::_fullName)))
-                    uasserted(ErrorCodes::BadValue, "can't use sharded collection from db.eval");
-            }
         }
 
         return;
diff --git a/src/mongo/scripting/mozjs/dbcollection.cpp b/src/mongo/scripting/mozjs/dbcollection.cpp
index e0f903dd1fc..1de0f49212f 100644
--- a/src/mongo/scripting/mozjs/dbcollection.cpp
+++ b/src/mongo/scripting/mozjs/dbcollection.cpp
@@ -32,7 +32,6 @@
 
 #include "mongo/db/namespace_string.h"
 #include "mongo/db/operation_context.h"
-#include "mongo/s/local_sharding_info.h"
 #include "mongo/scripting/mozjs/bson.h"
 #include "mongo/scripting/mozjs/db.h"
 #include "mongo/scripting/mozjs/implscope.h"
@@ -72,13 +71,6 @@ void DBCollectionInfo::construct(JSContext* cx, JS::CallArgs args) {
     o.setValue(InternedString::_shortName, args.get(2));
     o.setValue(InternedString::_fullName, args.get(3));
 
-    std::string fullName = ValueWriter(cx, args.get(3)).toString();
-
-    auto context = scope->getOpContext();
-    if (context && haveLocalShardingInfo(context, fullName)) {
-        uasserted(ErrorCodes::BadValue, "can't use sharded collection from db.eval");
-    }
-
     args.rval().setObjectOrNull(thisv);
 }