SERVER-35323 Guarantee the command object sent by the ShardingTaskExecutor will always have the correct logical session id

4 files changed, 191 insertions, 8 deletions

diff --git a/src/mongo/db/commands/getmore_cmd.cpp b/src/mongo/db/commands/getmore_cmd.cpp
index 1292c37b3f9..456a2cc6122 100644
--- a/src/mongo/db/commands/getmore_cmd.cpp
+++ b/src/mongo/db/commands/getmore_cmd.cpp
@@ -88,7 +88,6 @@ void validateLSID(OperationContext* opCtx, const GetMoreRequest& request, Client
                           << ", without an lsid",
             opCtx->getLogicalSessionId() || !cursor->getSessionId());
 
-    // TODO: SERVER-35323 - compare logicalSessionId that include userId.
     uassert(50738,
             str::stream() << "Cannot run getMore on cursor " << request.cursorid
                           << ", which was created in session "
@@ -96,7 +95,7 @@ void validateLSID(OperationContext* opCtx, const GetMoreRequest& request, Client
                           << ", in session "
                           << *opCtx->getLogicalSessionId(),
             !opCtx->getLogicalSessionId() || !cursor->getSessionId() ||
-                (opCtx->getLogicalSessionId()->getId() == cursor->getSessionId()->getId()));
+                (opCtx->getLogicalSessionId() == cursor->getSessionId()));
 }
 
 /**
diff --git a/src/mongo/s/SConscript b/src/mongo/s/SConscript
index b5bb23f1ddb..ebe11e262d9 100644
--- a/src/mongo/s/SConscript
+++ b/src/mongo/s/SConscript
@@ -322,6 +322,18 @@ env.Library(
     ],
 )
 
+env.CppUnitTest(
+    target='sharding_task_executor_test',
+    source=[
+        'sharding_task_executor_test.cpp',
+    ],
+    LIBDEPS=[
+        '$BUILD_DIR/mongo/db/auth/authmocks',
+        'sharding_task_executor',
+        'shard_server_test_fixture',
+    ],
+)
+
 # This library contains sharding functionality used by both mongod and mongos
 env.Library(
     target='grid',
diff --git a/src/mongo/s/sharding_task_executor.cpp b/src/mongo/s/sharding_task_executor.cpp
index 5ae93c3e33e..c95e343de94 100644
--- a/src/mongo/s/sharding_task_executor.cpp
+++ b/src/mongo/s/sharding_task_executor.cpp
@@ -121,20 +121,41 @@ StatusWith<TaskExecutor::CallbackHandle> ShardingTaskExecutor::scheduleRemoteCom
         return _executor->scheduleRemoteCommand(request, cb, baton);
     }
 
-    boost::optional<RemoteCommandRequest> newRequest;
+    boost::optional<RemoteCommandRequest> requestWithFixedLsid = [&] {
+        boost::optional<RemoteCommandRequest> newRequest;
 
-    if (request.opCtx->getLogicalSessionId() && !request.cmdObj.hasField("lsid")) {
-        newRequest.emplace(request);
+        if (!request.opCtx->getLogicalSessionId()) {
+            return newRequest;
+        }
+
+        if (request.cmdObj.hasField("lsid")) {
+            auto cmdObjLsid =
+                LogicalSessionFromClient::parse("lsid"_sd, request.cmdObj["lsid"].Obj());
+
+            if (cmdObjLsid.getUid()) {
+                invariant(*cmdObjLsid.getUid() == request.opCtx->getLogicalSessionId()->getUid());
+                return newRequest;
+            }
+
+            newRequest.emplace(request);
+            newRequest->cmdObj = newRequest->cmdObj.removeField("lsid");
+        }
+
+        if (!newRequest) {
+            newRequest.emplace(request);
+        }
 
         BSONObjBuilder bob(std::move(newRequest->cmdObj));
         {
-            // TODO SERVER-33702.
             BSONObjBuilder subbob(bob.subobjStart("lsid"));
             request.opCtx->getLogicalSessionId()->serialize(&subbob);
+            subbob.done();
         }
 
         newRequest->cmdObj = bob.obj();
-    }
+
+        return newRequest;
+    }();
 
     std::shared_ptr<OperationTimeTracker> timeTracker = OperationTimeTracker::get(request.opCtx);
 
@@ -204,7 +225,8 @@ StatusWith<TaskExecutor::CallbackHandle> ShardingTaskExecutor::scheduleRemoteCom
         }
     };
 
-    return _executor->scheduleRemoteCommand(newRequest ? *newRequest : request, shardingCb, baton);
+    return _executor->scheduleRemoteCommand(
+        requestWithFixedLsid ? *requestWithFixedLsid : request, shardingCb, baton);
 }
 
 void ShardingTaskExecutor::cancel(const CallbackHandle& cbHandle) {
diff --git a/src/mongo/s/sharding_task_executor_test.cpp b/src/mongo/s/sharding_task_executor_test.cpp
new file mode 100644
index 00000000000..27df594923a
--- /dev/null
+++ b/src/mongo/s/sharding_task_executor_test.cpp
@@ -0,0 +1,150 @@
+/**
+ *    Copyright (C) 2018 MongoDB 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.
+ */
+
+#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kExecutor
+
+#include "mongo/platform/basic.h"
+
+#include "mongo/s/sharding_task_executor.h"
+
+#include "mongo/executor/network_interface.h"
+#include "mongo/executor/network_interface_mock.h"
+#include "mongo/executor/task_executor_test_common.h"
+#include "mongo/executor/task_executor_test_fixture.h"
+#include "mongo/executor/thread_pool_mock.h"
+#include "mongo/executor/thread_pool_task_executor.h"
+#include "mongo/executor/thread_pool_task_executor_test_fixture.h"
+#include "mongo/s/shard_server_test_fixture.h"
+
+namespace mongo {
+namespace {
+
+using executor::NetworkInterfaceMock;
+using executor::RemoteCommandRequest;
+using executor::TaskExecutor;
+
+class ShardingTaskExecutorTest : public ShardServerTestFixture {
+private:
+    void setUp() final;
+    void tearDown() final;
+
+protected:
+    LogicalSessionId constructFullLsid();
+    void assertOpCtxLsidEqualsCmdObjLsid(const BSONObj& cmdObj);
+
+    executor::NetworkInterfaceMock* _network{nullptr};
+
+    std::unique_ptr<executor::ThreadPoolTaskExecutor> _threadPool;
+};
+
+void ShardingTaskExecutorTest::setUp() {
+    ShardServerTestFixture::setUp();
+
+    auto netForFixedTaskExecutor = std::make_unique<executor::NetworkInterfaceMock>();
+    _network = netForFixedTaskExecutor.get();
+
+    _threadPool = makeThreadPoolTestExecutor(std::move(netForFixedTaskExecutor));
+}
+
+void ShardingTaskExecutorTest::tearDown() {
+    ShardServerTestFixture::tearDown();
+}
+
+LogicalSessionId ShardingTaskExecutorTest::constructFullLsid() {
+    auto id = UUID::gen();
+    auto uid = SHA256Block{};
+
+    return LogicalSessionId(id, uid);
+}
+
+void ShardingTaskExecutorTest::assertOpCtxLsidEqualsCmdObjLsid(const BSONObj& cmdObj) {
+    auto opCtxLsid = operationContext()->getLogicalSessionId();
+
+    ASSERT(opCtxLsid);
+
+    auto cmdObjLsid = LogicalSessionFromClient::parse("lsid"_sd, cmdObj["lsid"].Obj());
+
+    ASSERT_EQ(opCtxLsid->getId(), cmdObjLsid.getId());
+    ASSERT_EQ(opCtxLsid->getUid(), *cmdObjLsid.getUid());
+}
+
+TEST_F(ShardingTaskExecutorTest, MissingLsidAddsLsidInCommand) {
+    operationContext()->setLogicalSessionId(constructFullLsid());
+    ASSERT(operationContext()->getLogicalSessionId());
+
+    executor::ShardingTaskExecutor executor(std::move(_threadPool));
+    executor.startup();
+    _network->enterNetwork();
+
+    const RemoteCommandRequest request(HostAndPort("localhost", 27017),
+                                       "mydb",
+                                       BSON("whatsUp"
+                                            << "doc"),
+                                       operationContext());
+
+    TaskExecutor::CallbackHandle cbHandle = unittest::assertGet(executor.scheduleRemoteCommand(
+        request, [=](const executor::TaskExecutor::RemoteCommandCallbackArgs) -> void {}, nullptr));
+
+    ASSERT(_network->hasReadyRequests());
+    NetworkInterfaceMock::NetworkOperationIterator noi = _network->getNextReadyRequest();
+    auto cmdObj = noi->getRequest().cmdObj;
+
+    assertOpCtxLsidEqualsCmdObjLsid(cmdObj);
+}
+
+TEST_F(ShardingTaskExecutorTest, IncompleteLsidAddsLsidInCommand) {
+    operationContext()->setLogicalSessionId(constructFullLsid());
+    ASSERT(operationContext()->getLogicalSessionId());
+
+    executor::ShardingTaskExecutor executor(std::move(_threadPool));
+    executor.startup();
+    _network->enterNetwork();
+
+    BSONObjBuilder bob;
+    bob.append("whatsUp", "doc");
+    {
+        BSONObjBuilder subbob(bob.subobjStart("lsid"));
+        subbob << "id" << operationContext()->getLogicalSessionId()->getId();
+        subbob.done();
+    }
+
+    const RemoteCommandRequest request(
+        HostAndPort("localhost", 27017), "mydb", bob.obj(), operationContext());
+
+    TaskExecutor::CallbackHandle cbHandle = unittest::assertGet(executor.scheduleRemoteCommand(
+        request, [=](const executor::TaskExecutor::RemoteCommandCallbackArgs) -> void {}, nullptr));
+
+    ASSERT(_network->hasReadyRequests());
+    NetworkInterfaceMock::NetworkOperationIterator noi = _network->getNextReadyRequest();
+    auto cmdObj = noi->getRequest().cmdObj;
+
+    assertOpCtxLsidEqualsCmdObjLsid(cmdObj);
+}
+
+}  // namespace
+}  // namespace mongo