Skip to content

Commit d4843fc

Browse files
jseysterjseyster
committed
SERVER-40383 Handle week, day edge cases in timelib_date_from_isodate
1 parent fbf80b6 commit d4843fc

File tree

5 files changed

+256
-59
lines changed

5 files changed

+256
-59
lines changed

‎jstests/aggregation/expressions/date_from_parts.js‎

Lines changed: 113 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
99
/* Basic Sanity Checks */
1010
coll.drop();
1111

12-
assert.writeOK(coll.insert([
12+
assert.commandWorked(coll.insert([
1313
{_id: 0, year: 2017, month: 6, day: 19, hour: 15, minute: 13, second: 25, millisecond: 713},
1414
{
1515
_id: 1,
@@ -105,7 +105,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
105105

106106
coll.drop();
107107

108-
assert.writeOK(coll.insert([
108+
assert.commandWorked(coll.insert([
109109
{
110110
_id: 0,
111111
year: 2017,
@@ -389,7 +389,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
389389

390390
coll.drop();
391391

392-
assert.writeOK(coll.insert([
392+
assert.commandWorked(coll.insert([
393393
{_id: 0},
394394
]));
395395

@@ -421,7 +421,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
421421

422422
coll.drop();
423423

424-
assert.writeOK(coll.insert([
424+
assert.commandWorked(coll.insert([
425425
{_id: 0, falseValue: false},
426426
]));
427427

@@ -453,7 +453,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
453453

454454
coll.drop();
455455

456-
assert.writeOK(coll.insert([
456+
assert.commandWorked(coll.insert([
457457
{_id: 0, outOfRangeValue: 10002},
458458
]));
459459

@@ -472,7 +472,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
472472

473473
coll.drop();
474474

475-
assert.writeOK(coll.insert([{
475+
assert.commandWorked(coll.insert([{
476476
_id: 0,
477477
minusOne: -1,
478478
zero: 0,
@@ -548,7 +548,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
548548
*/
549549
coll.drop();
550550

551-
assert.writeOK(coll.insert([{
551+
assert.commandWorked(coll.insert([{
552552
_id: 0,
553553
veryBigDoubleA: 18014398509481984.0,
554554
veryBigDecimal128A: NumberDecimal("9223372036854775807"), // 2^63-1
@@ -580,12 +580,113 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
580580
[{$project: {date: {"$dateFromParts": {year: 1970, millisecond: "$veryBigDecimal128B"}}}}];
581581
assertErrMsgContains(coll, pipeline, 40515, "'millisecond' must evaluate to an integer");
582582

583+
/* --------------------------------------------------------------------------------------- */
584+
/* Testing that year values are only allowed in the range [0, 9999] and that month, day, hour,
585+
* and minute values are only allowed in the range [-32,768, 32,767]. */
586+
coll.drop();
587+
588+
assert.commandWorked(coll.insert([{
589+
_id: 0,
590+
bigYear: 10000,
591+
smallYear: -1,
592+
prettyBigInt: 32768,
593+
prettyBigNegativeInt: -32769
594+
}]));
595+
596+
pipeline = [{$project: {date: {"$dateFromParts": {year: "$bigYear"}}}}];
597+
assertErrMsgContains(
598+
coll, pipeline, 40523, "'year' must evaluate to an integer in the range 0 to 9999");
599+
600+
pipeline = [{$project: {date: {"$dateFromParts": {year: "$smallYear"}}}}];
601+
assertErrMsgContains(
602+
coll, pipeline, 40523, "'year' must evaluate to an integer in the range 0 to 9999");
603+
604+
pipeline = [{$project: {date: {"$dateFromParts": {year: 1970, month: "$prettyBigInt"}}}}];
605+
assertErrMsgContains(
606+
coll, pipeline, 31034, "'month' must evaluate to a value in the range [-32768, 32767]");
607+
608+
pipeline =
609+
[{$project: {date: {"$dateFromParts": {year: 1970, month: "$prettyBigNegativeInt"}}}}];
610+
assertErrMsgContains(
611+
coll, pipeline, 31034, "'month' must evaluate to a value in the range [-32768, 32767]");
612+
613+
pipeline =
614+
[{$project: {date: {"$dateFromParts": {year: 1970, month: 1, day: "$prettyBigInt"}}}}];
615+
assertErrMsgContains(
616+
coll, pipeline, 31034, "'day' must evaluate to a value in the range [-32768, 32767]");
617+
618+
pipeline = [{
619+
$project:
620+
{date: {"$dateFromParts": {year: 1970, month: 1, day: "$prettyBigNegativeInt"}}}
621+
}];
622+
assertErrMsgContains(
623+
coll, pipeline, 31034, "'day' must evaluate to a value in the range [-32768, 32767]");
624+
625+
pipeline = [{$project: {date: {"$dateFromParts": {year: 1970, hour: "$prettyBigInt"}}}}];
626+
assertErrMsgContains(
627+
coll, pipeline, 31034, "'hour' must evaluate to a value in the range [-32768, 32767]");
628+
629+
pipeline =
630+
[{$project: {date: {"$dateFromParts": {year: 1970, hour: "$prettyBigNegativeInt"}}}}];
631+
assertErrMsgContains(
632+
coll, pipeline, 31034, "'hour' must evaluate to a value in the range [-32768, 32767]");
633+
634+
pipeline =
635+
[{$project: {date: {"$dateFromParts": {year: 1970, hour: 0, minute: "$prettyBigInt"}}}}];
636+
assertErrMsgContains(
637+
coll, pipeline, 31034, "'minute' must evaluate to a value in the range [-32768, 32767]");
638+
639+
pipeline = [{
640+
$project:
641+
{date: {"$dateFromParts": {year: 1970, hour: 0, minute: "$prettyBigNegativeInt"}}}
642+
}];
643+
assertErrMsgContains(
644+
coll, pipeline, 31034, "'minute' must evaluate to a value in the range [-32768, 32767]");
645+
646+
pipeline = [{$project: {date: {"$dateFromParts": {isoWeekYear: "$bigYear"}}}}];
647+
assertErrMsgContains(
648+
coll, pipeline, 31095, "'isoWeekYear' must evaluate to an integer in the range 0 to 9999");
649+
650+
pipeline = [{$project: {date: {"$dateFromParts": {isoWeekYear: "$smallYear"}}}}];
651+
assertErrMsgContains(
652+
coll, pipeline, 31095, "'isoWeekYear' must evaluate to an integer in the range 0 to 9999");
653+
654+
pipeline =
655+
[{$project: {date: {"$dateFromParts": {isoWeekYear: 1970, isoWeek: "$prettyBigInt"}}}}];
656+
assertErrMsgContains(
657+
coll, pipeline, 31034, "'isoWeek' must evaluate to a value in the range [-32768, 32767]");
658+
659+
pipeline = [{
660+
$project:
661+
{date: {"$dateFromParts": {isoWeekYear: 1970, isoWeek: "$prettyBigNegativeInt"}}}
662+
}];
663+
assertErrMsgContains(
664+
coll, pipeline, 31034, "'isoWeek' must evaluate to a value in the range [-32768, 32767]");
665+
666+
pipeline = [
667+
{$project: {date: {"$dateFromParts": {isoWeekYear: 1970, isoDayOfWeek: "$prettyBigInt"}}}}
668+
];
669+
assertErrMsgContains(coll,
670+
pipeline,
671+
31034,
672+
"'isoDayOfWeek' must evaluate to a value in the range [-32768, 32767]");
673+
674+
pipeline = [{
675+
$project: {
676+
date: {"$dateFromParts": {isoWeekYear: 1970, isoDayOfWeek: "$prettyBigNegativeInt"}}
677+
}
678+
}];
679+
assertErrMsgContains(coll,
680+
pipeline,
681+
31034,
682+
"'isoDayOfWeek' must evaluate to a value in the range [-32768, 32767]");
683+
583684
/* --------------------------------------------------------------------------------------- */
584685
/* Testing wrong arguments */
585686

586687
coll.drop();
587688

588-
assert.writeOK(coll.insert([
689+
assert.commandWorked(coll.insert([
589690
{_id: 0},
590691
]));
591692

@@ -630,7 +731,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
630731

631732
coll.drop();
632733

633-
assert.writeOK(coll.insert([
734+
assert.commandWorked(coll.insert([
634735
{_id: 0, floatField: 2017.5, decimalField: NumberDecimal("2017.5")},
635736
]));
636737

@@ -653,7 +754,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
653754

654755
coll.drop();
655756

656-
assert.writeOK(coll.insert([
757+
assert.commandWorked(coll.insert([
657758
{_id: 0, year: NumberDecimal("2017"), month: 6.0, day: NumberInt(19), hour: NumberLong(15)},
658759
{
659760
_id: 1,
@@ -733,7 +834,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
733834

734835
coll.drop();
735836

736-
assert.writeOK(coll.insert([
837+
assert.commandWorked(coll.insert([
737838
{
738839
_id: 0,
739840
year: NumberDecimal("2017"),
@@ -783,7 +884,7 @@ load("jstests/aggregation/extras/utils.js"); // For assertErrorCode and assertE
783884

784885
coll.drop();
785886

786-
assert.writeOK(coll.insert([
887+
assert.commandWorked(coll.insert([
787888
{
788889
_id: 0,
789890
isoWeekYear: NumberDecimal("2017"),

‎src/mongo/db/pipeline/expression.cpp‎

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,7 +1204,7 @@ Value ExpressionDateFromParts::serialize(bool explain) const {
12041204
}
12051205

12061206
bool ExpressionDateFromParts::evaluateNumberWithDefault(const Document& root,
1207-
intrusive_ptr<Expression> field,
1207+
const Expression* field,
12081208
StringData fieldName,
12091209
long long defaultValue,
12101210
long long* returnValue,
@@ -1232,14 +1232,39 @@ bool ExpressionDateFromParts::evaluateNumberWithDefault(const Document& root,
12321232
return true;
12331233
}
12341234

1235+
bool ExpressionDateFromParts::evaluateNumberWithDefaultAndBounds(const Document& root,
1236+
const Expression* field,
1237+
StringData fieldName,
1238+
long long defaultValue,
1239+
long long* returnValue,
1240+
Variables* variables) const {
1241+
bool result =
1242+
evaluateNumberWithDefault(root, field, fieldName, defaultValue, returnValue, variables);
1243+
1244+
uassert(31034,
1245+
str::stream() << "'" << fieldName << "'"
1246+
<< " must evaluate to a value in the range ["
1247+
<< kMinValueForDatePart
1248+
<< ", "
1249+
<< kMaxValueForDatePart
1250+
<< "]; value "
1251+
<< *returnValue
1252+
<< " is not in range",
1253+
!result ||
1254+
(*returnValue >= kMinValueForDatePart && *returnValue <= kMaxValueForDatePart));
1255+
1256+
return result;
1257+
}
1258+
12351259
Value ExpressionDateFromParts::evaluate(const Document& root, Variables* variables) const {
12361260
long long hour, minute, second, millisecond;
12371261

1238-
if (!evaluateNumberWithDefault(root, _hour, "hour"_sd, 0, &hour, variables) ||
1239-
!evaluateNumberWithDefault(root, _minute, "minute"_sd, 0, &minute, variables) ||
1240-
!evaluateNumberWithDefault(root, _second, "second"_sd, 0, &second, variables) ||
1262+
if (!evaluateNumberWithDefaultAndBounds(root, _hour.get(), "hour"_sd, 0, &hour, variables) ||
1263+
!evaluateNumberWithDefaultAndBounds(
1264+
root, _minute.get(), "minute"_sd, 0, &minute, variables) ||
1265+
!evaluateNumberWithDefault(root, _second.get(), "second"_sd, 0, &second, variables) ||
12411266
!evaluateNumberWithDefault(
1242-
root, _millisecond, "millisecond"_sd, 0, &millisecond, variables)) {
1267+
root, _millisecond.get(), "millisecond"_sd, 0, &millisecond, variables)) {
12431268
// One of the evaluated inputs in nullish.
12441269
return Value(BSONNULL);
12451270
}
@@ -1254,9 +1279,10 @@ Value ExpressionDateFromParts::evaluate(const Document& root, Variables* variabl
12541279
if (_year) {
12551280
long long year, month, day;
12561281

1257-
if (!evaluateNumberWithDefault(root, _year, "year"_sd, 1970, &year, variables) ||
1258-
!evaluateNumberWithDefault(root, _month, "month"_sd, 1, &month, variables) ||
1259-
!evaluateNumberWithDefault(root, _day, "day"_sd, 1, &day, variables)) {
1282+
if (!evaluateNumberWithDefault(root, _year.get(), "year"_sd, 1970, &year, variables) ||
1283+
!evaluateNumberWithDefaultAndBounds(
1284+
root, _month.get(), "month"_sd, 1, &month, variables) ||
1285+
!evaluateNumberWithDefaultAndBounds(root, _day.get(), "day"_sd, 1, &day, variables)) {
12601286
// One of the evaluated inputs in nullish.
12611287
return Value(BSONNULL);
12621288
}
@@ -1276,14 +1302,23 @@ Value ExpressionDateFromParts::evaluate(const Document& root, Variables* variabl
12761302
long long isoWeekYear, isoWeek, isoDayOfWeek;
12771303

12781304
if (!evaluateNumberWithDefault(
1279-
root, _isoWeekYear, "isoWeekYear"_sd, 1970, &isoWeekYear, variables) ||
1280-
!evaluateNumberWithDefault(root, _isoWeek, "isoWeek"_sd, 1, &isoWeek, variables) ||
1281-
!evaluateNumberWithDefault(
1282-
root, _isoDayOfWeek, "isoDayOfWeek"_sd, 1, &isoDayOfWeek, variables)) {
1305+
root, _isoWeekYear.get(), "isoWeekYear"_sd, 1970, &isoWeekYear, variables) ||
1306+
!evaluateNumberWithDefaultAndBounds(
1307+
root, _isoWeek.get(), "isoWeek"_sd, 1, &isoWeek, variables) ||
1308+
!evaluateNumberWithDefaultAndBounds(
1309+
root, _isoDayOfWeek.get(), "isoDayOfWeek"_sd, 1, &isoDayOfWeek, variables)) {
12831310
// One of the evaluated inputs in nullish.
12841311
return Value(BSONNULL);
12851312
}
12861313

1314+
uassert(31095,
1315+
str::stream() << "'isoWeekYear' must evaluate to an integer in the range " << 0
1316+
<< " to "
1317+
<< 9999
1318+
<< ", found "
1319+
<< isoWeekYear,
1320+
isoWeekYear >= 0 && isoWeekYear <= 9999);
1321+
12871322
return Value(timeZone->createFromIso8601DateParts(
12881323
isoWeekYear, isoWeek, isoDayOfWeek, hour, minute, second, millisecond));
12891324
}

‎src/mongo/db/pipeline/expression.h‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1100,12 +1100,24 @@ class ExpressionDateFromParts final : public Expression {
11001100
* out parameter, and the function returns true.
11011101
*/
11021102
bool evaluateNumberWithDefault(const Document& root,
1103-
boost::intrusive_ptr<Expression> field,
1103+
const Expression* field,
11041104
StringData fieldName,
11051105
long long defaultValue,
11061106
long long* returnValue,
11071107
Variables* variables) const;
11081108

1109+
/**
1110+
* This function has the same behavior as evaluteNumberWithDefault(), except that it uasserts if
1111+
* the resulting value is not in the range defined by kMaxValueForDatePart and
1112+
* kMinValueForDatePart.
1113+
*/
1114+
bool evaluateNumberWithDefaultAndBounds(const Document& root,
1115+
const Expression* field,
1116+
StringData fieldName,
1117+
long long defaultValue,
1118+
long long* returnValue,
1119+
Variables* variables) const;
1120+
11091121
boost::intrusive_ptr<Expression>& _year;
11101122
boost::intrusive_ptr<Expression>& _month;
11111123
boost::intrusive_ptr<Expression>& _day;
@@ -1117,6 +1129,12 @@ class ExpressionDateFromParts final : public Expression {
11171129
boost::intrusive_ptr<Expression>& _isoWeek;
11181130
boost::intrusive_ptr<Expression>& _isoDayOfWeek;
11191131
boost::intrusive_ptr<Expression>& _timeZone;
1132+
1133+
// Some date conversions spend a long time iterating through date tables when dealing with large
1134+
// input numbers, so we place a reasonable limit on the magnitude of any argument to
1135+
// $dateFromParts: inputs that fit within a 16-bit int are permitted.
1136+
static constexpr long long kMaxValueForDatePart = std::numeric_limits<int16_t>::max();
1137+
static constexpr long long kMinValueForDatePart = std::numeric_limits<int16_t>::lowest();
11201138
};
11211139

11221140
class ExpressionDateToParts final : public Expression {

0 commit comments

Comments
 (0)