SERVER-30371 renameCollection() across databases returns InvalidLength if source collection's indexes are too long for temporary collection

3 files changed, 76 insertions, 1 deletions

diff --git a/src/mongo/db/catalog/SConscript b/src/mongo/db/catalog/SConscript
index 9479faf9f4c..1a06fe1ba80 100644
--- a/src/mongo/db/catalog/SConscript
+++ b/src/mongo/db/catalog/SConscript
@@ -279,6 +279,7 @@ env.CppUnitTest(
     ],
     LIBDEPS=[
         'catalog_helpers',
+        'index_create',
         '$BUILD_DIR/mongo/db/db_raii',
         '$BUILD_DIR/mongo/db/namespace_string',
         '$BUILD_DIR/mongo/db/repl/optime',
diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp
index 788b1bc4303..30a1ac28d9c 100644
--- a/src/mongo/db/catalog/rename_collection.cpp
+++ b/src/mongo/db/catalog/rename_collection.cpp
@@ -244,6 +244,13 @@ Status renameCollectionCommon(OperationContext* opCtx,
                                     << tmpNameResult.getStatus().reason());
     }
     const auto& tmpName = tmpNameResult.getValue();
+
+    // Check if all the source collection's indexes can be recreated in the temporary collection.
+    status = tmpName.checkLengthForRename(longestIndexNameLength);
+    if (!status.isOK()) {
+        return status;
+    }
+
     Collection* tmpColl = nullptr;
     OptionalCollectionUUID newUUID;
     bool isSourceCollectionTemporary = false;
@@ -301,7 +308,10 @@ Status renameCollectionCommon(OperationContext* opCtx,
             }
             indexesToCopy.push_back(newIndex.obj());
         }
-        indexer.init(indexesToCopy).status_with_transitional_ignore();
+        status = indexer.init(indexesToCopy).getStatus();
+        if (!status.isOK()) {
+            return status;
+        }
     }
 
     {
diff --git a/src/mongo/db/catalog/rename_collection_test.cpp b/src/mongo/db/catalog/rename_collection_test.cpp
index 02edeeade89..fa32ebaac77 100644
--- a/src/mongo/db/catalog/rename_collection_test.cpp
+++ b/src/mongo/db/catalog/rename_collection_test.cpp
@@ -32,6 +32,7 @@
 
 #include "mongo/db/catalog/collection_catalog_entry.h"
 #include "mongo/db/catalog/collection_options.h"
+#include "mongo/db/catalog/index_create.h"
 #include "mongo/db/catalog/rename_collection.h"
 #include "mongo/db/client.h"
 #include "mongo/db/concurrency/write_conflict_exception.h"
@@ -229,6 +230,34 @@ bool _isTempCollection(OperationContext* opCtx, const NamespaceString& nss) {
     return options.temp;
 }
 
+/**
+ * Creates an index using the given index name with a bogus key spec.
+ */
+void _createIndex(OperationContext* opCtx,
+                  const NamespaceString& nss,
+                  const std::string& indexName) {
+    writeConflictRetry(opCtx, "_createIndex", nss.ns(), [=] {
+        AutoGetCollection autoColl(opCtx, nss, MODE_X);
+        auto collection = autoColl.getCollection();
+        ASSERT_TRUE(collection) << "Cannot create index in collection " << nss
+                                << " because collection " << nss.ns() << " does not exist.";
+
+        auto indexInfoObj = BSON(
+            "v" << int(IndexDescriptor::kLatestIndexVersion) << "key" << BSON("a" << 1) << "name"
+                << indexName
+                << "ns"
+                << nss.ns());
+
+        MultiIndexBlock indexer(opCtx, collection);
+        ASSERT_OK(indexer.init(indexInfoObj).getStatus());
+        WriteUnitOfWork wuow(opCtx);
+        indexer.commit();
+        wuow.commit();
+    });
+
+    ASSERT_TRUE(AutoGetCollectionForRead(opCtx, nss).getCollection());
+}
+
 TEST_F(RenameCollectionTest, RenameCollectionReturnsNamespaceNotFoundIfDatabaseDoesNotExist) {
     ASSERT_FALSE(AutoGetDb(_opCtx.get(), _sourceNss.db(), MODE_X).getDb());
     ASSERT_EQUALS(ErrorCodes::NamespaceNotFound,
@@ -244,6 +273,41 @@ TEST_F(RenameCollectionTest, RenameCollectionReturnsNotMasterIfNotPrimary) {
                   renameCollection(_opCtx.get(), _sourceNss, _targetNss, {}));
 }
 
+TEST_F(RenameCollectionTest, IndexNameTooLongForTargetCollection) {
+    ASSERT_GREATER_THAN(_targetNssDifferentDb.size(), _sourceNss.size());
+    std::size_t longestIndexNameAllowedForSource =
+        NamespaceString::MaxNsLen - 2U /*strlen(".$")*/ - _sourceNss.size();
+    ASSERT_OK(_sourceNss.checkLengthForRename(longestIndexNameAllowedForSource));
+    ASSERT_EQUALS(ErrorCodes::InvalidLength,
+                  _targetNssDifferentDb.checkLengthForRename(longestIndexNameAllowedForSource));
+
+    _createCollection(_opCtx.get(), _sourceNss);
+    const std::string indexName(longestIndexNameAllowedForSource, 'a');
+    _createIndex(_opCtx.get(), _sourceNss, indexName);
+    ASSERT_EQUALS(ErrorCodes::InvalidLength,
+                  renameCollection(_opCtx.get(), _sourceNss, _targetNssDifferentDb, {}));
+}
+
+TEST_F(RenameCollectionTest, IndexNameTooLongForTemporaryCollectionForRenameAcrossDatabase) {
+    ASSERT_GREATER_THAN(_targetNssDifferentDb.size(), _sourceNss.size());
+    std::size_t longestIndexNameAllowedForTarget =
+        NamespaceString::MaxNsLen - 2U /*strlen(".$")*/ - _targetNssDifferentDb.size();
+    ASSERT_OK(_sourceNss.checkLengthForRename(longestIndexNameAllowedForTarget));
+    ASSERT_OK(_targetNssDifferentDb.checkLengthForRename(longestIndexNameAllowedForTarget));
+
+    // Using XXXXX to check namespace length. Each 'X' will be replaced by a random character in
+    // renameCollection().
+    const NamespaceString tempNss(_targetNssDifferentDb.getSisterNS("tmpXXXXX.renameCollection"));
+    ASSERT_EQUALS(ErrorCodes::InvalidLength,
+                  tempNss.checkLengthForRename(longestIndexNameAllowedForTarget));
+
+    _createCollection(_opCtx.get(), _sourceNss);
+    const std::string indexName(longestIndexNameAllowedForTarget, 'a');
+    _createIndex(_opCtx.get(), _sourceNss, indexName);
+    ASSERT_EQUALS(ErrorCodes::InvalidLength,
+                  renameCollection(_opCtx.get(), _sourceNss, _targetNssDifferentDb, {}));
+}
+
 TEST_F(RenameCollectionTest, RenameCollectionAcrossDatabaseWithoutUuid) {
     _createCollection(_opCtx.get(), _sourceNss);
     ASSERT_OK(renameCollection(_opCtx.get(), _sourceNss, _targetNssDifferentDb, {}));