SERVER-65797 Remove invalid index specs in memory before parsing for `listIndexes`

(cherry picked from commit 142cdf278feee6ce0c4bb1b121fd13d69c61de34)

6 files changed, 186 insertions, 45 deletions

diff --git a/jstests/multiVersion/targetedTestsLastLtsFeatures/invalid_index_options.js b/jstests/multiVersion/targetedTestsLastLtsFeatures/invalid_index_options.js
new file mode 100644
index 00000000000..249dd015450
--- /dev/null
+++ b/jstests/multiVersion/targetedTestsLastLtsFeatures/invalid_index_options.js
@@ -0,0 +1,56 @@
+/**
+ * Tests that in 5.3 version listIndexes can parse invalid index specs created before 5.0 version.
+ *
+ * @tags: [requires_replication]
+ */
+(function() {
+"use strict";
+
+load('jstests/multiVersion/libs/multi_rs.js');
+
+var nodes = {
+    n1: {binVersion: "4.4"},
+    n2: {binVersion: "4.4"},
+};
+
+var rst = new ReplSetTest({nodes: nodes});
+rst.startSet();
+rst.initiate();
+
+const dbName = "test";
+const collName = jsTestName();
+
+let primaryDB = rst.getPrimary().getDB(dbName);
+let primaryColl = primaryDB.getCollection(collName);
+
+// In earlier versions, users were able to add invalid index options when creating an index. The
+// option could still be interpreted accordingly.
+assert.commandWorked(primaryColl.createIndex({x: 1}, {sparse: "yes"}));
+
+// Upgrades from 4.4 to 5.0.
+jsTestLog("Upgrading to version 5.0");
+rst.upgradeSet({binVersion: "last-lts"});
+assert.commandWorked(rst.getPrimary().adminCommand({setFeatureCompatibilityVersion: lastLTSFCV}));
+
+// Upgrades from 5.0 to 5.3.
+jsTestLog("Upgrading to version latest");
+rst.upgradeSet({binVersion: "latest"});
+const primary = rst.getPrimary();
+assert.commandWorked(primary.adminCommand({setFeatureCompatibilityVersion: latestFCV}));
+
+primaryDB = primary.getDB(dbName);
+
+// Verify listIndexes command can correctly output the repaired index specs.
+assert.commandWorked(primaryDB.runCommand({listIndexes: collName}));
+
+// Add a new node to make sure the initial sync works correctly with the invalid index specs.
+jsTestLog("Bringing up a new node");
+rst.add();
+rst.reInitiate();
+
+jsTestLog("Waiting for new node to be synced.");
+rst.awaitReplication();
+rst.awaitSecondaryNodes();
+
+rst.stopSet();
+})();
\ No newline at end of file
diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp
index 0e9c4a28239..7f227717ec8 100644
--- a/src/mongo/db/catalog/collection_impl.cpp
+++ b/src/mongo/db/catalog/collection_impl.cpp
@@ -2004,7 +2004,8 @@ std::vector<std::string> CollectionImpl::removeInvalidIndexOptions(OperationCont
             }
 
             indexesWithInvalidOptions.push_back(std::string(index.nameStringData()));
-            index.spec = index_key_validate::removeUnknownFields(oldSpec);
+            index.spec = index_key_validate::removeUnknownFields(
+                NamespaceString(md.tenantNs.getNss()), oldSpec);
         }
     });
 
