SERVER-30523: dateFromParts should not reject out-of-range numbers for date/time properties

8 files changed, 207 insertions, 86 deletions

diff --git a/src/mongo/db/pipeline/document_value_test.cpp b/src/mongo/db/pipeline/document_value_test.cpp
index 613ec1de0d1..d0c72d1d991 100644
--- a/src/mongo/db/pipeline/document_value_test.cpp
+++ b/src/mongo/db/pipeline/document_value_test.cpp
@@ -1,5 +1,3 @@
-// documenttests.cpp : Unit tests for Document, Value, and related classes.
-
 /**
  *    Copyright (C) 2012 10gen Inc.
  *
@@ -1716,6 +1714,75 @@ public:
                         Value::deserializeForSorter(reader, Value::SorterDeserializeSettings()));
     }
 };
+
+namespace {
+
+// Integer limits.
+const int kIntMax = std::numeric_limits<int>::max();
+const int kIntMin = std::numeric_limits<int>::lowest();
+const long long kIntMaxAsLongLong = kIntMax;
+const long long kIntMinAsLongLong = kIntMin;
+const double kIntMaxAsDouble = kIntMax;
+const double kIntMinAsDouble = kIntMin;
+const Decimal128 kIntMaxAsDecimal = Decimal128(kIntMax);
+const Decimal128 kIntMinAsDecimal = Decimal128(kIntMin);
+
+// 64-bit integer limits.
+const long long kLongLongMax = std::numeric_limits<long long>::max();
+const long long kLongLongMin = std::numeric_limits<long long>::lowest();
+const double kLongLongMaxAsDouble = static_cast<double>(kLongLongMax);
+const double kLongLongMinAsDouble = static_cast<double>(kLongLongMin);
+const Decimal128 kLongLongMaxAsDecimal = Decimal128(static_cast<int64_t>(kLongLongMax));
+const Decimal128 kLongLongMinAsDecimal = Decimal128(static_cast<int64_t>(kLongLongMin));
+
+// Double limits.
+const double kDoubleMax = std::numeric_limits<double>::max();
+const double kDoubleMin = std::numeric_limits<double>::lowest();
+const Decimal128 kDoubleMaxAsDecimal = Decimal128(kDoubleMin);
+const Decimal128 kDoubleMinAsDecimal = Decimal128(kDoubleMin);
+
+}  // namespace
+
+TEST(ValueIntegral, CorrectlyIdentifiesValidIntegralValues) {
+    ASSERT_TRUE(Value(kIntMax).integral());
+    ASSERT_TRUE(Value(kIntMin).integral());
+    ASSERT_TRUE(Value(kIntMaxAsLongLong).integral());
+    ASSERT_TRUE(Value(kIntMinAsLongLong).integral());
+    ASSERT_TRUE(Value(kIntMaxAsDouble).integral());
+    ASSERT_TRUE(Value(kIntMinAsDouble).integral());
+    ASSERT_TRUE(Value(kIntMaxAsDecimal).integral());
+    ASSERT_TRUE(Value(kIntMinAsDecimal).integral());
+}
+
+TEST(ValueIntegral, CorrectlyIdentifiesInvalidIntegralValues) {
+    ASSERT_FALSE(Value(kLongLongMax).integral());
+    ASSERT_FALSE(Value(kLongLongMin).integral());
+    ASSERT_FALSE(Value(kLongLongMaxAsDouble).integral());
+    ASSERT_FALSE(Value(kLongLongMinAsDouble).integral());
+    ASSERT_FALSE(Value(kLongLongMaxAsDecimal).integral());
+    ASSERT_FALSE(Value(kLongLongMinAsDecimal).integral());
+    ASSERT_FALSE(Value(kDoubleMax).integral());
+    ASSERT_FALSE(Value(kDoubleMin).integral());
+}
+
+TEST(ValueIntegral, CorrectlyIdentifiesValid64BitIntegralValues) {
+    ASSERT_TRUE(Value(kIntMax).integral64Bit());
+    ASSERT_TRUE(Value(kIntMin).integral64Bit());
+    ASSERT_TRUE(Value(kLongLongMax).integral64Bit());
+    ASSERT_TRUE(Value(kLongLongMin).integral64Bit());
+    ASSERT_TRUE(Value(kLongLongMinAsDouble).integral64Bit());
+    ASSERT_TRUE(Value(kLongLongMaxAsDecimal).integral64Bit());
+    ASSERT_TRUE(Value(kLongLongMinAsDecimal).integral64Bit());
+}
+
+TEST(ValueIntegral, CorrectlyIdentifiesInvalid64BitIntegralValues) {
+    ASSERT_FALSE(Value(kLongLongMaxAsDouble).integral64Bit());
+    ASSERT_FALSE(Value(kDoubleMax).integral64Bit());
+    ASSERT_FALSE(Value(kDoubleMin).integral64Bit());
+    ASSERT_FALSE(Value(kDoubleMaxAsDecimal).integral64Bit());
+    ASSERT_FALSE(Value(kDoubleMinAsDecimal).integral64Bit());
+}
+
 }  // namespace Value
 
 class All : public Suite {
diff --git a/src/mongo/db/pipeline/expression.cpp b/src/mongo/db/pipeline/expression.cpp
index bbfe65ebb0c..3c4b5ef3dae 100644
--- a/src/mongo/db/pipeline/expression.cpp
+++ b/src/mongo/db/pipeline/expression.cpp
@@ -1147,27 +1147,11 @@ Value ExpressionDateFromParts::serialize(bool explain) const {
                   {"timezone", _timeZone ? _timeZone->serialize(explain) : Value()}}}});
 }
 
-/**
- * This function checks whether a field is a number, and fits in the given range.
- *
- * If the field does not exist, the default value is returned trough the returnValue out parameter
- * and the function returns true.
- *
- * If the field exists:
- * - if the value is "nullish", the function returns false, so that the calling function can return
- *   a BSONNULL value.
- * - if the value can not be coerced to an integral value, an exception is returned.
- * - if the value is out of the range [minValue..maxValue], an exception is returned.
- * - otherwise, the coerced integral value is returned through the returnValue
- *   out parameter, and the function returns true.
- */
-bool ExpressionDateFromParts::evaluateNumberWithinRange(const Document& root,
-                                                        const Expression* field,
+bool ExpressionDateFromParts::evaluateNumberWithDefault(const Document& root,
+                                                        intrusive_ptr<Expression> field,
                                                         StringData fieldName,
-                                                        int defaultValue,
-                                                        int minValue,
-                                                        int maxValue,
-                                                        int* returnValue) const {
+                                                        long long defaultValue,
+                                                        long long* returnValue) const {
     if (!field) {
         *returnValue = defaultValue;
         return true;
@@ -1184,30 +1168,21 @@ bool ExpressionDateFromParts::evaluateNumberWithinRange(const Document& root,
                           << typeName(fieldValue.getType())
                           << " with value "
                           << fieldValue.toString(),
-            fieldValue.integral());
-
-    *returnValue = fieldValue.coerceToInt();
+            fieldValue.integral64Bit());
 
-    uassert(40523,
-            str::stream() << "'" << fieldName << "' must evaluate to an integer in the range "
-                          << minValue
-                          << " to "
-                          << maxValue
-                          << ", found "
-                          << *returnValue,
-            *returnValue >= minValue && *returnValue <= maxValue);
+    *returnValue = fieldValue.coerceToLong();
 
     return true;
 }
 
 Value ExpressionDateFromParts::evaluate(const Document& root) const {
-    int hour, minute, second, millisecond;
+    long long hour, minute, second, millisecond;
 
-    if (!evaluateNumberWithinRange(root, _hour.get(), "hour"_sd, 0, 0, 24, &hour) ||
-        !evaluateNumberWithinRange(root, _minute.get(), "minute"_sd, 0, 0, 59, &minute) ||
-        !evaluateNumberWithinRange(root, _second.get(), "second"_sd, 0, 0, 59, &second) ||
-        !evaluateNumberWithinRange(
-            root, _millisecond.get(), "millisecond"_sd, 0, 0, 999, &millisecond)) {
+    if (!evaluateNumberWithDefault(root, _hour, "hour"_sd, 0, &hour) ||
+        !evaluateNumberWithDefault(root, _minute, "minute"_sd, 0, &minute) ||
+        !evaluateNumberWithDefault(root, _second, "second"_sd, 0, &second) ||
+        !evaluateNumberWithDefault(root, _millisecond, "millisecond"_sd, 0, &millisecond)) {
+        // One of the evaluated inputs in nullish.
         return Value(BSONNULL);
     }
 
@@ -1218,26 +1193,33 @@ Value ExpressionDateFromParts::evaluate(const Document& root) const {
     }
 
     if (_year) {
-        int year, month, day;
+        long long year, month, day;
 
-        if (!evaluateNumberWithinRange(root, _year.get(), "year"_sd, 1970, 0, 9999, &year) ||
-            !evaluateNumberWithinRange(root, _month.get(), "month"_sd, 1, 1, 12, &month) ||
-            !evaluateNumberWithinRange(root, _day.get(), "day"_sd, 1, 1, 31, &day)) {
+        if (!evaluateNumberWithDefault(root, _year, "year"_sd, 1970, &year) ||
+            !evaluateNumberWithDefault(root, _month, "month"_sd, 1, &month) ||
+            !evaluateNumberWithDefault(root, _day, "day"_sd, 1, &day)) {
+            // One of the evaluated inputs in nullish.
             return Value(BSONNULL);
         }
 
+        uassert(40523,
+                str::stream() << "'year' must evaluate to an integer in the range " << 0 << " to "
+                              << 9999
+                              << ", found "
+                              << year,
+                year >= 0 && year <= 9999);
+
         return Value(
             timeZone->createFromDateParts(year, month, day, hour, minute, second, millisecond));
     }
 
     if (_isoWeekYear) {
-        int isoWeekYear, isoWeek, isoDayOfWeek;
+        long long isoWeekYear, isoWeek, isoDayOfWeek;
 
-        if (!evaluateNumberWithinRange(
-                root, _isoWeekYear.get(), "isoWeekYear"_sd, 1970, 0, 9999, &isoWeekYear) ||
-            !evaluateNumberWithinRange(root, _isoWeek.get(), "isoWeek"_sd, 1, 1, 53, &isoWeek) ||
-            !evaluateNumberWithinRange(
-                root, _isoDayOfWeek.get(), "isoDayOfWeek"_sd, 1, 1, 7, &isoDayOfWeek)) {
+        if (!evaluateNumberWithDefault(root, _isoWeekYear, "isoWeekYear"_sd, 1970, &isoWeekYear) ||
+            !evaluateNumberWithDefault(root, _isoWeek, "isoWeek"_sd, 1, &isoWeek) ||
+            !evaluateNumberWithDefault(root, _isoDayOfWeek, "isoDayOfWeek"_sd, 1, &isoDayOfWeek)) {
+            // One of the evaluated inputs in nullish.
             return Value(BSONNULL);
         }
 
diff --git a/src/mongo/db/pipeline/expression.h b/src/mongo/db/pipeline/expression.h
index 7d055e1a963..2e0d899ed12 100644
--- a/src/mongo/db/pipeline/expression.h
+++ b/src/mongo/db/pipeline/expression.h
@@ -910,16 +910,22 @@ private:
                             boost::intrusive_ptr<Expression> timeZone);
 
     /**
-     * Evaluates the value in field as number, and makes sure it fits in the minValue..maxValue
-     * range. If the field is missing or empty, the function returns the defaultValue.
+     * This function checks whether a field is a number.
+     *
+     * If 'field' is null, the default value is returned trough the 'returnValue' out
+     * parameter and the function returns true.
+     *
+     * If 'field' is not null:
+     * - if the value is "nullish", the function returns false.
+     * - if the value can not be coerced to an integral value, a UserException is thrown.
+     * - otherwise, the coerced integral value is returned through the 'returnValue'
+     *   out parameter, and the function returns true.
      */
-    bool evaluateNumberWithinRange(const Document& root,
-                                   const Expression* field,
+    bool evaluateNumberWithDefault(const Document& root,
+                                   boost::intrusive_ptr<Expression> field,
                                    StringData fieldName,
-                                   int defaultValue,
-                                   int minValue,
-                                   int maxValue,
-                                   int* returnValue) const;
+                                   long long defaultValue,
+                                   long long* returnValue) const;
 
     boost::intrusive_ptr<Expression> _year;
     boost::intrusive_ptr<Expression> _month;
diff --git a/src/mongo/db/pipeline/expression_test.cpp b/src/mongo/db/pipeline/expression_test.cpp
index 656a347067a..b2610e97bdb 100644
--- a/src/mongo/db/pipeline/expression_test.cpp
+++ b/src/mongo/db/pipeline/expression_test.cpp
@@ -5365,14 +5365,42 @@ TEST_F(ExpressionDateFromPartsTest, OptimizesToConstantIfAllInputsAreConstant) {
     ASSERT_FALSE(dynamic_cast<ExpressionConstant*>(dateExp->optimize().get()));
 
     // Test that it does *not* become a constant if both isoWeekYear and isoDayOfWeek are provided,
-    // but
-    // isoDayOfWeek is not a constant.
+    // but isoDayOfWeek is not a constant.
     spec = BSON("$dateFromParts" << BSON("isoWeekYear" << 2017 << "isoDayOfWeek"
                                                        << "$isoDayOfWeekday"));
     dateExp = Expression::parseExpression(expCtx, spec, expCtx->variablesParseState);
     ASSERT_FALSE(dynamic_cast<ExpressionConstant*>(dateExp->optimize().get()));
 }
 
+TEST_F(ExpressionDateFromPartsTest, TestThatOutOfRangeValuesRollOver) {
+    auto expCtx = getExpCtx();
+
+    auto spec = BSON("$dateFromParts" << BSON("year" << 2017 << "month" << -1));
+    auto dateExp = Expression::parseExpression(expCtx, spec, expCtx->variablesParseState);
+    auto dateVal = Date_t::fromMillisSinceEpoch(1477958400000);  // 11/1/2016 in ms.
+    ASSERT_VALUE_EQ(Value(dateVal), dateExp->evaluate(Document{}));
+
+    spec = BSON("$dateFromParts" << BSON("year" << 2017 << "day" << -1));
+    dateExp = Expression::parseExpression(expCtx, spec, expCtx->variablesParseState);
+    dateVal = Date_t::fromMillisSinceEpoch(1483056000000);  // 12/30/2016
+    ASSERT_VALUE_EQ(Value(dateVal), dateExp->evaluate(Document{}));
+
+    spec = BSON("$dateFromParts" << BSON("year" << 2017 << "hour" << 25));
+    dateExp = Expression::parseExpression(expCtx, spec, expCtx->variablesParseState);
+    dateVal = Date_t::fromMillisSinceEpoch(1483318800000);  // 1/2/2017 01:00:00
+    ASSERT_VALUE_EQ(Value(dateVal), dateExp->evaluate(Document{}));
+
+    spec = BSON("$dateFromParts" << BSON("year" << 2017 << "minute" << 61));
+    dateExp = Expression::parseExpression(expCtx, spec, expCtx->variablesParseState);
+    dateVal = Date_t::fromMillisSinceEpoch(1483232460000);  // 1/1/2017 01:01:00
+    ASSERT_VALUE_EQ(Value(dateVal), dateExp->evaluate(Document{}));
+
+    spec = BSON("$dateFromParts" << BSON("year" << 2017 << "second" << 61));
+    dateExp = Expression::parseExpression(expCtx, spec, expCtx->variablesParseState);
+    dateVal = Date_t::fromMillisSinceEpoch(1483228861000);  // 1/1/2017 00:01:01
+    ASSERT_VALUE_EQ(Value(dateVal), dateExp->evaluate(Document{}));
+}
+
 }  // namespace ExpressionDateFromPartsTest
 
 namespace ExpressionDateToPartsTest {
diff --git a/src/mongo/db/pipeline/value.cpp b/src/mongo/db/pipeline/value.cpp
index 480330c5a2d..67f4e4eb08e 100644
--- a/src/mongo/db/pipeline/value.cpp
+++ b/src/mongo/db/pipeline/value.cpp
@@ -45,6 +45,7 @@
 #include "mongo/platform/decimal128.h"
 #include "mongo/util/hex.h"
 #include "mongo/util/mongoutils/str.h"
+#include "mongo/util/represent_as.h"
 
 namespace mongo {
 using namespace mongoutils;
@@ -1013,14 +1014,11 @@ bool Value::integral() const {
         case NumberInt:
             return true;
         case NumberLong:
-            return (_storage.longValue <= numeric_limits<int>::max() &&
-                    _storage.longValue >= numeric_limits<int>::min());
+            return bool(representAs<int>(_storage.longValue));
         case NumberDouble:
-            return (_storage.doubleValue <= numeric_limits<int>::max() &&
-                    _storage.doubleValue >= numeric_limits<int>::min() &&
-                    _storage.doubleValue == static_cast<int>(_storage.doubleValue));
+            return bool(representAs<int>(_storage.doubleValue));
         case NumberDecimal: {
-            // If we are able to convert the decimal to an int32_t without an rounding errors,
+            // If we are able to convert the decimal to an int32_t without any rounding errors,
             // then it is integral.
             uint32_t signalingFlags = Decimal128::kNoFlag;
             (void)_storage.getDecimal().toIntExact(&signalingFlags);
@@ -1031,6 +1029,25 @@ bool Value::integral() const {
     }
 }
 
+bool Value::integral64Bit() const {
+    switch (getType()) {
+        case NumberInt:
+        case NumberLong:
+            return true;
+        case NumberDouble:
+            return bool(representAs<int64_t>(_storage.doubleValue));
+        case NumberDecimal: {
+            // If we are able to convert the decimal to an int64_t without any rounding errors,
+            // then it is a 64-bit.
+            uint32_t signalingFlags = Decimal128::kNoFlag;
+            (void)_storage.getDecimal().toLongExact(&signalingFlags);
+            return signalingFlags == Decimal128::kNoFlag;
+        }
+        default:
+            return false;
+    }
+}
+
 size_t Value::getApproximateSize() const {
     switch (getType()) {
         case Code:
diff --git a/src/mongo/db/pipeline/value.h b/src/mongo/db/pipeline/value.h
index 6cef70b3f95..9fef5e8fda6 100644
--- a/src/mongo/db/pipeline/value.h
+++ b/src/mongo/db/pipeline/value.h
@@ -168,6 +168,12 @@ public:
      */
     bool integral() const;
 
+    /**
+     * Returns true if this value is a numeric type that can be represented as a 64-bit integer,
+     * and false otherwise.
+     */
+    bool integral64Bit() const;
+
     /// Get the BSON type of the field.
     BSONType getType() const {
         return _storage.bsonType();
diff --git a/src/mongo/db/query/datetime/date_time_support.cpp b/src/mongo/db/query/datetime/date_time_support.cpp
index c41df5b28d6..c13bce509c4 100644
--- a/src/mongo/db/query/datetime/date_time_support.cpp
+++ b/src/mongo/db/query/datetime/date_time_support.cpp
@@ -47,9 +47,14 @@
 namespace mongo {
 
 namespace {
+
 const auto getTimeZoneDatabase =
     ServiceContext::declareDecoration<std::unique_ptr<TimeZoneDatabase>>();
 
+std::unique_ptr<_timelib_time, TimeZone::TimelibTimeDeleter> createTimelibTime() {
+    return std::unique_ptr<_timelib_time, TimeZone::TimelibTimeDeleter>(timelib_time_ctor());
+}
+
 // Converts a date to a number of seconds, being careful to round appropriately for negative numbers
 // of seconds.
 long long seconds(Date_t date) {
@@ -373,9 +378,14 @@ void TimeZone::adjustTimeZone(timelib_time* timelibTime) const {
     timelib_update_from_sse(timelibTime);
 }
 
-Date_t TimeZone::createFromDateParts(
-    int year, int month, int day, int hour, int minute, int second, int millisecond) const {
-    std::unique_ptr<timelib_time, TimeZone::TimelibTimeDeleter> newTime(timelib_time_ctor());
+Date_t TimeZone::createFromDateParts(long long year,
+                                     long long month,
+                                     long long day,
+                                     long long hour,
+                                     long long minute,
+                                     long long second,
+                                     long long millisecond) const {
+    auto newTime = createTimelibTime();
 
     newTime->y = year;
     newTime->m = month;
@@ -393,14 +403,14 @@ Date_t TimeZone::createFromDateParts(
     return returnValue;
 }
 
-Date_t TimeZone::createFromIso8601DateParts(int isoYear,
-                                            int isoWeekYear,
-                                            int isoDayOfWeek,
-                                            int hour,
-                                            int minute,
-                                            int second,
-                                            int millisecond) const {
-    std::unique_ptr<timelib_time, TimeZone::TimelibTimeDeleter> newTime(timelib_time_ctor());
+Date_t TimeZone::createFromIso8601DateParts(long long isoYear,
+                                            long long isoWeekYear,
+                                            long long isoDayOfWeek,
+                                            long long hour,
+                                            long long minute,
+                                            long long second,
+                                            long long millisecond) const {
+    auto newTime = createTimelibTime();
 
     timelib_date_from_isodate(
         isoYear, isoWeekYear, isoDayOfWeek, &newTime->y, &newTime->m, &newTime->d);
@@ -468,7 +478,7 @@ void TimeZone::TimelibTimeDeleter::operator()(timelib_time* time) {
 
 std::unique_ptr<timelib_time, TimeZone::TimelibTimeDeleter> TimeZone::getTimelibTime(
     Date_t date) const {
-    std::unique_ptr<timelib_time, TimeZone::TimelibTimeDeleter> time(timelib_time_ctor());
+    auto time = createTimelibTime();
 
     timelib_unixtime2gmt(time.get(), seconds(date));
     adjustTimeZone(time.get());
diff --git a/src/mongo/db/query/datetime/date_time_support.h b/src/mongo/db/query/datetime/date_time_support.h
index 36125268532..9a94fd657b2 100644
--- a/src/mongo/db/query/datetime/date_time_support.h
+++ b/src/mongo/db/query/datetime/date_time_support.h
@@ -96,19 +96,24 @@ public:
     /**
      * Returns a Date_t populated with the argument values for the current timezone.
      */
-    Date_t createFromDateParts(
-        int year, int month, int day, int hour, int minute, int second, int millisecond) const;
+    Date_t createFromDateParts(long long year,
+                               long long month,
+                               long long day,
+                               long long hour,
+                               long long minute,
+                               long long second,
+                               long long millisecond) const;
 
     /**
      * Returns a Date_t populated with the argument values for the current timezone.
      */
-    Date_t createFromIso8601DateParts(int isoYear,
-                                      int isoWeekYear,
-                                      int isoDayOfWeek,
-                                      int hour,
-                                      int minute,
-                                      int second,
-                                      int millisecond) const;
+    Date_t createFromIso8601DateParts(long long isoYear,
+                                      long long isoWeekYear,
+                                      long long isoDayOfWeek,
+                                      long long hour,
+                                      long long minute,
+                                      long long second,
+                                      long long millisecond) const;
     /**
      * Returns a struct with members for each piece of the date.
      */