[SERVER-40383] dateFromParts does not overflow correctly if isoWeek smaller than 1 Created: 29/Mar/19  Updated: 29/Oct/23  Resolved: 01/Jul/19

Status: Closed
Project: Core Server
Component/s: Aggregation Framework
Affects Version/s: 4.0.6
Fix Version/s: 4.0.11, 4.2.0-rc3, 4.3.1

Type: Bug Priority: Major - P3
Reporter: Sam Tolmay Assignee: Justin Seyster
Resolution: Fixed Votes: 0
Labels: query-44-grooming
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: File SERVER-40383-master.patch     File SERVER-40383-v4.0.patch    
Issue Links:
Backports
Problem/Incident
Related
related to SERVER-30523 dateFromParts should not reject "out-... Closed
related to SERVER-40573 timelib can read past end of buffer, ... Closed
Backwards Compatibility: Minor Change
Operating System: ALL
Backport Requested:
v4.2, v4.0, v3.6
Sprint: Query 2019-05-06, Query 2019-05-20, Query 2019-06-03, Query 2019-06-17, Query 2019-07-01, Query 2019-07-15
Participants:
Linked BF Score: 31

 Description   

When using dateFromParts with ISO8601 weeks, the date does not calculate correctly if the isoWeek parameter is less than 1.

Some examples:

 'date'*:* { $dateFromParts: { isoWeekYear: 2019, isoWeek: 0 } } gives ** 2019-12-24
 
 'date'*:* { $dateFromParts: { isoWeekYear: 2019, isoWeek: -1 } } gives ** 2019-12-17
 
 'date'*:* { $dateFromParts: { isoWeekYear: 2019, isoWeek: -51 } } gives ** 2019-01-01
 
 'date'*:* { $dateFromParts: { isoWeekYear: 2019, isoWeek: -52 } } gives ** 2018-12-25



 Comments   
Comment by Githook User [ 01/Jul/19 ]

Author:

{'name': 'Justin Seyster', 'username': 'jseyster', 'email': 'justin.seyster@mongodb.com'}

Message: SERVER-40383 Handle week, day edge cases in timelib_date_from_isodate

(cherry picked from commit d4843fc49931c7ce4332dc373623267c293eb518)
Branch: v4.0
https://github.com/mongodb/mongo/commit/40543b7be5307541f4cab8df5bee73ffa65b5a1c

Comment by Githook User [ 01/Jul/19 ]

Author:

{'name': 'Justin Seyster', 'username': 'jseyster', 'email': 'justin.seyster@mongodb.com'}

Message: SERVER-40383 Handle week, day edge cases in timelib_date_from_isodate

(cherry picked from commit d4843fc49931c7ce4332dc373623267c293eb518)
Branch: v4.2
https://github.com/mongodb/mongo/commit/52f4f202fee36d16873e3765f7b20ef5e798f711

Comment by Githook User [ 01/Jul/19 ]

Author:

{'name': 'Justin Seyster', 'username': 'jseyster', 'email': 'justin.seyster@mongodb.com'}

Message: SERVER-40383 Handle week, day edge cases in timelib_date_from_isodate
Branch: master
https://github.com/mongodb/mongo/commit/d4843fc49931c7ce4332dc373623267c293eb518

Comment by Derick Rethans [ 30/Jun/19 ]

I've merged this patch in timelib now: https://github.com/derickr/timelib/pull/66

Comment by Justin Seyster [ 04/Jun/19 ]

The proposed changes introduce some minor changes in behavior that will cause failures in the multiversion fuzzer if we don't include the same changes from v4.0 up to master.

Comment by David Storch [ 10/Apr/19 ]

Hi SamTolmay,

Thanks for the issue report! I spent some time tracking this down, and there is definitely a flaw in year-related arithmetic in timelib_date_from_isodate(). The problematic code is little bit arcane, but I've prepared a fix that also simplifies the logic around computing the correct year:

diff --git a/src/third_party/timelib-2018.01/dow.c b/src/third_party/timelib-2018.01/dow.c
index 33553e7806..4bd5819450 100644
--- a/src/third_party/timelib-2018.01/dow.c
+++ b/src/third_party/timelib-2018.01/dow.c
@@ -144,7 +144,7 @@ void timelib_isodate_from_date(timelib_sll y, timelib_sll m, timelib_sll d, time
        *id = timelib_day_of_week_ex(y, m, d, 1);
 }
 
-static timelib_sll timelib_daynr_from_weeknr_ex(timelib_sll iy, timelib_sll iw, timelib_sll id, timelib_sll *y)
+timelib_sll timelib_daynr_from_weeknr(timelib_sll iy, timelib_sll iw, timelib_sll id)
 {
        timelib_sll dow, day;
 
@@ -152,30 +152,18 @@ static timelib_sll timelib_daynr_from_weeknr_ex(timelib_sll iy, timelib_sll iw,
        dow = timelib_day_of_week(iy, 1, 1);
        /* then use that to figure out the offset for day 1 of week 1 */
        day = 0 - (dow > 4 ? dow - 7 : dow);
-       /* and adjust the year to the natural year if we need to */
-       *y = (iw == 1 && day < 0 && id < dow) ? iy - 1 : iy;
 
        /* Add weeks and days */
        return day + ((iw - 1) * 7) + id;
 }
 
-timelib_sll timelib_daynr_from_weeknr(timelib_sll iy, timelib_sll iw, timelib_sll id)
-{
-       timelib_sll dummy_iso_year;
-
-       return timelib_daynr_from_weeknr_ex(iy, iw, id, &dummy_iso_year);
-}
-
 void timelib_date_from_isodate(timelib_sll iy, timelib_sll iw, timelib_sll id, timelib_sll *y, timelib_sll *m, timelib_sll *d)
 {
-       timelib_sll daynr = timelib_daynr_from_weeknr_ex(iy, iw, id, y) + 1;
+       timelib_sll daynr = timelib_daynr_from_weeknr(iy, iw, id) + 1;
        int *table;
 
        *m = 0;
-
-       if (daynr <= 0) {
-               *y += 1;
-       }
+        *y = iy;
 
        if (timelib_is_leap(*y)) {
                table = ml_table_leap;

 
The remaining work is to validate that this fix works as expected with the appropriate tests, and to get it upstreamed to https://github.com/derickr/timelib.

Comment by Danny Hatcher (Inactive) [ 29/Mar/19 ]

Hello Sam,

Thank you for the report. I've forwarded this on to our query team to determine whether this was intentionally not done for ISO or is a bug.

Danny

Generated at Thu Feb 08 04:54:48 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.