SR-7011: Fatal Error when Using Calendar.current.dateComponents(_:from:to:) #1658
Conversation
…m:to:) - _CFCalendarGetComponentDifferenceV() doesn't support a component character of 'l' (leap year) as it make no sense for the difference between two dates. - Dont add 'l' to the component string or parse the result into the .isLeapMonth property when called from NSCalendar.components(_:from:to:options)
|
@swift-ci please test |
|
@itaiferber probably would be the better one to review this. |
TestFoundation/TestCalendar.swift
Outdated
|
|
||
| // 1971-06-21 | ||
| let date2 = Date(timeIntervalSince1970: 46310400) | ||
| let difference = Calendar.current.dateComponents([.month, .year, .day], from: date1, to: date2) |
There was a problem hiding this comment.
for leap months you probably need the Chinese calendar iirc.
There was a problem hiding this comment.
On Darwin we actually almost always add "is leap month" to DateComponents in output, if that's what you're referring to. (Leap months are also applicable in the Hebrew calendar, and otherwise.)
There was a problem hiding this comment.
(But yes, in this case, on Darwin we don't add "is leap month" here.)
| @@ -980,8 +985,13 @@ Boolean _CFCalendarGetComponentDifferenceV(CFCalendarRef calendar, CFAbsoluteTim | |||
| divide = alwaysDivide; | |||
| } | |||
| } | |||
| if (count < 1) { | |||
There was a problem hiding this comment.
Should this be moved further down (right after the --) so we don't go through the loop an additional time and do work just to discover we can't get the difference?
There was a problem hiding this comment.
It should probably go at the top of the while loop in case count starts at 0. I will fix that
|
I know the unit tests were non-existent before, but it'd be nice to expand them further if we can before we merge. A variety of dates (future and past relative to one another) and a variety of components would be great! |
- _CFCalendarGetComponentDifferenceV: Add validation of datetime format character and bounds check multiple_table array lookup. - Allow -1 as a component value. - Reorder datetime fields from largest to smallest when creating datetime format and constructing components from response. - Add tests validated against Darwin's Foundation.
|
@swift-ci please test |
|
@itaiferber Ive added some extra tests and validated them against Darwin. There are probably some more cases to catch but this should cover a reasonable amount and does add some extra validation into the CF function. |
|
@swift-ci please test and merge |
- Nanoseconds were originally disable in apple#1658 to fix SR-7011, however the CF/NSCalendar rewrite in apple#1755 fixed them to work correctly so they can now be re-enabled.
- Nanoseconds were originally disable in apple#1658 to fix SR-7011, however the CF/NSCalendar rewrite in apple#1755 fixed them to work correctly so they can now be re-enabled. (cherry picked from commit d8f09b6)
_CFCalendarGetComponentDifferenceV() doesn't support a component
character of 'l' (leap year) as it make no sense for the difference
between two dates.
Dont add 'l' to the component string or parse the result into the
.isLeapMonth property when called from
NSCalendar.components(_:from:to:options)