Bug 150386 - Safari 8 and 9 have a Date bug with the "milliseconds" param.
Summary: Safari 8 and 9 have a Date bug with the "milliseconds" param.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Critical
Assignee: Nobody
URL: https://github.com/es-shims/es5-shim/...
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-21 00:31 PDT by Jordan Harband
Modified: 2015-10-25 18:21 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jordan Harband 2015-10-21 00:31:02 PDT
See https://github.com/es-shims/es5-shim/issues/329

`new Date(1970, 0, 1,0, 0, 0, 2147483647);` produces a valid Date object, `new Date(1970, 0, 1,0, 0, 0, 2147483648);` does not. 2147483648 is, of course, `Math.pow(2, 31)`.

So, the issue seems to be that Safari 8 and 9 - alone amongst the browsers, it's worth noting - is treating the millisecond parameter to `Date` as a signed 32-bit integer - when in fact, it should allow all integer values up to `Number.MAX_SAFE_INTEGER`: ie, a 64-bit signed integer, also known as a normal JS number value.

This does not appear to be an issue for the other parameters to the 7-arg form of the Date constructor, but when fixing this, please verify that all JS Number values work as expected when passed in as any of the 7 arguments.
Comment 1 Geoffrey Garen 2015-10-21 11:00:46 PDT
Note that Number.MAX_SAFE_INTEGER is 53 bits, not 64, since only 53 bits of precision fit into a double.
Comment 2 Jordan Harband 2015-10-21 13:59:13 PDT
Ah, right - in that case, despite not all integers being safe in [53,64] bits, it should still support any 64-bit signed integer.
Comment 3 Darin Adler 2015-10-24 15:13:41 PDT
The Date constructor code already treats the millisecond parameter as a double rather than an integer to avoid the 32-bit integer clamping problems. The problem is simply in the input validation. Here is the code validating the seventh argument in the millisecondsFromComponents function in DateConstructor.cpp:

!std::isfinite(doubleArguments[6]) || (doubleArguments[6] > INT_MAX) || (doubleArguments[6] < INT_MIN)

As you can see, it’s incorrectly comparing with 32 bit integer minimums and maximums; that’s where the bug comes from. (The code has no business using INT_MIN and INT_MAX for 32-bit integer range checking anyway.) The fix will be to write a corrected version of this range checking or possibly to remove it entirely. Someone just needs to figure out what the limits need to be.

The “any 64-bit signed integer” idea isn’t compatible with JavaScript’s usual type model; integer values are typically treated as a subset of double values rather than a distinct type with values not representable as doubles. Seems highly unlikely that is required, and equally unlikely that it is supported in other JavaScript engines. If this does turn out to be required, there will be substantial design and implementation work required to resolve this and it won’t just be a simple bug fix.

To make it easy to fix the bug we need a test case that covers the values as the limits of what are allowed for Date, and it would be worthwhile to both try the test cases on other browsers and perhaps make it part of an ES6 test suite.
Comment 4 Geoffrey Garen 2015-10-25 18:21:20 PDT
I tried just removing the INT_MAX and INT_MIN comparisons and it wasn't quite enough because some of our lower level date code uses int internally, and the upgrade to double is non-trivial.