From cab9719304212aa0de423816f03c1f9e0cd1fb6b Mon Sep 17 00:00:00 2001 From: Daniel Gottlieb Date: Thu, 19 Mar 2020 09:46:20 -0400 Subject: SERVER-46945: Correct FCV-aware error messages for overlong collection names. Remove test coverage that exercises cases where namespace length approaches the bson document size limit. --- ...ify_metadata_when_durable_catalog_entry_full.js | 119 --------------------- src/mongo/db/catalog/database_impl.cpp | 25 +++-- src/mongo/db/namespace_string.cpp | 19 ---- src/mongo/db/namespace_string.h | 6 +- src/mongo/db/ops/insert.cpp | 7 -- 5 files changed, 15 insertions(+), 161 deletions(-) delete mode 100644 jstests/noPassthroughWithMongod/modify_metadata_when_durable_catalog_entry_full.js diff --git a/jstests/noPassthroughWithMongod/modify_metadata_when_durable_catalog_entry_full.js b/jstests/noPassthroughWithMongod/modify_metadata_when_durable_catalog_entry_full.js deleted file mode 100644 index cdfd1bcfc29..00000000000 --- a/jstests/noPassthroughWithMongod/modify_metadata_when_durable_catalog_entry_full.js +++ /dev/null @@ -1,119 +0,0 @@ -/** - * Tests that basic operations can be performed while the metadata document in the catalog is at the - * size of the BSON document limit. - */ -(function() { -'use strict'; - -const dbName = 'modify_metadata_when_full'; -const testDB = db.getSiblingDB(dbName); - -/** - * Returns the largest size of a collection name length possible where creating a collection with - * one more character would fail. - */ -function createUntilFails(db, startingSize, increment) { - if (increment == 1) { - let newSize = startingSize; - let collName = 'a'.repeat(newSize); - - while (db.createCollection(collName).ok) { - assert.eq(true, db.getCollection(collName).drop()); - - newSize++; - collName = 'a'.repeat(newSize); - } - - // Subtract one to get the largest possible collection name length. - newSize -= 1; - return newSize; - } - - let newSize = startingSize + increment; - let collName = 'a'.repeat(newSize); - - let res = db.createCollection(collName); - if (res.ok) { - assert.eq(true, db.getCollection(collName).drop()); - return createUntilFails(db, newSize, increment); - } else { - assert.eq("BSONObjectTooLarge", res.codeName); - return createUntilFails(db, startingSize, Math.floor(increment / 2)); - } -} - -/* - * Creates the largest possible collection in terms of name length and returns it. - */ -function createLargeCollection(db) { - // Divide by two because 'ns' field is stored twice in the catalog. - const maxBsonObjectSize = db.isMaster().maxBsonObjectSize / 2; - let maxCollNameSize = maxBsonObjectSize; - let maxCollName = 'a'.repeat(maxCollNameSize); - - assert.commandWorked(testDB.createCollection(maxCollName)); - assert.eq(true, testDB.getCollection(maxCollName).drop()); - - maxCollNameSize = createUntilFails(db, maxCollNameSize, 1000); - - // Take away a few characters and ensure the collection creation works. - maxCollName = 'b'.repeat(maxCollNameSize - 10); - - // Add a couple extra characters and ensure the collection creation fails. - let nameTooBig = 'c'.repeat(maxCollNameSize + 10); - assert.commandFailedWithCode(testDB.createCollection(nameTooBig), - ErrorCodes.BSONObjectTooLarge); - - assert.commandWorked(testDB.createCollection(maxCollName)); - return testDB.getCollection(maxCollName); -} - -let largeColl = createLargeCollection(testDB); - -// Ensure creating another collection works. -let smallCollName = 'd'.repeat(10000); -assert.commandWorked(testDB.createCollection(smallCollName)); -let smallColl = testDB.getCollection(smallCollName); - -// The 'top' command should succeed even with a long collection name. -assert.commandWorked(largeColl.getDB().adminCommand('top')); - -// We should be able to add another collection with a long name successfully. After doing so, then -// the amount of data which Top needs to return will exceed the maximum BSON size, causing the -// command to fail. -const secondLargeCollName = 'e'.repeat(largeColl.getName().length); -assert.commandWorked(testDB.createCollection(secondLargeCollName)); -const secondLargeColl = testDB.getCollection(secondLargeCollName); -assert.commandFailedWithCode(testDB.adminCommand('top'), [13548, ErrorCodes.BSONObjectTooLarge]); - -// Adding indexes to the large collection should fail but not crash the server. -assert.commandFailedWithCode(largeColl.createIndex({x: 1}), ErrorCodes.BSONObjectTooLarge); -assert.commandWorked(smallColl.createIndex({x: 1})); - -// Inserting documents should work because it doesn't interact with the metadata document in the -// catalog. -assert.commandWorked(largeColl.insertMany([{x: 1}, {y: 2}, {z: 3}])); -assert.commandWorked(smallColl.insertMany([{x: 1}, {y: 2}, {z: 3}])); - -// Renaming the collection should work to get ourselves out of situations where the collection name -// is too long for operations. -let largeCollName = largeColl.getFullName(); -let adminDB = db.getSiblingDB('admin'); -assert.commandWorked(adminDB.runCommand( - {renameCollection: largeCollName, to: 'modify_metadata_when_full.smallName'})); -assert.commandWorked(adminDB.runCommand( - {renameCollection: 'modify_metadata_when_full.smallName', to: largeCollName})); - -// Renaming the small collection should fail because it has one more index than the large -// collection. -let otherLargeCollName = 'modify_metadata_when_full.' + - 'f'.repeat(largeColl.getName().length); -assert.commandFailedWithCode( - adminDB.runCommand({renameCollection: smallColl.getFullName(), to: otherLargeCollName}), - ErrorCodes.BSONObjectTooLarge); - -// Dropping both collections should work. -assert.eq(true, largeColl.drop()); -assert.eq(true, smallColl.drop()); -assert.eq(true, secondLargeColl.drop()); -}()); diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp index b1f70b658c9..30042647a1c 100644 --- a/src/mongo/db/catalog/database_impl.cpp +++ b/src/mongo/db/catalog/database_impl.cpp @@ -584,23 +584,26 @@ void DatabaseImpl::_checkCanCreateCollection(OperationContext* opCtx, str::stream() << "cannot create a collection with an empty name on db: " << nss.db(), !nss.coll().empty()); - uassert(17361, - str::stream() << "Fully qualified namespace is too long. Namespace: " << nss.ns() - << " Max: " << NamespaceString::MaxNsCollectionLen, - !nss.isNormalCollection() || nss.size() <= NamespaceString::MaxNsCollectionLen); - uassert(28838, "cannot create a non-capped oplog collection", options.capped || !nss.isOplog()); uassert(ErrorCodes::DatabaseDropPending, str::stream() << "Cannot create collection " << nss << " - database is in the process of being dropped.", !_dropPending.load()); - uassert(ErrorCodes::IncompatibleServerVersion, - str::stream() << "Cannot create collection with a long name " << nss - << " - upgrade to feature compatibility version " - << FeatureCompatibilityVersionParser::kVersion44 - << " to be able to do so.", - nss.checkLengthForFCV()); + uassert(17381, + str::stream() << "Fully qualified namespace is too long. Namespace: " << nss.ns() + << " Max: " << NamespaceString::MaxNsCollectionLen, + !nss.isNormalCollection() || nss.size() <= NamespaceString::MaxNsCollectionLen); + const auto& fcv = serverGlobalParams.featureCompatibility; + if (!fcv.isVersionInitialized() || + fcv.getVersion() < ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo44) { + uassert(ErrorCodes::IncompatibleServerVersion, + str::stream() << "Fully qualified namespace is too long for FCV 4.2. Upgrade to " + "FCV 4.4 to create this namespace. Namespace: " + << nss.ns() + << " FCV 4.2 Limit: " << NamespaceString::MaxNSCollectionLenFCV42, + nss.size() <= NamespaceString::MaxNSCollectionLenFCV42); + } } Status DatabaseImpl::createView(OperationContext* opCtx, diff --git a/src/mongo/db/namespace_string.cpp b/src/mongo/db/namespace_string.cpp index 54b333ed0a7..ba999a583fc 100644 --- a/src/mongo/db/namespace_string.cpp +++ b/src/mongo/db/namespace_string.cpp @@ -149,25 +149,6 @@ bool NamespaceString::isDropPendingNamespace() const { return coll().startsWith(dropPendingNSPrefix); } -bool NamespaceString::checkLengthForFCV() const { - // Prior to longer collection names, the limit in older versions was 120 characters. - const size_t previousMaxNSLength = 120U; - if (size() <= previousMaxNSLength) { - return true; - } - - if (!serverGlobalParams.featureCompatibility.isVersionInitialized()) { - return false; - } - - if (serverGlobalParams.featureCompatibility.getVersion() == - ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo44) { - return size() <= MaxNsCollectionLen; - } - - return false; -} - NamespaceString NamespaceString::makeDropPendingNamespace(const repl::OpTime& opTime) const { StringBuilder ss; ss << db() << "." << dropPendingNSPrefix; diff --git a/src/mongo/db/namespace_string.h b/src/mongo/db/namespace_string.h index 598ff87c4f0..ce720cffffb 100644 --- a/src/mongo/db/namespace_string.h +++ b/src/mongo/db/namespace_string.h @@ -47,6 +47,7 @@ class NamespaceString { public: constexpr static size_t MaxDatabaseNameLen = 128; // max str len for the db name, including null char + constexpr static size_t MaxNSCollectionLenFCV42 = 120U; constexpr static size_t MaxNsCollectionLen = 255; // Reserved system namespaces @@ -297,11 +298,6 @@ public: */ bool isDropPendingNamespace() const; - /** - * Returns true if the namespace length is valid based on the FCV setting. - */ - bool checkLengthForFCV() const; - /** * Returns the drop-pending namespace name for this namespace, provided the given optime. * diff --git a/src/mongo/db/ops/insert.cpp b/src/mongo/db/ops/insert.cpp index 1b053dea4fc..877545a9cda 100644 --- a/src/mongo/db/ops/insert.cpp +++ b/src/mongo/db/ops/insert.cpp @@ -203,13 +203,6 @@ Status userAllowedCreateNS(StringData db, StringData coll) { if (!NamespaceString::validCollectionName(coll)) return Status(ErrorCodes::InvalidNamespace, "invalid collection name"); - if (!NamespaceString(db, coll).checkLengthForFCV()) - return Status(ErrorCodes::IncompatibleServerVersion, - str::stream() << "Cannot create collection with a long name " << db << "." - << coll << " - upgrade to feature compatibility version " - << FeatureCompatibilityVersionParser::kVersion44 - << " to be able to do so."); - // check special areas if (db == "system") -- cgit v1.2.1