SERVER-94487 collMod refreshes catalog with index entry from older catalog state (#26764)r8.0.0-rc20

GitOrigin-RevId: 09707dd9a1fcd2429d10539a68ae30ce003aa712

3 files changed, 94 insertions, 5 deletions

diff --git a/jstests/noPassthrough/ttl_invalid_index_fixer_uses_writable_collection.js b/jstests/noPassthrough/ttl_invalid_index_fixer_uses_writable_collection.js
new file mode 100644
index 00000000000..fbd415d7ea6
--- /dev/null
+++ b/jstests/noPassthrough/ttl_invalid_index_fixer_uses_writable_collection.js
@@ -0,0 +1,86 @@
+/**
+ * Reproduces a seg-fault caused by the InvalidTTLIndexFixer trying to 'fix' an invalid TTL index
+ * spec with both a non-int 'expireAfterSeconds' field and an unexpected additional field.
+ * Specifically, the behavior is caused by the additional field being an accepted field for text
+ * indexes, 'weights', but not indexes of other types.
+ *
+ * The original issue occured when trying to upgrade from an older version, so the first steps are
+ * to bypass validation checks and introduce an invalid ttl index spec.
+ *
+ * @tags: [
+ *     requires_replication,
+ * ]
+ */
+import {configureFailPoint} from "jstests/libs/fail_point_util.js";
+
+const rst = new ReplSetTest({
+    nodes: [{}, {rsConfig: {votes: 0, priority: 0}}],
+    nodeOptions: {setParameter: {ttlMonitorSleepSecs: 1}},
+});
+rst.startSet();
+rst.initiate();
+
+let primary = rst.getPrimary();
+let secondary = rst.getSecondary();
+const db = primary.getDB('test');
+const expireAfterSecondsNonInt = 100.3;
+let coll = db.t;
+assert.commandWorked(coll.insert({_id: 0, t: ISODate()}));
+
+// Failpoints to circumvent validation when introducing the invalid index. It's possible we don't
+// need all of them. 'skipIndexCreateWeightsFieldValidation' is the most important.
+const fpNames = [
+    'skipTTLIndexValidationOnCreateIndex',
+    'skipTTLIndexExpireAfterSecondsValidation',
+    'skipIndexCreateFieldNameValidation',
+    'skipIndexCreateWeightsFieldValidation',
+];
+const fps = [];
+for (const fpName of fpNames) {
+    fps.push(configureFailPoint(primary, fpName));
+    fps.push(configureFailPoint(secondary, fpName));
+}
+try {
+    assert.commandWorked(
+        coll.createIndex({t: 1}, {expireAfterSeconds: expireAfterSecondsNonInt, weights: {}}));
+    const catalogContents = coll.aggregate([{$listCatalog: {}}]).toArray();
+    jsTestLog("Catalog contents: " + tojson(catalogContents));
+
+    rst.awaitReplication();
+    rst.awaitLastStableRecoveryTimestamp();
+} finally {
+    for (const fp of fps) {
+        fp.off();
+    }
+}
+
+// When the primary steps back up, we eventually trigger the seg-fault.
+jsTest.log(
+    `Forcing the primary to step down then step up again to trigger 'InvalidTTLIndexFixer' thread`);
+assert.commandWorked(primary.adminCommand({replSetStepDown: 5, force: true}));
+primary = rst.waitForPrimary();
+
+coll = primary.getDB(db.getName()).getCollection(coll.getName());
+jsTestLog("Catalog contents on primary: " + tojson(coll.aggregate([{$listCatalog: {}}]).toArray()));
+
+// Ensure the 'InvalidTTLIndexFixer' thread has time to run.
+assert.soon(
+    () => {
+        return 1 ==
+            rst.findOplog(primary,
+                          {
+                              op: 'c',
+                              ns: coll.getDB().getCollection('$cmd').getFullName(),
+                              'o.collMod': coll.getName(),
+                              'o.index.name': 't_1',
+                              'o.index.expireAfterSeconds': Math.floor(expireAfterSecondsNonInt),
+                          },
+                          /*limit=*/ 1)
+                .toArray()
+                .length;
+    },
+    'TTL index with ' + expireAfterSecondsNonInt +
+        ' expireAfterSeconds was not fixed using collMod during step-up: ' +
+        tojson(rst.findOplog(primary, {op: {$ne: 'n'}}, /*limit=*/ 10).toArray()));
+rst.awaitReplication();
+rst.stopSet();
diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp
index 5e23537afde..713c967a0b2 100644
--- a/src/mongo/db/catalog/coll_mod.cpp
+++ b/src/mongo/db/catalog/coll_mod.cpp
@@ -1055,7 +1055,7 @@ Status _collModInternal(OperationContext* opCtx,
             writableColl->repairInvalidIndexOptions(opCtx);
         for (const auto& indexWithInvalidOptions : indexesWithInvalidOptions) {
             const IndexDescriptor* desc =
-                coll->getIndexCatalog()->findIndexByName(opCtx, indexWithInvalidOptions);
+                writableColl->getIndexCatalog()->findIndexByName(opCtx, indexWithInvalidOptions);
             invariant(desc);
 
             // Notify the index catalog that the definition of this index changed.
diff --git a/src/mongo/db/catalog/index_key_validate.cpp b/src/mongo/db/catalog/index_key_validate.cpp
index 67b907f80fd..a16116e4604 100644
--- a/src/mongo/db/catalog/index_key_validate.cpp
+++ b/src/mongo/db/catalog/index_key_validate.cpp
@@ -100,6 +100,7 @@ namespace {
 // names will be disabled. This will allow for creation of indexes with invalid field names in their
 // specification.
 MONGO_FAIL_POINT_DEFINE(skipIndexCreateFieldNameValidation);
+MONGO_FAIL_POINT_DEFINE(skipIndexCreateWeightsFieldValidation);
 
 // When the skipTTLIndexExpireAfterSecondsValidation failpoint is enabled,
 // validation for TTL index 'expireAfterSeconds' will be disabled in certain codepaths.
@@ -706,10 +707,12 @@ StatusWith<BSONObj> validateIndexSpec(OperationContext* opCtx, const BSONObj& in
     }
 
     if (indexType != IndexNames::TEXT && hasWeightsField) {
-        return {ErrorCodes::CannotCreateIndex,
-                str::stream() << "Invalid index specification " << indexSpec << "; the field '"
-                              << IndexDescriptor::kWeightsFieldName
-                              << "' can only be specified with text indexes"};
+        if (!skipIndexCreateWeightsFieldValidation.shouldFail()) {
+            return {ErrorCodes::CannotCreateIndex,
+                    str::stream() << "Invalid index specification " << indexSpec << "; the field '"
+                                  << IndexDescriptor::kWeightsFieldName
+                                  << "' can only be specified with text indexes"};
+        }
     }
 
     if (unique && prepareUnique) {