SERVER-41263 Disallow empty string in index key type

(cherry picked from commit d5d674930f1c67f6c87d73715f82f608668d72fa)

SERVER-41263 Add update and repl tests for index keys with empty strings

(cherry picked from commit e09c045111610a0820cea3265d1986be5dacb541)

5 files changed, 131 insertions, 0 deletions

diff --git a/jstests/core/create_indexes.js b/jstests/core/create_indexes.js
index 3e4bd209e5b..fe3a7d885f2 100644
--- a/jstests/core/create_indexes.js
+++ b/jstests/core/create_indexes.js
@@ -165,6 +165,10 @@ assert.commandFailedWithCode(res, ErrorCodes.InvalidIndexSpecificationOption);
 res = t.runCommand('createIndexes', {indexes: [{key: {star: 1}, name: '*'}]});
 assert.commandFailedWithCode(res, ErrorCodes.BadValue);
 
+// Test that index creation fails with an index value of empty string.
+res = t.runCommand('createIndexes', {indexes: [{key: {f: ""}, name: 'f_1'}]});
+assert.commandFailedWithCode(res, ErrorCodes.CannotCreateIndex);
+
 // Test that user is not allowed to create indexes in config.transactions.
 var configDB = db.getSiblingDB('config');
 res =
diff --git a/jstests/multiVersion/index_value_empty_string_repl.js b/jstests/multiVersion/index_value_empty_string_repl.js
new file mode 100644
index 00000000000..1a638d92086
--- /dev/null
+++ b/jstests/multiVersion/index_value_empty_string_repl.js
@@ -0,0 +1,35 @@
+/*
+ * Test that a repl set with a 4.0 version primary and latest secondary will allow replication of
+ * index key values of empty strings.
+ */
+
+(function() {
+"use strict";
+load('./jstests/multiVersion/libs/multi_rs.js');
+
+const newVersion = "latest";
+const oldVersion = "last-stable";
+
+const name = "index_value_empty_string_repl";
+let nodes = {
+    n1: {binVersion: oldVersion},
+    n2: {binVersion: newVersion, rsConfig: {priority: 0}},
+};
+
+const rst = new ReplSetTest({name: name, nodes: nodes, waitForKeys: true});
+rst.startSet();
+rst.initiate();
+
+const primary = rst.getPrimary();
+const testDB = primary.getDB('test');
+
+assert.commandWorked(testDB.testColl.createIndex({x: ""}));
+rst.awaitReplication();
+
+rst.add({binVersion: newVersion, rsConfig: {priority: 0}});
+rst.reInitiate();
+
+rst.awaitSecondaryNodes();
+rst.awaitReplication();
+rst.stopSet();
+})();
diff --git a/jstests/multiVersion/index_value_empty_string_upgrade.js b/jstests/multiVersion/index_value_empty_string_upgrade.js
new file mode 100644
index 00000000000..aa9ebf3b4d6
--- /dev/null
+++ b/jstests/multiVersion/index_value_empty_string_upgrade.js
@@ -0,0 +1,58 @@
+/**
+ * Test that index keys with empty string values are allowed on 4.0 and that upgrading with
+ * such indexes will succeed.
+ */
+
+(function() {
+'use strict';
+
+load('jstests/libs/get_index_helpers.js');
+
+const dbpath = MongoRunner.dataPath + 'empty_string_index_value';
+resetDbpath(dbpath);
+
+const oldVersion = '4.0';
+const newVersion = 'latest';
+
+// We set noCleanData to true in order to preserve the data files across mongod restart.
+const mongodOptions = {
+    dbpath: dbpath,
+    noCleanData: true,
+    binVersion: oldVersion
+};
+
+// Start up an old binary version mongod.
+let conn = MongoRunner.runMongod(mongodOptions);
+
+assert.neq(null, conn, `mongod was unable able to start with version ${oldVersion}`);
+
+// Set up a collection on a 4.0 binary version node with one document and an index with
+// an empty string as index value, and then shut it down.
+let testDB = conn.getDB('test');
+assert.commandWorked(testDB.createCollection('testColl'));
+assert.commandWorked(testDB.testColl.insert({a: 1}));
+assert.commandWorked(testDB.testColl.createIndex({a: ""}));
+MongoRunner.stopMongod(conn);
+
+// Restart the mongod with the latest binary version and the 4.0 version data files.
+mongodOptions.binVersion = newVersion;
+conn = MongoRunner.runMongod(mongodOptions);
+assert.neq(null, conn);
+
+// Confirm that mongod startup does not fail due to the index specification
+// containing an empty string.
+testDB = conn.getDB('test');
+testDB.testColl.find();
+assert.eq(1,
+          testDB.testColl.count({}, {hint: {a: ""}}),
+          `data from ${oldVersion} should be available; options: ` + tojson(mongodOptions));
+
+assert.neq(null,
+           GetIndexHelpers.findByKeyPattern(testDB.testColl.getIndexes(), {a: ""}),
+           `index from ${oldVersion} should be available; options: ` + tojson(mongodOptions));
+
+// Verify that indexes with empty string values cannot be created
+assert.commandFailedWithCode(testDB.testColl.createIndex({x: ""}), ErrorCodes.CannotCreateIndex);
+
+MongoRunner.stopMongod(conn);
+})();
diff --git a/src/mongo/db/catalog/index_key_validate.cpp b/src/mongo/db/catalog/index_key_validate.cpp
index 56d521edbc4..2bc450516fb 100644
--- a/src/mongo/db/catalog/index_key_validate.cpp
+++ b/src/mongo/db/catalog/index_key_validate.cpp
@@ -300,6 +300,14 @@ StatusWith<BSONObj> validateIndexSpec(
                 return keyPatternValidateStatus;
             }
 
