diff options
author | Esha Maharishi <esha.maharishi@mongodb.com> | 2017-04-13 10:16:47 -0400 |
---|---|---|
committer | Esha Maharishi <esha.maharishi@mongodb.com> | 2017-04-14 18:28:44 -0400 |
commit | e96a5090084d5d80f1349b3223d38f5f52c013f0 (patch) | |
tree | 9c2a0daa1537fa603c8500f1e61a86bc8e583563 | |
parent | 12d552607487faa7fc457577845c2e3623b21541 (diff) | |
download | mongo-e96a5090084d5d80f1349b3223d38f5f52c013f0.tar.gz |
SERVER-28739 make the js scripting engine correctly verify that accessed collections are unsharded
-rw-r--r-- | jstests/core/eval1.js | 3 | ||||
-rw-r--r-- | jstests/core/eval5.js | 3 | ||||
-rw-r--r-- | jstests/core/eval6.js | 3 | ||||
-rw-r--r-- | jstests/core/eval9.js | 4 | ||||
-rw-r--r-- | jstests/core/eval_nolock.js | 3 | ||||
-rw-r--r-- | jstests/core/evala.js | 3 | ||||
-rw-r--r-- | jstests/core/evalc.js | 4 | ||||
-rw-r--r-- | jstests/core/evale.js | 6 | ||||
-rw-r--r-- | jstests/core/evalg.js | 4 | ||||
-rw-r--r-- | src/mongo/db/assemble_response.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/commands/dbcommands.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/commands/eval.cpp | 37 | ||||
-rw-r--r-- | src/mongo/db/ops/write_ops_exec.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/s/collection_sharding_state.cpp | 5 | ||||
-rw-r--r-- | src/mongo/db/s/collection_sharding_state_test.cpp | 17 | ||||
-rw-r--r-- | src/mongo/db/s/sharding_state.cpp | 26 | ||||
-rw-r--r-- | src/mongo/s/SConscript | 10 | ||||
-rw-r--r-- | src/mongo/s/local_sharding_info.cpp | 71 | ||||
-rw-r--r-- | src/mongo/s/local_sharding_info.h | 53 | ||||
-rw-r--r-- | src/mongo/scripting/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/db.cpp | 7 | ||||
-rw-r--r-- | src/mongo/scripting/mozjs/dbcollection.cpp | 8 |
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); } |