SERVER-57708 Ensure ClientMetadata left in valid state after failed parsing

(cherry picked from commit cc883e8854849bfcd7e0f4c670dfc5fdbbaf7abe)

2 files changed, 60 insertions, 16 deletions

diff --git a/src/mongo/rpc/metadata/client_metadata.cpp b/src/mongo/rpc/metadata/client_metadata.cpp
index 6ea09b44b02..86d7230120e 100644
--- a/src/mongo/rpc/metadata/client_metadata.cpp
+++ b/src/mongo/rpc/metadata/client_metadata.cpp
@@ -471,15 +471,19 @@ void ClientMetadata::setAndFinalize(Client* client, boost::optional<ClientMetada
 }
 
 void ClientMetadata::setFromMetadataForOperation(OperationContext* opCtx, BSONElement& elem) {
+    if (MONGO_unlikely(elem.eoo())) {
+        return;
+    }
     auto lk = stdx::lock_guard(*opCtx->getClient());
 
     auto& state = getOperationState(opCtx);
-    auto wasFinalized = std::exchange(state.isFinalized, true);
     uassert(ErrorCodes::ClientMetadataCannotBeMutated,
             "The client metadata document may only be set once per operation",
-            !state.meta && !wasFinalized);
+            !state.meta && !state.isFinalized);
+    auto inputMetadata = ClientMetadata::readFromMetadata(elem);
 
-    state.meta = ClientMetadata::readFromMetadata(elem);
+    state.isFinalized = true;
+    state.meta = std::move(inputMetadata);
 }
 
 void ClientMetadata::setFromMetadata(Client* client, BSONElement& elem) {
diff --git a/src/mongo/rpc/metadata/client_metadata_test.cpp b/src/mongo/rpc/metadata/client_metadata_test.cpp
index bae05d00e97..6dbaace6281 100644
--- a/src/mongo/rpc/metadata/client_metadata_test.cpp
+++ b/src/mongo/rpc/metadata/client_metadata_test.cpp
@@ -36,9 +36,13 @@
 #include <boost/filesystem.hpp>
 #include <map>
 
+#include "mongo/bson/bsonelement.h"
 #include "mongo/bson/bsonobj.h"
 #include "mongo/bson/bsonobjbuilder.h"
 #include "mongo/bson/json.h"
+#include "mongo/db/client.h"
+#include "mongo/db/operation_context.h"
+#include "mongo/db/service_context.h"
 #include "mongo/s/is_mongos.h"
 #include "mongo/unittest/unittest.h"
 #include "mongo/util/processinfo.h"
@@ -76,7 +80,7 @@ constexpr auto kUnknown = "unkown"_sd;
     } while (0)
 
 
