diff options
34 files changed, 225 insertions, 116 deletions
diff --git a/buildscripts/resmokeconfig/suites/decimal.yml b/buildscripts/resmokeconfig/suites/decimal.yml index e0da0a96358..62cb11ecab2 100644 --- a/buildscripts/resmokeconfig/suites/decimal.yml +++ b/buildscripts/resmokeconfig/suites/decimal.yml @@ -17,4 +17,3 @@ executor: mongod_options: set_parameters: enableTestCommands: 1 - enableBSON1_1: 1 diff --git a/jstests/decimal/decimal_feature_compatibility_version.js b/jstests/decimal/decimal_feature_compatibility_version.js new file mode 100644 index 00000000000..0b9d1903704 --- /dev/null +++ b/jstests/decimal/decimal_feature_compatibility_version.js @@ -0,0 +1,47 @@ +// Test that decimal usage is restricted when the featureCompatibilityVersion is 3.2. + +(function() { + "use strict"; + + const conn = MongoRunner.runMongod({}); + assert.neq(null, conn, "mongod was unable to start up"); + + const decimalDB = conn.getDB("decimal_feature_compatibility_version"); + assert.commandWorked(decimalDB.dropDatabase()); + assert.commandWorked(decimalDB.runCommand({create: "collection"})); + + const adminDB = conn.getDB("admin"); + + // Ensure the featureCompatibilityVersion is 3.4. + assert.commandWorked(adminDB.runCommand({setFeatureCompatibilityVersion: "3.4"})); + let res = adminDB.runCommand({getParameter: 1, featureCompatibilityVersion: 1}); + assert.commandWorked(res); + assert.eq("3.4", res.featureCompatibilityVersion); + + // Decimals can be inserted when the featureCompatibilityVersion is 3.4. + assert.writeOK(decimalDB.collection.insert({a: NumberDecimal(2.0)})); + + // Ensure the featureCompatibilityVersion is 3.2. + assert.commandWorked(adminDB.runCommand({setFeatureCompatibilityVersion: "3.2"})); + res = adminDB.runCommand({getParameter: 1, featureCompatibilityVersion: 1}); + assert.commandWorked(res); + assert.eq("3.2", res.featureCompatibilityVersion); + + // Decimals cannot be inserted when the featureCompatibilityVersion is 3.2. + // TODO SERVER-23972: Inserting decimals should also fail in legacy write mode when + // featureCompatibilityVersion is 3.2. + if (decimalDB.getMongo().writeMode() === "commands") { + assert.writeErrorWithCode(decimalDB.collection.insert({a: NumberDecimal(2.0)}), + ErrorCodes.InvalidBSON); + } + + // Decimals cannot be used in queries when the featureCompatibilityVersion is 3.2. + assert.throws(function() { + decimalDB.collection.findOne({a: NumberDecimal(2.0)}); + }); + + // Decimals can be read when the featureCompatibilityVersion is 3.2. + assert.eq(decimalDB.collection.findOne().a, NumberDecimal(2.0)); + + MongoRunner.stopMongod(conn); +}()); diff --git a/jstests/multiVersion/collation_downgrade.js b/jstests/multiVersion/collation_downgrade.js index ef454a482e4..75bf1c5f9eb 100644 --- a/jstests/multiVersion/collation_downgrade.js +++ b/jstests/multiVersion/collation_downgrade.js @@ -21,12 +21,7 @@ storageEngine: jsTest.options().storageEngine || "wiredTiger", }; - // Whenever we start "latest", we use the "enableBSON1_1" server parameter to force indices - // created with the wiredTiger storage engine to use KeyString V0. Otherwise, downgrade will - // fail due to creating KeyString V1 indices rather than exercising the code which prevents - // downgrading in the presence of non-simple collations. - var latestOptions = - Object.extend({binVersion: "latest", setParameter: "enableBSON1_1=false"}, defaultOptions); + var latestOptions = Object.extend({binVersion: "latest"}, defaultOptions); var downgradeVersion = "3.2"; var downgradeOptions = Object.extend({binVersion: downgradeVersion}, defaultOptions); @@ -40,6 +35,12 @@ var conn = MongoRunner.runMongod(latestOptions); assert.neq(null, conn, "mongod was unable to start up with options: " + tojson(latestOptions)); + // Set featureCompatibilityVersion to 3.2 to force indices created with the wiredTiger storage + // engine to use KeyString V0. Otherwise, downgrade will fail due to creating KeyString V1 + // indices rather than exercising the code which prevents downgrading in the presence of + // non-simple collations. + assert.commandWorked(conn.getDB("admin").runCommand({setFeatureCompatibilityVersion: "3.2"})); + var testDB = conn.getDB("test"); testDB.dropDatabase(); assert.commandWorked(testDB.createCollection("simplecollator")); diff --git a/jstests/multiVersion/multikey_paths_downgrade.js b/jstests/multiVersion/multikey_paths_downgrade.js index bcc8ba15009..24a445ff75c 100644 --- a/jstests/multiVersion/multikey_paths_downgrade.js +++ b/jstests/multiVersion/multikey_paths_downgrade.js @@ -87,11 +87,7 @@ // MongoRunner.stopMongod(conn); - // We use the "enableBSON1_1" server parameter to force indexes created on the "latest" version - // with the wiredTiger storage engine to use KeyString v0. Otherwise, we wouldn't be able to - // downgrade to 3.2 without first dropping the newly created index. - options = - Object.extend({binVersion: "latest", setParameter: "enableBSON1_1=false"}, defaultOptions); + options = Object.extend({binVersion: "latest"}, defaultOptions); conn = MongoRunner.runMongod(options); assert.neq(null, conn, @@ -157,8 +153,7 @@ // Upgrade from the "last-stable" version to the "latest" version. // MongoRunner.stopMongod(conn); - options = - Object.extend({binVersion: "latest", setParameter: "enableBSON1_1=false"}, defaultOptions); + options = Object.extend({binVersion: "latest"}, defaultOptions); conn = MongoRunner.runMongod(options); assert.neq( null, diff --git a/src/mongo/bson/bson_validate.cpp b/src/mongo/bson/bson_validate.cpp index c578231e435..116dab6d12a 100644 --- a/src/mongo/bson/bson_validate.cpp +++ b/src/mongo/bson/bson_validate.cpp @@ -237,9 +237,10 @@ Status validateElementInfo(Buffer* buffer, return makeError("Invalid bson", idElem); return Status::OK(); } else { - return Status( - ErrorCodes::InvalidBSON, - "Attempt to use a decimal BSON type when support is not currently enabled."); + return Status(ErrorCodes::InvalidBSON, + "Cannot use decimal BSON type when the featureCompatibilityVersion " + "is 3.2. See " + "http://dochub.mongodb.org/core/3.4-feature-compatibility."); } case DBRef: diff --git a/src/mongo/bson/bson_validate.h b/src/mongo/bson/bson_validate.h index a40d9368ed1..e2aea3f47db 100644 --- a/src/mongo/bson/bson_validate.h +++ b/src/mongo/bson/bson_validate.h @@ -44,8 +44,6 @@ class Status; * this is NOT the bson size, but how far we know the buffer is valid * @param version - newest version to accept */ -Status validateBSON(const char* buf, - uint64_t maxLength, - BSONVersion version = BSONVersion::kLatest); +Status validateBSON(const char* buf, uint64_t maxLength, BSONVersion version); } // namespace mongo diff --git a/src/mongo/bson/bson_validate_test.cpp b/src/mongo/bson/bson_validate_test.cpp index 1d44b237d35..de4e55acb7b 100644 --- a/src/mongo/bson/bson_validate_test.cpp +++ b/src/mongo/bson/bson_validate_test.cpp @@ -52,10 +52,10 @@ void appendInvalidStringElement(const char* fieldName, BufBuilder* bb) { TEST(BSONValidate, Basic) { BSONObj x; - ASSERT_TRUE(x.valid()); + ASSERT_TRUE(x.valid(BSONVersion::kLatest)); x = BSON("x" << 1); - ASSERT_TRUE(x.valid()); + ASSERT_TRUE(x.valid(BSONVersion::kLatest)); } TEST(BSONValidate, RandomData) { @@ -81,12 +81,12 @@ TEST(BSONValidate, RandomData) { ASSERT_EQUALS(size, o.objsize()); - if (o.valid()) { + if (o.valid(BSONVersion::kLatest)) { numValid++; jsonSize += o.jsonString().size(); - ASSERT_OK(validateBSON(o.objdata(), o.objsize())); + ASSERT_OK(validateBSON(o.objdata(), o.objsize(), BSONVersion::kLatest)); } else { - ASSERT_NOT_OK(validateBSON(o.objdata(), o.objsize())); + ASSERT_NOT_OK(validateBSON(o.objdata(), o.objsize(), BSONVersion::kLatest)); } delete[] x; @@ -129,12 +129,12 @@ TEST(BSONValidate, MuckingData1) { data[i] = 200; numToRun++; - if (mine.valid()) { + if (mine.valid(BSONVersion::kLatest)) { numValid++; jsonSize += mine.jsonString().size(); - ASSERT_OK(validateBSON(mine.objdata(), mine.objsize())); + ASSERT_OK(validateBSON(mine.objdata(), mine.objsize(), BSONVersion::kLatest)); } else { - ASSERT_NOT_OK(validateBSON(mine.objdata(), mine.objsize())); + ASSERT_NOT_OK(validateBSON(mine.objdata(), mine.objsize(), BSONVersion::kLatest)); } } @@ -187,29 +187,31 @@ TEST(BSONValidate, Fuzz) { BSONObj fuzzed(buffer.get()); // Check that the two validation implementations agree (and neither crashes). - ASSERT_EQUALS(fuzzed.valid(), validateBSON(fuzzed.objdata(), fuzzed.objsize()).isOK()); + ASSERT_EQUALS( + fuzzed.valid(BSONVersion::kLatest), + validateBSON(fuzzed.objdata(), fuzzed.objsize(), BSONVersion::kLatest).isOK()); } } TEST(BSONValidateFast, Empty) { BSONObj x; - ASSERT_OK(validateBSON(x.objdata(), x.objsize())); + ASSERT_OK(validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest)); } TEST(BSONValidateFast, RegEx) { BSONObjBuilder b; b.appendRegex("foo", "i"); BSONObj x = b.obj(); - ASSERT_OK(validateBSON(x.objdata(), x.objsize())); + ASSERT_OK(validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest)); } TEST(BSONValidateFast, Simple0) { BSONObj x; - ASSERT_OK(validateBSON(x.objdata(), x.objsize())); + ASSERT_OK(validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest)); x = BSON("foo" << 17 << "bar" << "eliot"); - ASSERT_OK(validateBSON(x.objdata(), x.objsize())); + ASSERT_OK(validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest)); } TEST(BSONValidateFast, Simple2) { @@ -221,7 +223,7 @@ TEST(BSONValidateFast, Simple2) { sprintf(buf, "bar%d", i); b.appendMaxForType(buf, i); BSONObj x = b.obj(); - ASSERT_OK(validateBSON(x.objdata(), x.objsize())); + ASSERT_OK(validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest)); } } @@ -236,14 +238,14 @@ TEST(BSONValidateFast, Simple3) { b.appendMaxForType(buf, i); } BSONObj x = b.obj(); - ASSERT_OK(validateBSON(x.objdata(), x.objsize())); + ASSERT_OK(validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest)); } TEST(BSONValidateFast, NestedObject) { BSONObj x = BSON("a" << 1 << "b" << BSON("c" << 2 << "d" << BSONArrayBuilder().obj() << "e" << BSON_ARRAY("1" << 2 << 3))); - ASSERT_OK(validateBSON(x.objdata(), x.objsize())); - ASSERT_NOT_OK(validateBSON(x.objdata(), x.objsize() / 2)); + ASSERT_OK(validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest)); + ASSERT_NOT_OK(validateBSON(x.objdata(), x.objsize() / 2, BSONVersion::kLatest)); } TEST(BSONValidateFast, ErrorWithId) { @@ -252,7 +254,7 @@ TEST(BSONValidateFast, ErrorWithId) { ob.append("_id", 1); appendInvalidStringElement("not_id", &bb); const BSONObj x = ob.done(); - const Status status = validateBSON(x.objdata(), x.objsize()); + const Status status = validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest); ASSERT_NOT_OK(status); ASSERT_EQUALS(status.reason(), "not null terminated string in object with _id: 1"); } @@ -263,7 +265,7 @@ TEST(BSONValidateFast, ErrorBeforeId) { appendInvalidStringElement("not_id", &bb); ob.append("_id", 1); const BSONObj x = ob.done(); - const Status status = validateBSON(x.objdata(), x.objsize()); + const Status status = validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest); ASSERT_NOT_OK(status); ASSERT_EQUALS(status.reason(), "not null terminated string in object with unknown _id"); } @@ -273,7 +275,7 @@ TEST(BSONValidateFast, ErrorNoId) { BSONObjBuilder ob(bb); appendInvalidStringElement("not_id", &bb); const BSONObj x = ob.done(); - const Status status = validateBSON(x.objdata(), x.objsize()); + const Status status = validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest); ASSERT_NOT_OK(status); ASSERT_EQUALS(status.reason(), "not null terminated string in object with unknown _id"); } @@ -283,7 +285,7 @@ TEST(BSONValidateFast, ErrorIsInId) { BSONObjBuilder ob(bb); appendInvalidStringElement("_id", &bb); const BSONObj x = ob.done(); - const Status status = validateBSON(x.objdata(), x.objsize()); + const Status status = validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest); ASSERT_NOT_OK(status); ASSERT_EQUALS(status.reason(), "not null terminated string in object with unknown _id"); } @@ -296,7 +298,7 @@ TEST(BSONValidateFast, NonTopLevelId) { << "not the real _id")); appendInvalidStringElement("not_id2", &bb); const BSONObj x = ob.done(); - const Status status = validateBSON(x.objdata(), x.objsize()); + const Status status = validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest); ASSERT_NOT_OK(status); ASSERT_EQUALS(status.reason(), "not null terminated string in object with unknown _id"); } @@ -317,14 +319,14 @@ TEST(BSONValidateFast, StringHasSomething) { 4 // size , x.objsize()); - ASSERT_NOT_OK(validateBSON(x.objdata(), x.objsize())); + ASSERT_NOT_OK(validateBSON(x.objdata(), x.objsize(), BSONVersion::kLatest)); } TEST(BSONValidateBool, BoolValuesAreValidated) { BSONObjBuilder bob; bob.append("x", false); const BSONObj obj = bob.done(); - ASSERT_OK(validateBSON(obj.objdata(), obj.objsize())); + ASSERT_OK(validateBSON(obj.objdata(), obj.objsize(), BSONVersion::kLatest)); const BSONElement x = obj["x"]; // Legal, because we know that the BufBuilder gave // us back some heap memory, which isn't oringinally const. @@ -334,9 +336,9 @@ TEST(BSONValidateBool, BoolValuesAreValidated) { ++val) { *writable = static_cast<char>(val); if ((val == 0) || (val == 1)) { - ASSERT_OK(validateBSON(obj.objdata(), obj.objsize())); + ASSERT_OK(validateBSON(obj.objdata(), obj.objsize(), BSONVersion::kLatest)); } else { - ASSERT_NOT_OK(validateBSON(obj.objdata(), obj.objsize())); + ASSERT_NOT_OK(validateBSON(obj.objdata(), obj.objsize(), BSONVersion::kLatest)); } } } diff --git a/src/mongo/bson/bsonobj.cpp b/src/mongo/bson/bsonobj.cpp index 06d8a28c950..c3a561b0cdf 100644 --- a/src/mongo/bson/bsonobj.cpp +++ b/src/mongo/bson/bsonobj.cpp @@ -99,8 +99,8 @@ string BSONObj::jsonString(JsonStringFormat format, int pretty, bool isArray) co return s.str(); } -bool BSONObj::valid() const { - return validateBSON(objdata(), objsize()).isOK(); +bool BSONObj::valid(BSONVersion version) const { + return validateBSON(objdata(), objsize(), version).isOK(); } int BSONObj::woCompare(const BSONObj& r, diff --git a/src/mongo/bson/bsonobj.h b/src/mongo/bson/bsonobj.h index 77f13fd533b..a39bdfb64a6 100644 --- a/src/mongo/bson/bsonobj.h +++ b/src/mongo/bson/bsonobj.h @@ -532,8 +532,11 @@ public: passed object. */ BSONObj replaceFieldNames(const BSONObj& obj) const; - /** true unless corrupt */ - bool valid() const; + /** + * Returns true if this object is valid according to the specified BSON version, and returns + * false otherwise. + */ + bool valid(BSONVersion version) const; enum MatchType { Equality = 0, diff --git a/src/mongo/bson/bsontypes.cpp b/src/mongo/bson/bsontypes.cpp index 8ff370ec12f..b1f545ee180 100644 --- a/src/mongo/bson/bsontypes.cpp +++ b/src/mongo/bson/bsontypes.cpp @@ -34,8 +34,6 @@ namespace mongo { -bool enableBSON1_1 = true; - const char kMaxKeyData[] = {7, 0, 0, 0, static_cast<char>(MaxKey), 0, 0}; const BSONObj kMaxBSONKey(kMaxKeyData); diff --git a/src/mongo/bson/bsontypes.h b/src/mongo/bson/bsontypes.h index e3fde085ad9..ce3283ebb59 100644 --- a/src/mongo/bson/bsontypes.h +++ b/src/mongo/bson/bsontypes.h @@ -56,12 +56,6 @@ extern const BSONObj kMinBSONKey; enum class BSONVersion { kV1_0, kV1_1, kLatest = kV1_1 }; /** - Flag that determines whether we should accept decimal types in object validation, and default - to KeyString V1 indexes on non-MMAP storage engines. Set by enableBSON1_1 server parameter. -*/ -extern bool enableBSON1_1; - -/** the complete list of valid BSON types see also bsonspec.org */ diff --git a/src/mongo/client/dbclientcursor.cpp b/src/mongo/client/dbclientcursor.cpp index 2b534defbba..325fa9c9fbd 100644 --- a/src/mongo/client/dbclientcursor.cpp +++ b/src/mongo/client/dbclientcursor.cpp @@ -40,6 +40,7 @@ #include "mongo/rpc/factory.h" #include "mongo/rpc/get_status_from_command_result.h" #include "mongo/rpc/metadata.h" +#include "mongo/rpc/object_check.h" #include "mongo/rpc/request_builder_interface.h" #include "mongo/s/stale_exception.h" #include "mongo/stdx/memory.h" @@ -346,12 +347,11 @@ BSONObj DBClientCursor::next() { uassert(13422, "DBClientCursor next() called but more() is false", batch.pos < batch.nReturned); + auto status = validateBSON(batch.data, batch.remainingBytes, _enabledBSONVersion); uassert(ErrorCodes::InvalidBSON, - "Got invalid BSON from external server while reading from cursor.", - validateBSON(batch.data, - batch.remainingBytes, - enableBSON1_1 ? BSONVersion::kV1_1 : BSONVersion::kV1_0) - .isOK()); + str::stream() << "Got invalid BSON from external server while reading from cursor" + << causedBy(status), + status.isOK()); BSONObj o(batch.data); @@ -510,7 +510,8 @@ DBClientCursor::DBClientCursor(DBClientBase* client, resultFlags(0), cursorId(cursorId), _ownCursor(true), - wasError(false) {} + wasError(false), + _enabledBSONVersion(Validator<BSONObj>::enabledBSONVersion()) {} DBClientCursor::~DBClientCursor() { kill(); diff --git a/src/mongo/client/dbclientcursor.h b/src/mongo/client/dbclientcursor.h index c0367b830a0..e75bea59a15 100644 --- a/src/mongo/client/dbclientcursor.h +++ b/src/mongo/client/dbclientcursor.h @@ -267,6 +267,7 @@ private: std::string _scopedHost; std::string _lazyHost; bool wasError; + BSONVersion _enabledBSONVersion; void dataReceived() { bool retry; diff --git a/src/mongo/db/catalog/collection.cpp b/src/mongo/db/catalog/collection.cpp index cfd67e79ede..2e4c2154385 100644 --- a/src/mongo/db/catalog/collection.cpp +++ b/src/mongo/db/catalog/collection.cpp @@ -1024,7 +1024,11 @@ public: virtual Status validate(const RecordId& recordId, const RecordData& record, size_t* dataSize) { BSONObj recordBson = record.toBson(); - const Status status = validateBSON(recordBson.objdata(), recordBson.objsize()); + + // Use the latest BSON validation version. We do not say the collection is invalid for + // containing decimal data, even if decimal is disabled. + const Status status = + validateBSON(recordBson.objdata(), recordBson.objsize(), BSONVersion::kLatest); if (status.isOK()) { *dataSize = recordBson.objsize(); } else { diff --git a/src/mongo/db/catalog/collection_compact.cpp b/src/mongo/db/catalog/collection_compact.cpp index b8fbffe2f69..e61e8606b84 100644 --- a/src/mongo/db/catalog/collection_compact.cpp +++ b/src/mongo/db/catalog/collection_compact.cpp @@ -79,7 +79,9 @@ public: : _collection(collection), _multiIndexBlock(indexBlock) {} virtual bool isDataValid(const RecordData& recData) { - return recData.toBson().valid(); + // Use the latest BSON validation version. We allow compaction of collections containing + // decimal data even if decimal is disabled. + return recData.toBson().valid(BSONVersion::kLatest); } virtual size_t dataSize(const RecordData& recData) { diff --git a/src/mongo/db/cloner.cpp b/src/mongo/db/cloner.cpp index 9afef63d2e4..ec78619b22e 100644 --- a/src/mongo/db/cloner.cpp +++ b/src/mongo/db/cloner.cpp @@ -218,7 +218,9 @@ struct Cloner::Fun { } /* assure object is valid. note this will slow us down a little. */ - const Status status = validateBSON(tmp.objdata(), tmp.objsize()); + // Use the latest BSON validation version. We allow cloning of collections containing + // decimal data even if decimal is disabled. + const Status status = validateBSON(tmp.objdata(), tmp.objsize(), BSONVersion::kLatest); if (!status.isOK()) { str::stream ss; ss << "Cloner: found corrupt document in " << from_collection.toString() << ": " diff --git a/src/mongo/db/dbmessage.cpp b/src/mongo/db/dbmessage.cpp index f16ec2b6276..4b5f5054ccf 100644 --- a/src/mongo/db/dbmessage.cpp +++ b/src/mongo/db/dbmessage.cpp @@ -125,7 +125,10 @@ BSONObj DbMessage::nextJsObj() { _nextjsobj != NULL && _theEnd - _nextjsobj >= 5); if (serverGlobalParams.objcheck) { - Status status = validateBSON(_nextjsobj, _theEnd - _nextjsobj); + // TODO SERVER-23972: Change BSONVersion::kLatest to + // Validator<BSONObj>::enabledBSONVersion() so that we disallow decimal inserts in legacy + // write mode if decimal is disabled. + Status status = validateBSON(_nextjsobj, _theEnd - _nextjsobj, BSONVersion::kLatest); massert(10307, str::stream() << "Client Error: bad object in message: " << status.reason(), status.isOK()); diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp index 6a9b909a827..790c492e7a5 100644 --- a/src/mongo/db/instance.cpp +++ b/src/mongo/db/instance.cpp @@ -142,7 +142,7 @@ void generateLegacyQueryErrorResponse(const AssertionException* exception, curop->debug().exceptionInfo = exception->getInfo(); log(LogComponent::kQuery) << "assertion " << exception->toString() << " ns:" << queryMessage.ns - << " query:" << (queryMessage.query.valid() + << " query:" << (queryMessage.query.valid(BSONVersion::kLatest) ? queryMessage.query.toString() : "query object is corrupt"); if (queryMessage.ntoskip || queryMessage.ntoreturn) { diff --git a/src/mongo/db/lasterror.cpp b/src/mongo/db/lasterror.cpp index 8789609e2cd..370c38f9f6f 100644 --- a/src/mongo/db/lasterror.cpp +++ b/src/mongo/db/lasterror.cpp @@ -58,7 +58,10 @@ void LastError::recordUpdate(bool updateObjects, long long nObjects, BSONObj ups reset(true); _nObjects = nObjects; _updatedExisting = updateObjects ? True : False; - if (upsertedId.valid() && upsertedId.hasField(kUpsertedFieldName)) + + // Use the latest BSON validation version. We record updates containing decimal data even if + // decimal is disabled. + if (upsertedId.valid(BSONVersion::kLatest) && upsertedId.hasField(kUpsertedFieldName)) _upsertedId = upsertedId; } diff --git a/src/mongo/db/repair_database.cpp b/src/mongo/db/repair_database.cpp index f34050db044..4f5665e55da 100644 --- a/src/mongo/db/repair_database.cpp +++ b/src/mongo/db/repair_database.cpp @@ -138,7 +138,9 @@ Status rebuildIndexesOnCollection(OperationContext* txn, RecordId id = record->id; RecordData& data = record->data; - Status status = validateBSON(data.data(), data.size()); + // Use the latest BSON validation version. We retain decimal data when repairing the + // database even if decimal is disabled. + Status status = validateBSON(data.data(), data.size(), BSONVersion::kLatest); if (!status.isOK()) { log() << "Invalid BSON detected at " << id << ": " << redact(status) << ". Deleting."; cursor->save(); // 'data' is no longer valid. diff --git a/src/mongo/db/storage/oplog_hack.cpp b/src/mongo/db/storage/oplog_hack.cpp index e42946e5611..730466f79dc 100644 --- a/src/mongo/db/storage/oplog_hack.cpp +++ b/src/mongo/db/storage/oplog_hack.cpp @@ -65,7 +65,9 @@ StatusWith<RecordId> keyForOptime(const Timestamp& opTime) { * data and len must be the arguments from RecordStore::insert() on an oplog collection. */ StatusWith<RecordId> extractKey(const char* data, int len) { - DEV invariant(validateBSON(data, len).isOK()); + // Use the latest BSON validation version. Oplog entries are allowed to contain decimal data + // even if decimal is disabled. + DEV invariant(validateBSON(data, len, BSONVersion::kLatest).isOK()); const BSONObj obj(data); const BSONElement elem = obj["ts"]; diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp index 45f96c5040d..d968a628c43 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_index.cpp @@ -41,6 +41,7 @@ #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/index/index_descriptor.h" #include "mongo/db/json.h" +#include "mongo/db/server_options.h" #include "mongo/db/service_context.h" #include "mongo/db/storage/key_string.h" #include "mongo/db/storage/storage_options.h" @@ -196,8 +197,11 @@ StatusWith<std::string> WiredTigerIndex::generateCreateString(const std::string& // Index metadata ss << ",app_metadata=(" - << "formatVersion=" << (enableBSON1_1 ? kKeyStringV1Version : kKeyStringV0Version) << ',' - << "infoObj=" << desc.infoObj().jsonString() << "),"; + << "formatVersion=" << (serverGlobalParams.featureCompatibilityVersion.load() == + ServerGlobalParams::FeatureCompatibilityVersion_34 + ? kKeyStringV1Version + : kKeyStringV0Version) + << ',' << "infoObj=" << desc.infoObj().jsonString() << "),"; LOG(3) << "index create string: " << ss.ss.str(); return StatusWith<std::string>(ss); diff --git a/src/mongo/db/views/durable_view_catalog.cpp b/src/mongo/db/views/durable_view_catalog.cpp index ba84432b7cf..c8dd9119023 100644 --- a/src/mongo/db/views/durable_view_catalog.cpp +++ b/src/mongo/db/views/durable_view_catalog.cpp @@ -78,7 +78,9 @@ Status DurableViewCatalogImpl::iterate(OperationContext* txn, Callback callback) RecordData& data = record->data; // Check the document is valid BSON, with only the expected fields. - fassertStatusOK(40224, validateBSON(data.data(), data.size())); + // Use the latest BSON validation version. Existing view definitions are allowed to contain + // decimal data even if decimal is disabled. + fassertStatusOK(40224, validateBSON(data.data(), data.size(), BSONVersion::kLatest)); BSONObj viewDef = data.toBson(); // Check read definitions for correct structure, and refuse reading past invalid diff --git a/src/mongo/dbtests/jsobjtests.cpp b/src/mongo/dbtests/jsobjtests.cpp index a6b3fea9547..2334cadd512 100644 --- a/src/mongo/dbtests/jsobjtests.cpp +++ b/src/mongo/dbtests/jsobjtests.cpp @@ -582,7 +582,7 @@ public: bb << "a" << 1; BSONObj tmp = bb.asTempObj(); ASSERT(tmp.objsize() == 4 + (1 + 2 + 4) + 1); - ASSERT(tmp.valid()); + ASSERT(tmp.valid(BSONVersion::kLatest)); ASSERT(tmp.hasField("a")); ASSERT(!tmp.hasField("b")); ASSERT_BSONOBJ_EQ(tmp, BSON("a" << 1)); @@ -590,7 +590,7 @@ public: bb << "b" << 2; BSONObj obj = bb.obj(); ASSERT_EQUALS(obj.objsize(), 4 + (1 + 2 + 4) + (1 + 2 + 4) + 1); - ASSERT(obj.valid()); + ASSERT(obj.valid(BSONVersion::kLatest)); ASSERT(obj.hasField("a")); ASSERT(obj.hasField("b")); ASSERT_BSONOBJ_EQ(obj, BSON("a" << 1 << "b" << 2)); @@ -600,7 +600,7 @@ public: bb << "a" << GT << 1; BSONObj tmp = bb.asTempObj(); ASSERT(tmp.objsize() == 4 + (1 + 2 + (4 + 1 + 4 + 4 + 1)) + 1); - ASSERT(tmp.valid()); + ASSERT(tmp.valid(BSONVersion::kLatest)); ASSERT(tmp.hasField("a")); ASSERT(!tmp.hasField("b")); ASSERT_BSONOBJ_EQ(tmp, BSON("a" << BSON("$gt" << 1))); @@ -609,7 +609,7 @@ public: BSONObj obj = bb.obj(); ASSERT(obj.objsize() == 4 + (1 + 2 + (4 + 1 + 4 + 4 + 1)) + (1 + 2 + (4 + 1 + 4 + 4 + 1)) + 1); - ASSERT(obj.valid()); + ASSERT(obj.valid(BSONVersion::kLatest)); ASSERT(obj.hasField("a")); ASSERT(obj.hasField("b")); ASSERT_BSONOBJ_EQ(obj, BSON("a" << BSON("$gt" << 1) << "b" << BSON("$lt" << 2))); @@ -619,7 +619,7 @@ public: bb << "a" << 1; BSONObj tmp = bb.asTempObj(); ASSERT(tmp.objsize() == 4 + (1 + 2 + 4) + 1); - ASSERT(tmp.valid()); + ASSERT(tmp.valid(BSONVersion::kLatest)); ASSERT(tmp.hasField("a")); ASSERT(!tmp.hasField("b")); ASSERT_BSONOBJ_EQ(tmp, BSON("a" << 1)); @@ -631,7 +631,7 @@ public: } bb << "b" << arr.arr(); BSONObj obj = bb.obj(); - ASSERT(obj.valid()); + ASSERT(obj.valid(BSONVersion::kLatest)); ASSERT(obj.hasField("a")); ASSERT(obj.hasField("b")); } @@ -1070,8 +1070,8 @@ class Base { public: virtual ~Base() {} void run() { - ASSERT(valid().valid()); - ASSERT(!invalid().valid()); + ASSERT(valid().valid(BSONVersion::kLatest)); + ASSERT(!invalid().valid(BSONVersion::kLatest)); } protected: @@ -1120,7 +1120,7 @@ public: b.appendNull("a"); BSONObj o = b.done(); set(o, 4, mongo::Undefined); - ASSERT(o.valid()); + ASSERT(o.valid(BSONVersion::kLatest)); } }; @@ -1303,7 +1303,7 @@ public: void run() { const char data[] = {0x07, 0x00, 0x00, 0x00, char(type_), 'a', 0x00}; BSONObj o(data); - ASSERT(!o.valid()); + ASSERT(!o.valid(BSONVersion::kLatest)); } private: @@ -1666,7 +1666,7 @@ public: b2.done(); b1.append("f", 10.0); BSONObj ret = b1.done(); - ASSERT(ret.valid()); + ASSERT(ret.valid(BSONVersion::kLatest)); ASSERT(ret.woCompare(fromjson("{a:'bcd',foo:{ggg:44},f:10}")) == 0); } }; @@ -1687,7 +1687,7 @@ public: BSONObj o = BSON("now" << DATENOW); Date_t after = jsTime(); - ASSERT(o.valid()); + ASSERT(o.valid(BSONVersion::kLatest)); BSONElement e = o["now"]; ASSERT(e.type() == Date); @@ -1705,7 +1705,7 @@ public: b.appendTimeT("now", aTime); BSONObj o = b.obj(); - ASSERT(o.valid()); + ASSERT(o.valid(BSONVersion::kLatest)); BSONElement e = o["now"]; ASSERT_EQUALS(Date, e.type()); @@ -1719,8 +1719,8 @@ public: BSONObj min = BSON("a" << MINKEY); BSONObj max = BSON("b" << MAXKEY); - ASSERT(min.valid()); - ASSERT(max.valid()); + ASSERT(min.valid(BSONVersion::kLatest)); + ASSERT(max.valid(BSONVersion::kLatest)); BSONElement minElement = min["a"]; BSONElement maxElement = max["b"]; diff --git a/src/mongo/dbtests/jsontests.cpp b/src/mongo/dbtests/jsontests.cpp index ecd156ed5a6..5ba320b3297 100644 --- a/src/mongo/dbtests/jsontests.cpp +++ b/src/mongo/dbtests/jsontests.cpp @@ -625,7 +625,7 @@ class Base { public: virtual ~Base() {} void run() { - ASSERT(fromjson(json()).valid()); + ASSERT(fromjson(json()).valid(BSONVersion::kLatest)); assertEquals(bson(), fromjson(json()), "mode: json-to-bson"); assertEquals(bson(), fromjson(tojson(bson())), "mode: <default>"); assertEquals(bson(), fromjson(tojson(bson(), Strict)), "mode: strict"); diff --git a/src/mongo/dbtests/jstests.cpp b/src/mongo/dbtests/jstests.cpp index 3ec0130cdb2..d9dd6b69ce7 100644 --- a/src/mongo/dbtests/jstests.cpp +++ b/src/mongo/dbtests/jstests.cpp @@ -789,6 +789,11 @@ public: class NumberDecimal { public: void run() { + // Set the featureCompatibilityVersion to 3.4 so that BSON validation always uses + // BSONVersion::kLatest. + serverGlobalParams.featureCompatibilityVersion.store( + ServerGlobalParams::FeatureCompatibilityVersion_34); + unique_ptr<Scope> s(globalScriptEngine->newScope()); BSONObjBuilder b; Decimal128 val = Decimal128("2.010"); @@ -818,6 +823,11 @@ public: class NumberDecimalGetFromScope { public: void run() { + // Set the featureCompatibilityVersion to 3.4 so that BSON validation always uses + // BSONVersion::kLatest. + serverGlobalParams.featureCompatibilityVersion.store( + ServerGlobalParams::FeatureCompatibilityVersion_34); + unique_ptr<Scope> s(globalScriptEngine->newScope()); ASSERT(s->exec("a = 5;", "a", false, true, false)); ASSERT_TRUE(Decimal128(5).isEqual(s->getNumberDecimal("a"))); @@ -827,6 +837,11 @@ public: class NumberDecimalBigObject { public: void run() { + // Set the featureCompatibilityVersion to 3.4 so that BSON validation always uses + // BSONVersion::kLatest. + serverGlobalParams.featureCompatibilityVersion.store( + ServerGlobalParams::FeatureCompatibilityVersion_34); + unique_ptr<Scope> s(globalScriptEngine->newScope()); BSONObj in; @@ -1121,6 +1136,11 @@ public: void run() { // Insert in Javascript -> Find using DBDirectClient + // Set the featureCompatibilityVersion to 3.4 so that BSON validation always uses + // BSONVersion::kLatest. + serverGlobalParams.featureCompatibilityVersion.store( + ServerGlobalParams::FeatureCompatibilityVersion_34); + // Drop the collection const ServiceContext::UniqueOperationContext txnPtr = cc().makeOperationContext(); OperationContext& txn = *txnPtr; diff --git a/src/mongo/dbtests/querytests.cpp b/src/mongo/dbtests/querytests.cpp index cdcca08b505..888d9bd4454 100644 --- a/src/mongo/dbtests/querytests.cpp +++ b/src/mongo/dbtests/querytests.cpp @@ -1200,7 +1200,7 @@ public: unique_ptr<DBClientCursor> cursor = _client.query(ns, Query().sort("7")); while (cursor->more()) { BSONObj o = cursor->next(); - verify(o.valid()); + verify(o.valid(BSONVersion::kLatest)); // cout << " foo " << o << endl; } } diff --git a/src/mongo/rpc/SConscript b/src/mongo/rpc/SConscript index c3b2b73ddaf..fcd14efee87 100644 --- a/src/mongo/rpc/SConscript +++ b/src/mongo/rpc/SConscript @@ -51,7 +51,6 @@ env.Library( ], source=[ 'factory.cpp', - 'object_check.cpp', ], LIBDEPS=[ 'command_reply', @@ -59,14 +58,27 @@ env.Library( 'legacy_reply', 'legacy_request', 'metadata', + 'object_check', 'protocol', - '$BUILD_DIR/mongo/db/server_parameters', '$BUILD_DIR/mongo/db/dbmessage', ], ) env.Library( target=[ + 'object_check', + ], + source=[ + 'object_check.cpp', + ], + LIBDEPS=[ + '$BUILD_DIR/mongo/base', + '$BUILD_DIR/mongo/db/server_parameters', + ], +) + +env.Library( + target=[ 'command_request', ], source=[ @@ -122,6 +134,7 @@ env.Library( 'command_reply', 'document_range', 'metadata', + 'object_check', '$BUILD_DIR/mongo/util/net/network', ], ) diff --git a/src/mongo/rpc/legacy_reply.cpp b/src/mongo/rpc/legacy_reply.cpp index 572a96d2dd2..97aef06c4ca 100644 --- a/src/mongo/rpc/legacy_reply.cpp +++ b/src/mongo/rpc/legacy_reply.cpp @@ -36,6 +36,7 @@ #include "mongo/bson/bson_validate.h" #include "mongo/rpc/legacy_reply_builder.h" #include "mongo/rpc/metadata.h" +#include "mongo/rpc/object_check.h" #include "mongo/util/assert_util.h" #include "mongo/util/mongoutils/str.h" @@ -68,14 +69,11 @@ LegacyReply::LegacyReply(const Message* message) : _message(std::move(message)) << qr.getStartingFrom(), qr.getStartingFrom() == 0); - if (serverGlobalParams.objcheck) { - uassert(ErrorCodes::InvalidBSON, - "Got legacy command reply with invalid BSON in the metadata field.", - validateBSON(qr.data(), - qr.dataLen(), - enableBSON1_1 ? BSONVersion::kV1_1 : BSONVersion::kV1_0) - .isOK()); - } + auto status = Validator<BSONObj>::validateLoad(qr.data(), qr.dataLen()); + uassert(ErrorCodes::InvalidBSON, + str::stream() << "Got legacy command reply with invalid BSON in the metadata field" + << causedBy(status), + status.isOK()); std::tie(_commandReply, _metadata) = uassertStatusOK(rpc::upconvertReplyMetadata(BSONObj(qr.data()))); diff --git a/src/mongo/rpc/object_check.cpp b/src/mongo/rpc/object_check.cpp index c542e94b8f9..98ddda58697 100644 --- a/src/mongo/rpc/object_check.cpp +++ b/src/mongo/rpc/object_check.cpp @@ -31,17 +31,8 @@ #include "mongo/rpc/object_check.h" #include "mongo/base/status.h" -#include "mongo/db/jsobj.h" -#include "mongo/db/server_options.h" -#include "mongo/db/server_parameters.h" namespace mongo { -ExportedServerParameter<bool, ServerParameterType::kStartupOnly> enableBSON1_1Parameter( - ServerParameterSet::getGlobal(), "enableBSON1_1", &enableBSON1_1); - -BSONVersion Validator<BSONObj>::validateVersion() { - return enableBSON1_1 ? BSONVersion::kV1_1 : BSONVersion::kV1_0; -} Status Validator<BSONObj>::validateStore(const BSONObj& toStore) { return Status::OK(); diff --git a/src/mongo/rpc/object_check.h b/src/mongo/rpc/object_check.h index 8c8be115136..c4f999fde20 100644 --- a/src/mongo/rpc/object_check.h +++ b/src/mongo/rpc/object_check.h @@ -44,12 +44,16 @@ class Status; */ template <> struct Validator<BSONObj> { - static BSONVersion validateVersion(); + inline static BSONVersion enabledBSONVersion() { + return serverGlobalParams.featureCompatibilityVersion.load() == + ServerGlobalParams::FeatureCompatibilityVersion_34 + ? BSONVersion::kV1_1 + : BSONVersion::kV1_0; + } inline static Status validateLoad(const char* ptr, size_t length) { - return serverGlobalParams.objcheck - ? validateBSON(ptr, length, enableBSON1_1 ? BSONVersion::kV1_1 : BSONVersion::kV1_0) - : Status::OK(); + return serverGlobalParams.objcheck ? validateBSON(ptr, length, enabledBSONVersion()) + : Status::OK(); } static Status validateStore(const BSONObj& toStore); diff --git a/src/mongo/s/server.cpp b/src/mongo/s/server.cpp index 874cee5929f..cd98e0cc6dc 100644 --- a/src/mongo/s/server.cpp +++ b/src/mongo/s/server.cpp @@ -326,6 +326,15 @@ MONGO_INITIALIZER_GENERAL(ForkServer, ("EndStartupOptionHandling"), ("default")) return Status::OK(); } +// We set the featureCompatibilityVersion to 3.4 in the mongos so that BSON validation always uses +// BSONVersion::kLatest. +MONGO_INITIALIZER_WITH_PREREQUISITES(SetFeatureCompatibilityVersion34, ("EndStartupOptionStorage")) +(InitializerContext* context) { + mongo::serverGlobalParams.featureCompatibilityVersion.store( + ServerGlobalParams::FeatureCompatibilityVersion_34); + return Status::OK(); +} + /* * This function should contain the startup "actions" that we take based on the startup config. It * is intended to separate the actions from "storage" and "validation" of our startup configuration. diff --git a/src/mongo/shell/dbshell.cpp b/src/mongo/shell/dbshell.cpp index 9fc6b6de5df..e46f98bdb34 100644 --- a/src/mongo/shell/dbshell.cpp +++ b/src/mongo/shell/dbshell.cpp @@ -39,6 +39,7 @@ #include <stdio.h> #include <string.h> +#include "mongo/base/init.h" #include "mongo/base/initializer.h" #include "mongo/base/status.h" #include "mongo/client/dbclientinterface.h" @@ -87,6 +88,15 @@ static volatile bool atPrompt = false; // can eval before getting to prompt namespace { const auto kDefaultMongoURL = "mongodb://127.0.0.1:27017"_sd; + +// We set the featureCompatibilityVersion to 3.4 in the mongo shell so that BSON validation always +// uses BSONVersion::kLatest. +MONGO_INITIALIZER_WITH_PREREQUISITES(SetFeatureCompatibilityVersion34, ("EndStartupOptionSetup")) +(InitializerContext* context) { + mongo::serverGlobalParams.featureCompatibilityVersion.store( + ServerGlobalParams::FeatureCompatibilityVersion_34); + return Status::OK(); +} } namespace mongo { diff --git a/src/mongo/tools/sniffer.cpp b/src/mongo/tools/sniffer.cpp index 70b1d77dfb3..0808cf1e34d 100644 --- a/src/mongo/tools/sniffer.cpp +++ b/src/mongo/tools/sniffer.cpp @@ -282,7 +282,7 @@ public: AuditingDbMessage(const Message& m) : DbMessage(m) {} BSONObj nextJsObj(const char* context) { BSONObj ret = DbMessage::nextJsObj(); - if (objcheck && !ret.valid()) { + if (objcheck && !ret.valid(mongo::BSONVersion::kLatest)) { // TODO provide more debugging info cout << "invalid object in " << context << ": " << ret.hexDump() << endl; } |