diff options
-rw-r--r-- | src/mongo/rpc/metadata/client_metadata.cpp | 10 | ||||
-rw-r--r-- | src/mongo/rpc/metadata/client_metadata_test.cpp | 66 |
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 |