+            for (const auto& keyElement : indexSpecElem.Obj()) {
+                if (keyElement.type() == String && keyElement.str().empty()) {
+                    return {ErrorCodes::CannotCreateIndex,
+                            str::stream()
+                                << "Values in the index key pattern cannot be empty strings"};
+                }
+            }
+
             if ((featureCompatibility.getVersion() <
                  ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) &&
                 (IndexNames::findPluginName(indexSpec.getObjectField(
diff --git a/src/mongo/dbtests/commandtests.cpp b/src/mongo/dbtests/commandtests.cpp
index a97da36e73c..f8822b0a315 100644
--- a/src/mongo/dbtests/commandtests.cpp
+++ b/src/mongo/dbtests/commandtests.cpp
@@ -242,6 +242,31 @@ public:
     }
 };
 
+
+class CreateIndexWithEmptyStringAsValue : Base {
+public:
+    void run() {
+        ASSERT(db.createCollection(nss().ns()));
+
+        BSONObjBuilder indexSpec;
+        indexSpec.append("key",
+                         BSON("a"
+                              << ""));
+
+        BSONArrayBuilder indexes;
+        indexes.append(indexSpec.obj());
+
+        BSONObjBuilder cmd;
+        cmd.append("createIndexes", nsColl());
+        cmd.append("indexes", indexes.arr());
+
+        BSONObj result;
+        bool ok = db.runCommand(nsDb(), cmd.obj(), result);
+        log() << result.jsonString();
+        ASSERT(!ok);
+    }
+};
+
 class FindAndModify : Base {
 public:
     void run() {
@@ -370,6 +395,7 @@ public:
         add<SymbolArgument::GeoSearch>();
         add<SymbolArgument::CreateIndexWithNoKey>();
         add<SymbolArgument::CreateIndexWithDuplicateKey>();
+        add<SymbolArgument::CreateIndexWithEmptyStringAsValue>();
         add<RolesInfoShouldNotReturnDuplicateFieldNames>();
     }
 };