-TEST(ClientMetadatTest, TestLoopbackTest) {
+TEST(ClientMetadataTest, TestLoopbackTest) {
     // serializePrivate with appName
     {
         BSONObjBuilder builder;
@@ -143,7 +147,7 @@ TEST(ClientMetadatTest, TestLoopbackTest) {
 }
 
 // Mixed: no client metadata document
-TEST(ClientMetadatTest, TestEmptyDoc) {
+TEST(ClientMetadataTest, TestEmptyDoc) {
     {
         auto parseStatus = ClientMetadata::parse(BSONElement());
 
@@ -159,7 +163,7 @@ TEST(ClientMetadatTest, TestEmptyDoc) {
 }
 
 // Positive: test with only required fields
-TEST(ClientMetadatTest, TestRequiredOnlyFields) {
+TEST(ClientMetadataTest, TestRequiredOnlyFields) {
     // Without app name
     ASSERT_DOC_OK(kDriver << BSON(kName << "n1" << kVersion << "v1") << kOperatingSystem
                           << BSON(kType << kUnknown));
@@ -172,7 +176,7 @@ TEST(ClientMetadatTest, TestRequiredOnlyFields) {
 
 
 // Positive: test with app_name spelled wrong fields
-TEST(ClientMetadatTest, TestWithAppNameSpelledWrong) {
+TEST(ClientMetadataTest, TestWithAppNameSpelledWrong) {
     ASSERT_DOC_OK(kApplication << BSON("extra"
                                        << "1")
                                << kDriver << BSON(kName << "n1" << kVersion << "v1")
@@ -180,19 +184,19 @@ TEST(ClientMetadatTest, TestWithAppNameSpelledWrong) {
 }
 
 // Positive: test with empty application document
-TEST(ClientMetadatTest, TestWithEmptyApplication) {
+TEST(ClientMetadataTest, TestWithEmptyApplication) {
     ASSERT_DOC_OK(kApplication << BSONObj() << kDriver << BSON(kName << "n1" << kVersion << "v1")
                                << kOperatingSystem << BSON(kType << kUnknown));
 }
 
 // Negative: test with appplication wrong type
-TEST(ClientMetadatTest, TestNegativeWithAppNameWrongType) {
+TEST(ClientMetadataTest, TestNegativeWithAppNameWrongType) {
     ASSERT_DOC_NOT_OK(kApplication << "1" << kDriver << BSON(kName << "n1" << kVersion << "v1")
                                    << kOperatingSystem << BSON(kType << kUnknown));
 }
 
 // Positive: test with extra fields
-TEST(ClientMetadatTest, TestExtraFields) {
+TEST(ClientMetadataTest, TestExtraFields) {
     ASSERT_DOC_OK(kApplication << BSON(kName << "1"
                                              << "extra"
                                              << "v1")
@@ -222,14 +226,14 @@ TEST(ClientMetadatTest, TestExtraFields) {
 }
 
 // Negative: only application specified
-TEST(ClientMetadatTest, TestNegativeOnlyApplication) {
+TEST(ClientMetadataTest, TestNegativeOnlyApplication) {
     ASSERT_DOC_NOT_OK(kApplication << BSON(kName << "1"
                                                  << "extra"
                                                  << "v1"));
 }
 
 // Negative: all combinations of only missing 1 required field
-TEST(ClientMetadatTest, TestNegativeMissingRequiredOneField) {
+TEST(ClientMetadataTest, TestNegativeMissingRequiredOneField) {
     ASSERT_DOC_NOT_OK(kDriver << BSON(kVersion << "v1") << kOperatingSystem
                               << BSON(kType << kUnknown));
     ASSERT_DOC_NOT_OK(kDriver << BSON(kName << "n1") << kOperatingSystem
@@ -238,7 +242,7 @@ TEST(ClientMetadatTest, TestNegativeMissingRequiredOneField) {
 }
 
 // Negative: document with wrong types for required fields
-TEST(ClientMetadatTest, TestNegativeWrongTypes) {
+TEST(ClientMetadataTest, TestNegativeWrongTypes) {
     ASSERT_DOC_NOT_OK(kApplication << BSON(kName << 1) << kDriver
                                    << BSON(kName << "n1" << kVersion << "v1") << kOperatingSystem
                                    << BSON(kType << kUnknown));
@@ -254,7 +258,7 @@ TEST(ClientMetadatTest, TestNegativeWrongTypes) {
 }
 
 // Negative: document larger than 512 bytes
-TEST(ClientMetadatTest, TestNegativeLargeDocument) {
+TEST(ClientMetadataTest, TestNegativeLargeDocument) {
     bool savedMongos = isMongos();
     auto unsetMongoS = makeGuard([&] { setMongos(savedMongos); });
 
@@ -274,7 +278,7 @@ TEST(ClientMetadatTest, TestNegativeLargeDocument) {
 }
 
 // Negative: document with app_name larger than 128 bytes
-TEST(ClientMetadatTest, TestNegativeLargeAppName) {
+TEST(ClientMetadataTest, TestNegativeLargeAppName) {
     {
         std::string str(128, 'x');
         ASSERT_DOC_OK(kApplication << BSON(kName << str) << kDriver
@@ -296,7 +300,7 @@ TEST(ClientMetadatTest, TestNegativeLargeAppName) {
 }
 
 // Serialize and attach mongos information
-TEST(ClientMetadatTest, TestMongoSAppend) {
+TEST(ClientMetadataTest, TestMongoSAppend) {
     BSONObjBuilder builder;
     ASSERT_OK(ClientMetadata::serializePrivate("a", "b", "c", "d", "e", "f", "g", &builder));
 
@@ -338,4 +342,40 @@ TEST(ClientMetadatTest, TestMongoSAppend) {
     ASSERT_BSONOBJ_EQ(doc, outDoc);
 }
 
+TEST(ClientMetadataTest, TestInvalidDocWhileSettingOpCtxMetadata) {
+    auto svcCtx = ServiceContext::make();
+    auto client = svcCtx->makeClient("ClientMetadataTest");
+    auto opCtx = client->makeOperationContext();
+    // metadataElem is of BSON type int
+    auto obj = BSON("a" << 4);
+    auto metadataElem = obj["a"];
+
+    // We expect parsing to fail because metadata must be of BSON type "document"
+    ASSERT_THROWS_CODE(ClientMetadata::setFromMetadataForOperation(opCtx.get(), metadataElem),
+                       DBException,
+                       ErrorCodes::TypeMismatch);
+
+    // Ensure we can still safely retrieve a ClientMetadata* from the opCtx, and that it was left
+    // unset
+    auto clientMetaDataPtr = ClientMetadata::getForOperation(opCtx.get());
+    ASSERT_FALSE(clientMetaDataPtr);
+}
+
+TEST(ClientMetadataTest, TestEooElemAsValueToSetOpCtxMetadata) {
+    auto svcCtx = ServiceContext::make();
+    auto client = svcCtx->makeClient("ClientMetadataTest");
+    auto opCtx = client->makeOperationContext();
+    // metadataElem is of BSON type eoo
+    auto metadataElem = BSONElement();
+
+    // BSON type "eoo" represents no data; we expect setting the metadata to be a no-op in this
+    // case.
+    ClientMetadata::setFromMetadataForOperation(opCtx.get(), metadataElem);
+
+    // Ensure we can still safely retrieve a ClientMetadata* from the opCtx, and that it was left
+    // unset
+    auto clientMetaDataPtr = ClientMetadata::getForOperation(opCtx.get());
+    ASSERT_FALSE(clientMetaDataPtr);
+}
+
 }  // namespace mongo