SERVER-20374 replica set configuration field electionTimeoutOffsetLimitMillis must be positive

3 files changed, 48 insertions, 28 deletions

diff --git a/src/mongo/db/repl/replica_set_config.cpp b/src/mongo/db/repl/replica_set_config.cpp
index 2f33d82c689..f076fb5a2f1 100644
--- a/src/mongo/db/repl/replica_set_config.cpp
+++ b/src/mongo/db/repl/replica_set_config.cpp
@@ -53,6 +53,7 @@ const Milliseconds ReplicaSetConfig::kDefaultHeartbeatInterval(2000);
 const Seconds ReplicaSetConfig::kDefaultHeartbeatTimeoutPeriod(10);
 const Milliseconds ReplicaSetConfig::kDefaultElectionTimeoutPeriod(10000);
 const int ReplicaSetConfig::kDefaultElectionTimeoutOffsetLimit(2000);
+const bool ReplicaSetConfig::kDefaultChainingAllowed(true);
 
 namespace {
 
@@ -203,18 +204,19 @@ Status ReplicaSetConfig::_parseSettingsSubdocument(const BSONObj& settings) {
     //
     // Parse electionTimeoutOffsetLimit
     //
-    BSONElement electionTimeoutOffsetLimitElement = settings[kElectionTimeoutOffsetLimitFieldName];
-    if (electionTimeoutOffsetLimitElement.eoo()) {
-        _electionTimeoutOffsetLimit = kDefaultElectionTimeoutOffsetLimit;
-    } else if (electionTimeoutOffsetLimitElement.isNumber()) {
-        _electionTimeoutOffsetLimit = electionTimeoutOffsetLimitElement.numberInt();
-    } else {
-        return Status(ErrorCodes::TypeMismatch,
-                      str::stream() << "Expected type of " << kSettingsFieldName << "."
-                                    << kElectionTimeoutOffsetLimitFieldName
-                                    << " to be a number, but found a value of type "
-                                    << typeName(electionTimeoutOffsetLimitElement.type()));
-    }
+    auto greaterThanZero = stdx::bind(std::greater<long long>(), stdx::placeholders::_1, 0);
+    long long electionTimeoutOffsetLimitMillis;
+    auto electionTimeoutOffsetLimitStatus =
+        bsonExtractIntegerFieldWithDefaultIf(settings,
+                                             kElectionTimeoutOffsetLimitFieldName,
+                                             kDefaultElectionTimeoutOffsetLimit,
+                                             greaterThanZero,
+                                             "election timeout offset limit must be greater than 0",
+                                             &electionTimeoutOffsetLimitMillis);
+    if (!electionTimeoutOffsetLimitStatus.isOK()) {
+        return electionTimeoutOffsetLimitStatus;
+    }
+    _electionTimeoutOffsetLimit = electionTimeoutOffsetLimitMillis;
 
     //
     // Parse heartbeatTimeoutSecs
@@ -236,7 +238,7 @@ Status ReplicaSetConfig::_parseSettingsSubdocument(const BSONObj& settings) {
     // Parse chainingAllowed
     //
     Status status = bsonExtractBooleanFieldWithDefault(
-        settings, kChainingAllowedFieldName, true, &_chainingAllowed);
+        settings, kChainingAllowedFieldName, kDefaultChainingAllowed, &_chainingAllowed);
     if (!status.isOK())
         return status;
 
@@ -343,13 +345,6 @@ Status ReplicaSetConfig::validate() const {
                                        "but found "
                                     << durationCount<Seconds>(_heartbeatTimeoutPeriod));
     }
-    if (_electionTimeoutOffsetLimit < 0) {
-        return Status(ErrorCodes::BadValue,
-                      str::stream() << kSettingsFieldName << '.'
-                                    << kElectionTimeoutOffsetLimitFieldName
-                                    << " field value must be non-negative, "
-                                       "but found " << _electionTimeoutOffsetLimit);
-    }
     if (_electionTimeoutPeriod < Milliseconds(0)) {
         return Status(ErrorCodes::BadValue,
                       str::stream() << kSettingsFieldName << '.' << kElectionTimeoutFieldName
diff --git a/src/mongo/db/repl/replica_set_config.h b/src/mongo/db/repl/replica_set_config.h
index 5de6ed0b0d5..a702fd0dc5c 100644
--- a/src/mongo/db/repl/replica_set_config.h
+++ b/src/mongo/db/repl/replica_set_config.h
@@ -63,6 +63,7 @@ public:
     static const Milliseconds kDefaultHeartbeatInterval;
     static const Seconds kDefaultHeartbeatTimeoutPeriod;
     static const int kDefaultElectionTimeoutOffsetLimit;
+    static const bool kDefaultChainingAllowed;
 
     /**
      * Initializes this ReplicaSetConfig from the contents of "cfg".
@@ -314,18 +315,18 @@ private:
     void _addInternalWriteConcernModes();
 
     bool _isInitialized = false;
-    long long _version;
+    long long _version = 1;
     std::string _replSetName;
     std::vector<MemberConfig> _members;
     WriteConcernOptions _defaultWriteConcern;
-    Milliseconds _electionTimeoutPeriod = Milliseconds(2000);
+    Milliseconds _electionTimeoutPeriod = kDefaultElectionTimeoutPeriod;
     Milliseconds _heartbeatInterval = kDefaultHeartbeatInterval;
     Seconds _heartbeatTimeoutPeriod = Seconds(0);
-    bool _chainingAllowed;
-    int _electionTimeoutOffsetLimit;
-    int _majorityVoteCount;
-    int _writeMajority;
-    int _totalVotingMembers;
+    bool _chainingAllowed = kDefaultChainingAllowed;
+    int _electionTimeoutOffsetLimit = kDefaultElectionTimeoutOffsetLimit;
+    int _majorityVoteCount = 0;
+    int _writeMajority = 0;
+    int _totalVotingMembers = 0;
     ReplicaSetTagConfig _tagConfig;
     StringMap<ReplicaSetTagPattern> _customWriteConcernModes;
     long long _protocolVersion = 0;
diff --git a/src/mongo/db/repl/replica_set_config_test.cpp b/src/mongo/db/repl/replica_set_config_test.cpp
index 9b7ad2a26e8..698bb4ee6ca 100644
--- a/src/mongo/db/repl/replica_set_config_test.cpp
+++ b/src/mongo/db/repl/replica_set_config_test.cpp
@@ -498,6 +498,30 @@ TEST(ReplicaSetConfig, ParseFailsWithNonNumericElectionTimeoutOffsetLimitMillisF
     ASSERT_EQUALS(ErrorCodes::TypeMismatch, status);
 }
 
+TEST(ReplicaSetConfig, ParseFailsWithZeroElectionTimeoutOffsetLimitMillisField) {
+    ReplicaSetConfig config;
+    Status status =
+        config.initialize(BSON("_id"
+                               << "rs0"
+                               << "version" << 1 << "members"
+                               << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                                        << "localhost:12345")) << "settings"
+                               << BSON("electionTimeoutOffsetLimitMillis" << 0)));
+    ASSERT_EQUALS(ErrorCodes::BadValue, status);
+}
+
+TEST(ReplicaSetConfig, ParseFailsWithNegativeElectionTimeoutOffsetLimitMillisField) {
+    ReplicaSetConfig config;
+    Status status =
+        config.initialize(BSON("_id"
+                               << "rs0"
+                               << "version" << 1 << "members"
+                               << BSON_ARRAY(BSON("_id" << 0 << "host"
+                                                        << "localhost:12345")) << "settings"
+                               << BSON("electionTimeoutOffsetLimitMillis" << -1000)));
+    ASSERT_EQUALS(ErrorCodes::BadValue, status);
+}
+
 TEST(ReplicaSetConfig, ParseFailsWithNonNumericHeartbeatTimeoutSecsField) {
     ReplicaSetConfig config;
     Status status = config.initialize(BSON("_id"
@@ -978,7 +1002,7 @@ TEST(ReplicaSetConfig, toBSONRoundTripAbilityInvalid) {
                                          << "votes" << 0 << "priority" << 0)) << "settings"
              << BSON("heartbeatIntervalMillis" << -5000 << "heartbeatTimeoutSecs" << -20
                                                << "electionTimeoutMillis" << -2
-                                               << "electionTimeoutOffsetLimitMillis" << -2))));
+                                               << "electionTimeoutOffsetLimitMillis" << 2))));
     ASSERT_OK(configB.initialize(configA.toBSON()));
     ASSERT_NOT_OK(configA.validate());
     ASSERT_NOT_OK(configB.validate());