Chakracore: Date parsing of mm/dd/yy incompatible with other engines

[dilijev]

All engines (at least for machines on en-us locales which I tested on, not sure if there's any difference between locales for non-Intl Date parsing across any engines) parse ##/##/## as mm/dd/yy (the en-us short date convention).

If you try to write a date as dd/mm/yy using large values (not ambiguous), it still doesn't switch parsing over to dd/mm/yy (which is a good thing as logic for that would be very confusing), but every engine except for ChakraCore treats the first number as a month out of range and parses the date as NaN (out of range) which toString's to "Invalid Date".

ChakraCore apparently just adds that number of months to the date, changing 31/12/50 into 7/12/52.

My reading of the spec is that this date should be parsed as NaN:

https://tc39.github.io/ecma262/#sec-date-year-month-date-hours-minutes-seconds-ms

• Let finalDate be MakeDate(MakeDay(yr, m, dt), MakeTime(h, min, s, milli)).

https://tc39.github.io/ecma262/#sec-makeday

• Find a value t such that YearFromTime(t) is ym and MonthFromTime(t) is mn and DateFromTime(t) is 1; but if this is not possible (because some argument is out of range), return NaN.
• See https://tc39.github.io/ecma262/#eqn-MonthFromTime ( => not possible because out of range)

>eshost --tags web -itse "new Date('12/31/50')"
## Source
print(new Date('12/31/50'))

┌─────┬───────────────────────────────────────────────────────────┐
│ ch  │ Sun Dec 31 1950 00:00:00 GMT-0800 (Pacific Standard Time) │
│ d8  │                                                           │
│ jsc │                                                           │
│ sm  │                                                           │
└─────┴───────────────────────────────────────────────────────────┘

eshost --tags web -itse "new Date('31/12/50')"
## Source
print(new Date('31/12/50'))

┌─────┬───────────────────────────────────────────────────────────┐
│ d8  │ Invalid Date                                              │
│ jsc │                                                           │
│ sm  │                                                           │
├─────┼───────────────────────────────────────────────────────────┤
│ ch  │ Sat Jul 12 1952 00:00:00 GMT-0700 (Pacific Daylight Time) │
└─────┴───────────────────────────────────────────────────────────┘

>eshost --tags web -itse "new Date('31/12/50').valueOf()"
## Source
print(new Date('31/12/50').valueOf())

┌─────┬───────────────┐
│ d8  │ NaN           │
│ jsc │               │
│ sm  │               │
├─────┼───────────────┤
│ ch  │ -551379600000 │
└─────┴───────────────┘

/cc @xiaoyinl @bterlson

[mattzeunert]

I'm taking a look at fixing this.

[mattzeunert]

I've got a change that makes ChakraCore match V8's behavior.

V8 does a simple < 31 check for dates, whereas for ISO dates SpiderMonkey and JavaScriptCore check that the date actually exists in the month. I'll try looking at the spec to see which one is the more correct behavior.

ch-mod is the ChakraCore build with the change.

eshost -itse "new Date('2000-13-01')"; eshost -itse "new Date('02/39/2000')"; eshost -itse "new Date('02/30/2000')"; eshost -itse "new Date('2000-02-39')"; eshost -itse "new Date('2000-02-30')";
## Source
print(new Date('2000-13-01'))

┌─────────┬─────────────────────────────────────────┐
│ ch      │ Mon Jan 01 2001 00:00:00 GMT+0800 (CST) │
├─────────┼─────────────────────────────────────────┤
│ ch-mod  │ Invalid Date                            │
│ d8      │                                         │
│ jsc     │                                         │
│ jsshell │                                         │
└─────────┴─────────────────────────────────────────┘
## Source
print(new Date('02/39/2000'))

┌─────────┬─────────────────────────────────────────┐
│ ch-mod  │ Invalid Date                            │
│ d8      │                                         │
│ jsc     │                                         │
│ jsshell │                                         │
├─────────┼─────────────────────────────────────────┤
│ ch      │ Fri Mar 10 2000 00:00:00 GMT+0800 (CST) │
└─────────┴─────────────────────────────────────────┘
## Source
print(new Date('02/30/2000'))

┌─────────┬─────────────────────────────────────────┐
│ ch      │ Wed Mar 01 2000 00:00:00 GMT+0800 (CST) │
│ ch-mod  │                                         │
│ d8      │                                         │
│ jsc     │                                         │
│ jsshell │                                         │
└─────────┴─────────────────────────────────────────┘
## Source
print(new Date('2000-02-39'))

┌─────────┬─────────────────────────────────────────┐
│ ch      │ Fri Mar 10 2000 00:00:00 GMT+0800 (CST) │
├─────────┼─────────────────────────────────────────┤
│ ch-mod  │ Invalid Date                            │
│ d8      │                                         │
│ jsc     │                                         │
│ jsshell │                                         │
└─────────┴─────────────────────────────────────────┘
## Source
print(new Date('2000-02-30'))

┌─────────┬─────────────────────────────────────────┐
│ ch      │ Wed Mar 01 2000 08:00:00 GMT+0800 (CST) │
│ ch-mod  │                                         │
│ d8      │                                         │
├─────────┼─────────────────────────────────────────┤
│ jsc     │ Invalid Date                            │
│ jsshell │                                         │
└─────────┴─────────────────────────────────────────┘

[mattzeunert]

My understanding:

• new Date should behave exactly like the Date.parse method when passing in a string
• Date.parse does not have a step by step description
• Dates like 12/31/50 are not mentioned in the spec, and are an implementation-specific date format
• ISO strings are mentioned in the spec

ISO strings

They follow the Date Time String Format, which merely specifies the day as "the day of the month from 01 to 31."

From that it doesn't sound like the 31st of February is clearly out-of-bounds.

User expectation

As a user I would expect any date that's not in the calendar to be invalid. I would also not expect a parsing difference between mm/dd/yy and ISO dates.

Compatibility

JavaScriptCore and SpiderMonkey are more strict with ISO dates than with mm/dd/yy ones.

No engine validates the 29-31 range for the mm/dd/yy format.

What to do

I'm not sure. Maybe this?

• 31th of Feb invalid for ISO dates -> better from user standpoint, same as JavaScriptCore and SpiderMonkey
• 31st of Feb valid for mm/dd/yy dates -> because that's what every other engine does

[dilijev]

Hey @mattzeunert, thanks for the thorough investigation so far. Your understanding of the spec is correct AFAICT, especially w.r.t. to mm/dd/yy format being implementation-defined. Thus our current implementation is not "wrong" per the spec, but is wrong from a perspective of user expectation, most importantly in that we differ from the other engines.

"Dates not in the calendar" is a more complicated issue because of leap years and changes to the calendar over time. Right now, I'm guessing we just treat any dd value from 1-31 as valid, even if that month would never have that day. In a "more correct" world (without having the dd be sensitive to the year), we could have the correctness of dd be w.r.t. to the month only, so that Feb can have up to 29 days (and some months only allowed up to 30), even though in most years Feb would only have 28 days.

Since any such checks complicate the logic and add some performance cost, and if we over-correct beyond other engines we would again be web-incompatible, I'd recommend fixing only enough to bring us into compatibility with other engines.

ISO date string parsing is a separate issue so I think we should open another issue to discuss what to do there and keep the PR for the issue about making mm/dd/yy format align with other engines.

[dilijev]

For "not in the calendar" dates (sensitive to year and month), jsc and sm/jsshell do the "right thing" from a real calendar perspective. Otherwise for days up to 31, we do the same thing as d8. Since this behavior is 50/50 split there's no compelling reason to change it, especially with the (albeit minor) perf impact we'd expect.

However, for days above 31 and months above 12, we do behave in a way users would certainly not expect and differ from the others.

The original description shows the behavior for months larger than 12. Here's a test for days beyond the end of a month:

>eshost --tags web -its E:\work\date\issue4329\test.js

## Source
// non-leap year
print(new Date('2005-02-28'))
print(new Date('2005-02-29')) // not a real date
print(new Date('2005-02-30'))
print(new Date('2005-02-31'))
print(new Date('2005-02-32'))

// leap year
print(new Date('2004-02-28'))
print(new Date('2004-02-29')) // real date
print(new Date('2004-02-30')) // not a real date
print(new Date('2004-02-31'))
print(new Date('2004-02-32'))


┌─────┬───────────────────────────────────────────────────────────┐
│ jsc │ Sun Feb 27 2005 16:00:00 GMT-0800 (Pacific Standard Time) │
│ sm  │ Invalid Date                                              │
│     │ Invalid Date                                              │
│     │ Invalid Date                                              │
│     │ Invalid Date                                              │
│     │ Fri Feb 27 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Sat Feb 28 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Invalid Date                                              │
│     │ Invalid Date                                              │
│     │ Invalid Date                                              │
├─────┼───────────────────────────────────────────────────────────┤
│ d8  │ Sun Feb 27 2005 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Mon Feb 28 2005 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Tue Mar 01 2005 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Wed Mar 02 2005 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Invalid Date                                              │
│     │ Fri Feb 27 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Sat Feb 28 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Sun Feb 29 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Mon Mar 01 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Invalid Date                                              │
├─────┼───────────────────────────────────────────────────────────┤
│ ch  │ Sun Feb 27 2005 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Mon Feb 28 2005 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Tue Mar 01 2005 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Wed Mar 02 2005 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Fri Mar 04 2005 00:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Fri Feb 27 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Sat Feb 28 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Sun Feb 29 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Mon Mar 01 2004 16:00:00 GMT-0800 (Pacific Standard Time) │
│     │ Wed Mar 03 2004 00:00:00 GMT-0800 (Pacific Standard Time) │
└─────┴───────────────────────────────────────────────────────────┘

If you change aligns with Chrome on days > 31 and months > 12, I'd say we take that change. Feel free to open a PR.

[mattzeunert]

Cool, I'll open a PR.

Actually the ISO date logic fails as the date/month are invalid (>12/31). But if the ISO date logic fails the parsing code falls back to the implementation specific date parsing logic, which does support invalid dates that look ISO-like (e.g. 2000-01-40).