diff options
author | Nick Zolnierz <nicholas.zolnierz@mongodb.com> | 2018-02-01 11:12:12 -0500 |
---|---|---|
committer | Nick Zolnierz <nicholas.zolnierz@mongodb.com> | 2018-02-09 10:12:05 -0500 |
commit | 695d94255348302be2d804e2187eb61e15cbb412 (patch) | |
tree | 6d0ab734ac26b158358ceb5fc9a42b7b0c3bb064 /src | |
parent | d1b2515a2d7dd4a22222de7d8905d6dc6b1ab1be (diff) | |
download | mongo-695d94255348302be2d804e2187eb61e15cbb412.tar.gz |
SERVER-30523: dateFromParts should not reject out-of-range numbers for date/time properties
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/pipeline/document_value_test.cpp | 71 | ||||
-rw-r--r-- | src/mongo/db/pipeline/expression.cpp | 76 | ||||
-rw-r--r-- | src/mongo/db/pipeline/expression.h | 22 | ||||
-rw-r--r-- | src/mongo/db/pipeline/expression_test.cpp | 32 | ||||
-rw-r--r-- | src/mongo/db/pipeline/value.cpp | 29 | ||||
-rw-r--r-- | src/mongo/db/pipeline/value.h | 6 | ||||
-rw-r--r-- | src/mongo/db/query/datetime/date_time_support.cpp | 34 | ||||
-rw-r--r-- | src/mongo/db/query/datetime/date_time_support.h | 23 |
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. */ |