diff --git a/src/mongo/db/catalog/index_key_validate.cpp b/src/mongo/db/catalog/index_key_validate.cpp
index 3a663298d5d..d01cdf3e3ad 100644
--- a/src/mongo/db/catalog/index_key_validate.cpp
+++ b/src/mongo/db/catalog/index_key_validate.cpp
@@ -68,36 +68,6 @@ namespace {
 // specification.
 MONGO_FAIL_POINT_DEFINE(skipIndexCreateFieldNameValidation);
 
-static std::set<StringData> allowedFieldNames = {
-    IndexDescriptor::k2dIndexBitsFieldName,
-    IndexDescriptor::k2dIndexMaxFieldName,
-    IndexDescriptor::k2dIndexMinFieldName,
-    IndexDescriptor::k2dsphereCoarsestIndexedLevel,
-    IndexDescriptor::k2dsphereFinestIndexedLevel,
-    IndexDescriptor::k2dsphereVersionFieldName,
-    IndexDescriptor::kBackgroundFieldName,
-    IndexDescriptor::kCollationFieldName,
-    IndexDescriptor::kDefaultLanguageFieldName,
-    IndexDescriptor::kDropDuplicatesFieldName,
-    IndexDescriptor::kExpireAfterSecondsFieldName,
-    IndexDescriptor::kHiddenFieldName,
-    IndexDescriptor::kIndexNameFieldName,
-    IndexDescriptor::kIndexVersionFieldName,
-    IndexDescriptor::kKeyPatternFieldName,
-    IndexDescriptor::kLanguageOverrideFieldName,
-    IndexDescriptor::kNamespaceFieldName,
-    IndexDescriptor::kPartialFilterExprFieldName,
-    IndexDescriptor::kPathProjectionFieldName,
-    IndexDescriptor::kSparseFieldName,
-    IndexDescriptor::kStorageEngineFieldName,
-    IndexDescriptor::kTextVersionFieldName,
-    IndexDescriptor::kUniqueFieldName,
-    IndexDescriptor::kWeightsFieldName,
-    IndexDescriptor::kOriginalSpecFieldName,
-    IndexDescriptor::kDisallowNewDuplicateKeysFieldName,
-    // Index creation under legacy writeMode can result in an index spec with an _id field.
-    "_id"};
-
 static const std::set<StringData> allowedIdIndexFieldNames = {
     IndexDescriptor::kCollationFieldName,
     IndexDescriptor::kIndexNameFieldName,
@@ -130,6 +100,27 @@ Status isIndexVersionAllowedForCreation(IndexVersion indexVersion, const BSONObj
             str::stream() << "Invalid index specification " << indexSpec
                           << "; cannot create an index with v=" << static_cast<int>(indexVersion)};
 }
+
+BSONObj buildRepairedIndexSpec(
+    const NamespaceString& ns,
+    const BSONObj& indexSpec,
+    const std::set<StringData>& allowedFieldNames,
+    std::function<void(const BSONElement&, BSONObjBuilder*)> indexSpecHandleFn) {
+    BSONObjBuilder builder;
+    for (const auto& indexSpecElem : indexSpec) {
+        StringData fieldName = indexSpecElem.fieldNameStringData();
+        if (allowedFieldNames.count(fieldName)) {
+            indexSpecHandleFn(indexSpecElem, &builder);
+        } else {
+            LOGV2_WARNING(23878,
+                          "Removing unknown field from index spec",
+                          "namespace"_attr = redact(ns.toString()),
+                          "fieldName"_attr = redact(fieldName),
+                          "indexSpec"_attr = redact(indexSpec));
+        }
+    }
+    return builder.obj();
+}
 }  // namespace
 
 Status validateKeyPattern(const BSONObj& key, IndexDescriptor::IndexVersion indexVersion) {
@@ -263,21 +254,37 @@ Status validateKeyPattern(const BSONObj& key, IndexDescriptor::IndexVersion inde
     return Status::OK();
 }
 
-BSONObj removeUnknownFields(const BSONObj& indexSpec) {
-    BSONObjBuilder builder;
-    for (const auto& indexSpecElem : indexSpec) {
+BSONObj removeUnknownFields(const NamespaceString& ns, const BSONObj& indexSpec) {
+    auto appendIndexSpecFn = [](const BSONElement& indexSpecElem, BSONObjBuilder* builder) {
+        builder->append(indexSpecElem);
+    };
+    return buildRepairedIndexSpec(ns, indexSpec, allowedFieldNames, appendIndexSpecFn);
+}
+
+BSONObj repairIndexSpec(const NamespaceString& ns,
+                        const BSONObj& indexSpec,
+                        const std::set<StringData>& allowedFieldNames) {
+    auto fixBoolIndexSpecFn = [&indexSpec, &ns](const BSONElement& indexSpecElem,
+                                                BSONObjBuilder* builder) {
         StringData fieldName = indexSpecElem.fieldNameStringData();
-        if (allowedFieldNames.count(fieldName)) {
-            builder.append(indexSpecElem);
-        } else {
-            LOGV2_WARNING(23878,
-                          "Removing field '{fieldName}' from index spec: {indexSpec}",
-                          "Removing unknown field from index spec",
+        if ((IndexDescriptor::kBackgroundFieldName == fieldName ||
+             IndexDescriptor::kUniqueFieldName == fieldName ||
+             IndexDescriptor::kSparseFieldName == fieldName ||
+             IndexDescriptor::kDropDuplicatesFieldName == fieldName ||
+             IndexDescriptor::kDisallowNewDuplicateKeysFieldName == fieldName ||
+             "clustered" == fieldName) &&
+            !indexSpecElem.isNumber() && !indexSpecElem.isBoolean() && indexSpecElem.trueValue()) {
+            LOGV2_WARNING(6444400,
+                          "Fixing boolean field from index spec",
+                          "namespace"_attr = redact(ns.toString()),
                           "fieldName"_attr = redact(fieldName),
                           "indexSpec"_attr = redact(indexSpec));
+            builder->appendBool(fieldName, true);
+        } else {
+            builder->append(indexSpecElem);
         }
-    }
-    return builder.obj();
+    };
+    return buildRepairedIndexSpec(ns, indexSpec, allowedFieldNames, fixBoolIndexSpecFn);
 }
 
 StatusWith<BSONObj> validateIndexSpec(OperationContext* opCtx, const BSONObj& indexSpec) {
diff --git a/src/mongo/db/catalog/index_key_validate.h b/src/mongo/db/catalog/index_key_validate.h
index 50ab26129b3..ea90b4fbac2 100644
--- a/src/mongo/db/catalog/index_key_validate.h
+++ b/src/mongo/db/catalog/index_key_validate.h
@@ -43,6 +43,36 @@ class StatusWith;
 
 namespace index_key_validate {
 
+static std::set<StringData> allowedFieldNames = {
+    IndexDescriptor::k2dIndexBitsFieldName,
+    IndexDescriptor::k2dIndexMaxFieldName,
+    IndexDescriptor::k2dIndexMinFieldName,
+    IndexDescriptor::k2dsphereCoarsestIndexedLevel,
+    IndexDescriptor::k2dsphereFinestIndexedLevel,
+    IndexDescriptor::k2dsphereVersionFieldName,
+    IndexDescriptor::kBackgroundFieldName,
+    IndexDescriptor::kCollationFieldName,
+    IndexDescriptor::kDefaultLanguageFieldName,
+    IndexDescriptor::kDropDuplicatesFieldName,
+    IndexDescriptor::kExpireAfterSecondsFieldName,
+    IndexDescriptor::kHiddenFieldName,
+    IndexDescriptor::kIndexNameFieldName,
+    IndexDescriptor::kIndexVersionFieldName,
+    IndexDescriptor::kKeyPatternFieldName,
+    IndexDescriptor::kLanguageOverrideFieldName,
+    IndexDescriptor::kNamespaceFieldName,
+    IndexDescriptor::kPartialFilterExprFieldName,
+    IndexDescriptor::kPathProjectionFieldName,
+    IndexDescriptor::kSparseFieldName,
+    IndexDescriptor::kStorageEngineFieldName,
+    IndexDescriptor::kTextVersionFieldName,
+    IndexDescriptor::kUniqueFieldName,
+    IndexDescriptor::kWeightsFieldName,
+    IndexDescriptor::kOriginalSpecFieldName,
+    IndexDescriptor::kDisallowNewDuplicateKeysFieldName,
+    // Index creation under legacy writeMode can result in an index spec with an _id field.
+    "_id"};
+
 /**
  * Checks if the key is valid for building an index according to the validation rules for the given
  * index version.
@@ -59,7 +89,15 @@ StatusWith<BSONObj> validateIndexSpec(OperationContext* opCtx, const BSONObj& in
 /**
  * Returns a new index spec with any unknown field names removed from 'indexSpec'.
  */
-BSONObj removeUnknownFields(const BSONObj& indexSpec);
+BSONObj removeUnknownFields(const NamespaceString& ns, const BSONObj& indexSpec);
+
+/**
+ * Returns a new index spec with boolean values in correct types and unkown field names removed.
+ */
+BSONObj repairIndexSpec(
+    const NamespaceString& ns,
+    const BSONObj& indexSpec,
+    const std::set<StringData>& allowedFieldNames = index_key_validate::allowedFieldNames);
 
 /**
  * Performs additional validation for _id index specifications. This should be called after
diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp
index 8496c2fe0ee..e2dafe6c2e0 100644
--- a/src/mongo/db/commands/create_indexes.cpp
+++ b/src/mongo/db/commands/create_indexes.cpp
@@ -111,7 +111,7 @@ std::vector<BSONObj> parseAndValidateIndexSpecs(OperationContext* opCtx,
     for (const auto& index : cmd.getIndexes()) {
         BSONObj parsedIndexSpec = index;
         if (ignoreUnknownIndexOptions) {
-            parsedIndexSpec = index_key_validate::removeUnknownFields(parsedIndexSpec);
+            parsedIndexSpec = index_key_validate::removeUnknownFields(ns, parsedIndexSpec);
         }
 
         auto indexSpecStatus = index_key_validate::validateIndexSpec(opCtx, parsedIndexSpec);
diff --git a/src/mongo/db/commands/list_indexes.cpp b/src/mongo/db/commands/list_indexes.cpp
index 4198a8c8886..9271aa3224d 100644
--- a/src/mongo/db/commands/list_indexes.cpp
+++ b/src/mongo/db/commands/list_indexes.cpp
@@ -36,6 +36,7 @@
 
 #include "mongo/bson/bsonobjbuilder.h"
 #include "mongo/db/auth/authorization_session.h"
+#include "mongo/db/catalog/index_key_validate.h"
 #include "mongo/db/catalog/list_indexes.h"
 #include "mongo/db/clientcursor.h"
 #include "mongo/db/commands.h"
@@ -63,6 +64,40 @@
 namespace mongo {
 namespace {
 
+// The allowed fields have to be in sync with those defined in 'src/mongo/db/list_indexes.idl'.
+static std::set<StringData> allowedFieldNames = {
+    ListIndexesReplyItem::k2dsphereIndexVersionFieldName,
+    ListIndexesReplyItem::kBackgroundFieldName,
+    ListIndexesReplyItem::kBitsFieldName,
+    ListIndexesReplyItem::kBucketSizeFieldName,
+    ListIndexesReplyItem::kBuildUUIDFieldName,
+    ListIndexesReplyItem::kClusteredFieldName,
+    ListIndexesReplyItem::kCoarsestIndexedLevelFieldName,
+    ListIndexesReplyItem::kCollationFieldName,
+    ListIndexesReplyItem::kDefault_languageFieldName,
+    ListIndexesReplyItem::kDropDupsFieldName,
+    ListIndexesReplyItem::kExpireAfterSecondsFieldName,
+    ListIndexesReplyItem::kFinestIndexedLevelFieldName,
+    ListIndexesReplyItem::kHiddenFieldName,
+    ListIndexesReplyItem::kIndexBuildInfoFieldName,
+    ListIndexesReplyItem::kKeyFieldName,
+    ListIndexesReplyItem::kLanguage_overrideFieldName,
+    ListIndexesReplyItem::kMaxFieldName,
+    ListIndexesReplyItem::kMinFieldName,
+    ListIndexesReplyItem::kNameFieldName,
+    ListIndexesReplyItem::kNsFieldName,
+    ListIndexesReplyItem::kOriginalSpecFieldName,
+    ListIndexesReplyItem::kPartialFilterExpressionFieldName,
+    ListIndexesReplyItem::kDisallowNewDuplicateKeysFieldName,
+    ListIndexesReplyItem::kSparseFieldName,
+    ListIndexesReplyItem::kSpecFieldName,
+    ListIndexesReplyItem::kStorageEngineFieldName,
+    ListIndexesReplyItem::kTextIndexVersionFieldName,
+    ListIndexesReplyItem::kUniqueFieldName,
+    ListIndexesReplyItem::kVFieldName,
+    ListIndexesReplyItem::kWeightsFieldName,
+    ListIndexesReplyItem::kWildcardProjectionFieldName};
+
 /**
  * Returns index specs, with resolved namespace, from the catalog for this listIndexes request.
  */
@@ -279,6 +314,7 @@ public:
                     break;
                 }
                 invariant(state == PlanExecutor::ADVANCED);
+                nextDoc = index_key_validate::repairIndexSpec(nss, nextDoc, allowedFieldNames);
 
                 // If we can't fit this result inside the current batch, then we stash it for
                 // later.
@@ -296,7 +332,10 @@ public:
                                 "entry"_attr = nextDoc,
                                 "error"_attr = exc);
                     uasserted(5254501,
-                              "Could not parse catalog entry while replying to listIndexes");
+                              fmt::format("Could not parse catalog entry while replying to "
+                                          "listIndexes. Entry: '{}'. Error: '{}'.",
+                                          nextDoc.toString(),
+                                          exc.toString()));
                 }
                 bytesBuffered += nextDoc.objsize();
             }