SERVER-59765 Improved 'amount' parameter validation for $dateAdd/$dateSubtract

4 files changed, 151 insertions, 5 deletions

diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml
index 85a3e6a4adf..9a99520b891 100644
--- a/etc/backports_required_for_multiversion_tests.yml
+++ b/etc/backports_required_for_multiversion_tests.yml
@@ -334,6 +334,8 @@ last-lts:
       test_file: jstests/replsets/tenant_migration_fetch_committed_transactions_retry.js
     - ticket: SERVER-59023
       test_file: jstests/sharding/resharding_secondary_recovers_temp_ns_metadata.js
+    - ticket: SERVER-59765
+      test_file: jstests/aggregation/expressions/date_add_subtract.js
 
   # Tests that should only be excluded from particular suites should be listed under that suite.
   suites:
diff --git a/jstests/aggregation/expressions/date_add_subtract.js b/jstests/aggregation/expressions/date_add_subtract.js
index 57490d19622..8079e64ae83 100644
--- a/jstests/aggregation/expressions/date_add_subtract.js
+++ b/jstests/aggregation/expressions/date_add_subtract.js
@@ -204,9 +204,13 @@ function runAndAssertErrorCode(dateArithmeticsSpec, expectedErrorCode) {
     runAndAssertErrorCode({$dateSubtract: {startDate: "$date", unit: "year", amount: 1.001}},
                           5166405);
 
-    // Overflow error of dateAdd operation due to high amount.
-    runAndAssertErrorCode({$dateSubtract: {startDate: "$date", unit: "month", amount: 30000000000}},
-                          5166406);
+    // Overflow error of dateAdd operation due to large amount.
+    runAndAssertErrorCode(
+        {$dateSubtract: {startDate: "$date", unit: "month", amount: 12 * 300000000}}, 5166406);
+
+    // Invalid 'amount' parameter error of dateAdd operation due to large amount.
+    runAndAssertErrorCode(
+        {$dateSubtract: {startDate: "$date", unit: "month", amount: -30000000000}}, 5976500);
 
     // Invalid value of timezone argument.
     runAndAssertErrorCode(
diff --git a/src/mongo/db/query/datetime/date_time_support.cpp b/src/mongo/db/query/datetime/date_time_support.cpp
index fb38063b0af..1fe0ecd81d0 100644
--- a/src/mongo/db/query/datetime/date_time_support.cpp
+++ b/src/mongo/db/query/datetime/date_time_support.cpp
@@ -635,7 +635,8 @@ inline long leapYearsSinceReferencePoint(long year) {
 
 /**
  * Sums the number of days in the Gregorian calendar in years: 'startYear',
- * 'startYear'+1, .., 'endYear'-1.
+ * 'startYear'+1, .., 'endYear'-1. 'startYear' and 'endYear' are expected to be from the range
+ * (-1000'000'000; +1000'000'000).
  */
 inline long long daysBetweenYears(long startYear, long endYear) {
     return leapYearsSinceReferencePoint(endYear - 1) - leapYearsSinceReferencePoint(startYear - 1) +
@@ -1134,6 +1135,45 @@ std::pair<Date_t, Date> defaultReferencePointForDateTrunc(const TimeZone& timezo
     }
     return {Date_t::fromMillisSinceEpoch(referencePointMillis), referencePoint};
 }
+
+/**
+ * Determines if function 'dateAdd()' parameter 'amount' and 'unit' values are valid - the
+ * amount roughly fits the range of Date_t type.
+ */
+bool isDateAddAmountValid(long long amount, TimeUnit unit) {
+    constexpr long long maxDays{
+        std::numeric_limits<unsigned long long>::max() / kMillisecondsPerDay + 1};
+    constexpr auto maxYears = maxDays / 365 /* minimum number of days per year*/ + 1;
+    constexpr auto maxQuarters = maxYears * kQuartersPerYear;
+    constexpr auto maxMonths = maxYears * kMonthsInOneYear;
+    constexpr auto maxWeeks = maxDays / kDaysPerWeek;
+    constexpr auto maxHours = maxDays * kHoursPerDay;
+    constexpr auto maxMinutes = maxHours * kMinutesPerHour;
+    constexpr auto maxSeconds = maxMinutes * kSecondsPerMinute;
+    const auto maxAbsoluteAmountValue = [&](TimeUnit unit) {
+        switch (unit) {
+            case TimeUnit::year:
+                return maxYears;
+            case TimeUnit::quarter:
+                return maxQuarters;
+            case TimeUnit::month:
+                return maxMonths;
+            case TimeUnit::week:
+                return maxWeeks;
+            case TimeUnit::day:
+                return maxDays;
+            case TimeUnit::hour:
+                return maxHours;
+            case TimeUnit::minute:
+                return maxMinutes;
+            case TimeUnit::second:
+                return maxSeconds;
+            default:
+                MONGO_UNREACHABLE_TASSERT(5976501);
+        }
+    }(unit);
+    return -maxAbsoluteAmountValue < amount && amount < maxAbsoluteAmountValue;
+}
 }  // namespace
 
 Date_t dateAdd(Date_t date, TimeUnit unit, long long amount, const TimeZone& timezone) {
@@ -1141,6 +1181,14 @@ Date_t dateAdd(Date_t date, TimeUnit unit, long long amount, const TimeZone& tim
         return date + Milliseconds(amount);
     }
 
+    // Check that 'amount' value is within an acceptable range. If the value is within acceptable
+    // range, then the addition algorithm is expected to not overflow. The final determination if
+    // the result can be represented as Date_t is done after the addition result is computed.
+    uassert(5976500,
+            str::stream() << "invalid dateAdd 'amount' parameter value: " << amount << " "
+                          << serializeTimeUnit(unit),
+            isDateAddAmountValid(amount, unit));
+
     auto localTime = timezone.getTimelibTime(date);
     auto microSec = durationCount<Microseconds>(Milliseconds(date.toMillisSinceEpoch() % 1000));
     localTime->us = microSec;
diff --git a/src/mongo/db/query/datetime/date_time_support_test.cpp b/src/mongo/db/query/datetime/date_time_support_test.cpp
index 049b7fee220..d2b2e3b2d44 100644
--- a/src/mongo/db/query/datetime/date_time_support_test.cpp
+++ b/src/mongo/db/query/datetime/date_time_support_test.cpp
@@ -2174,7 +2174,7 @@ TEST(TruncateDate, ErrorHandling) {
     ASSERT_THROWS_CODE(
         truncateDate(dateBeforeReferencePoint, TimeUnit::year, 1'000'000'000ULL, *timezone),
         AssertionException,
-        5166406);
+        5976500);
 
     // Verify computation with large bin size when the result is in the long past succeeds.
     ASSERT_EQ(timezone->createFromDateParts(-200'000'000LL, 1, 1, 0, 0, 0, 0),
@@ -2207,6 +2207,90 @@ TEST(DateAdd, DateAddYear) {
               kDefaultTimeZone.createFromDateParts(2012, 2, 29, 0, 0, 0, 0));
 }
 
+TEST(DateAdd, LargeAmountValues) {
+    const auto anyDate = kDefaultTimeZone.createFromDateParts(2016, 1, 1, 0, 0, 0, 0);
+    const auto smallDate = kDefaultTimeZone.createFromDateParts(-291'000'000, 3, 31, 0, 0, 0, 0);
+    struct TestCase {
+        TimeUnit unit;
+        long long invalidAmount;   // Amount value rejected in initial validation.
+        long long overflowAmount;  // Amount to add to date -200'000'000-2-29 00:00:00.000 so the
+                                   // result cannot be represented as Date_t.
+        long long largeAmount;     // Large amount to add to 'smallDate' so the result is equal to
+                                   // 'largeAmountExpectedDate'.
+        Date_t largeAmountExpectedDate;
+    };
+    const auto maxValidYearAmountPlus1{584'942'417LL + 1};
+    const auto maxValidDayAmountPlus1{213'503'982'334LL + 1};
+    const std::vector<TestCase> testCases{
+        {TimeUnit::year,
+         maxValidYearAmountPlus1,      // Invalid amount.
+         maxValidYearAmountPlus1 - 1,  // Overflow amount.
+         550'000'000LL,                // Large amount.
+         kDefaultTimeZone.createFromDateParts(-291'000'000 + 550'000'000, 3, 31, 0, 0, 0, 0)},
+        {TimeUnit::quarter,
+         maxValidYearAmountPlus1 * 4,      // Invalid amount.
+         maxValidYearAmountPlus1 * 4 - 1,  // Overflow amount.
+         550'000'000LL * 4,                // Large amount.
+         kDefaultTimeZone.createFromDateParts(-291'000'000 + 550'000'000, 3, 31, 0, 0, 0, 0)},
+        {TimeUnit::month,
+         maxValidYearAmountPlus1 * 12,      // Invalid amount.
+         maxValidYearAmountPlus1 * 12 - 1,  // Overflow amount.
+         550'000'000LL * 12,                // Large amount.
+         kDefaultTimeZone.createFromDateParts(-291'000'000 + 550'000'000, 3, 31, 0, 0, 0, 0)},
+        {TimeUnit::day,
+         maxValidDayAmountPlus1,      // Invalid amount.
+         maxValidDayAmountPlus1 - 1,  // Overflow amount.
+         250'000'000LL * 365,         // Large amount.
+         smallDate + Days(250'000'000LL * 365)},
+        {TimeUnit::hour,
+         maxValidDayAmountPlus1 * 24,      // Invalid amount.
+         maxValidDayAmountPlus1 * 24 - 1,  // Overflow amount.
+         250'000'000LL * 365 * 24,         // Large amount.
+         smallDate + Days(250'000'000LL * 365)},
+        {TimeUnit::minute,
+         maxValidDayAmountPlus1 * 24 * 60,      // Invalid amount.
+         maxValidDayAmountPlus1 * 24 * 60 - 1,  // Overflow amount.
+         250'000'000LL * 365 * 24 * 60,         // Large amount.
+         smallDate + Days(250'000'000LL * 365)},
+        {TimeUnit::second,
+         maxValidDayAmountPlus1 * 24 * 60 * 60,      // Invalid amount.
+         maxValidDayAmountPlus1 * 24 * 60 * 60 - 1,  // Overflow amount.
+         250'000'000LL * 365 * 24 * 60 * 60,         // Large amount.
+         smallDate + Days(250'000'000LL * 365)},
+    };
+    int testCaseIdx{0};
+    for (auto&& testCase : testCases) {
+        // Verify that out-of-range amount values are rejected.
+        ASSERT_THROWS_CODE(
+            dateAdd(anyDate, testCase.unit, testCase.invalidAmount, kDefaultTimeZone),
+            AssertionException,
+            5976500)
+            << " test case# " << testCaseIdx;
+        ASSERT_THROWS_CODE(
+            dateAdd(anyDate, testCase.unit, -testCase.invalidAmount, kDefaultTimeZone),
+            AssertionException,
+            5976500)
+            << " test case# " << testCaseIdx;
+
+        // Verify that overflow is detected when the result cannot be represented as Date_t.
+        ASSERT_THROWS_CODE(
+            dateAdd(kDefaultTimeZone.createFromDateParts(-200'000'000, 2, 29, 0, 0, 0, 0),
+                    testCase.unit,
+                    testCase.overflowAmount,
+                    kDefaultTimeZone),
+            AssertionException,
+            5166406)
+            << " test case# " << testCaseIdx;
+
+        // Verify that adding large values works correctly.
+        ASSERT_EQ(dateAdd(smallDate, testCase.unit, testCase.largeAmount, kDefaultTimeZone),
+                  testCase.largeAmountExpectedDate)
+            << " test case# " << testCaseIdx;
+
+        ++testCaseIdx;
+    }
+}
+
 TEST(DateAdd, DateAddQuarter) {
     auto startDate = kDefaultTimeZone.createFromDateParts(2020, 1, 1, 0, 0, 0, 3);
     ASSERT_EQ(dateAdd(startDate, TimeUnit::quarter, 1, kDefaultTimeZone),
@@ -2850,6 +2934,14 @@ TEST(DateAdd, DateAddMillisecond) {
 
     ASSERT_EQ(dateAdd(startDate, TimeUnit::millisecond, -1501, kDefaultTimeZone),
               kDefaultTimeZone.createFromDateParts(2020, 12, 31, 23, 59, 13, 500));
+
+    // Verify that an overflow is detected.
+    ASSERT_THROWS_CODE(dateAdd(startDate,
+                               TimeUnit::millisecond,
+                               std::numeric_limits<long long>::max(),
+                               kDefaultTimeZone),
+                       AssertionException,
+                       ErrorCodes::Error::DurationOverflow);
 }
 
 TEST(IsValidDayOfWeek, Basic) {