react-native-windows icon indicating copy to clipboard operation
react-native-windows copied to clipboard

Hermes crashes on formatting out-of-range date/time inputs

Open mganandraj opened this issue 3 years ago • 3 comments

  1. Set useHermes to 'true'
  2. Call toLocaleString() on Date object for times after 03:14:07 UTC on 19 January 2038

Expected Results

Prints the formatted date/time

Actual Result

App crashes

Without an Intl implementation, hermes uses our NLS based implementation of unicode stubs defined in https://github.com/microsoft/hermes-windows/blob/fa9081d1a7f0b8b502c0db6e2a7b429502b33c60/lib/Platform/Unicode/PlatformUnicodeWinGlob.cpp#L324 which is buggy.

The code was originally copied/adapted from ChakraCore codebase here: https://github.com/chakra-core/ChakraCore/blob/13b5403dcc1270c1d4cdb075d1452ee17202c997/lib/Runtime/Library/DateImplementation.cpp#L419 https://github.com/chakra-core/ChakraCore/blob/13b5403dcc1270c1d4cdb075d1452ee17202c997/lib/Common/Common/DateUtilities.cpp#L234

mganandraj avatar Sep 17 '21 21:09 mganandraj

So the issue is with the intl implementation that was copied from Chakra, and we should be calling the native Windows APIs. But that introduces a support difference in Windows versions if those APIs are newer, which means both implementations have to stick around for awhile.

@mganandraj Is there an issue on the hermes-windows repo on this?

chrisglein avatar Sep 20 '21 18:09 chrisglein

@chrisglein The implementation that we copied from Chakra also calls into WIndows API for the locale specific date formatting.. but there is a preprocessing to massage the 'time since epoch' to 'SYSTEMTIME' which is the buggy piece of code .. Then we call into Windows NLS api ..

The proposed fix is to call Windows ICU APIs ..

Hence, the stable behavior across versions is possible only by packaging the intl libraries and data with the app .. which is usually many MBs in size (A full ICU library is ~30MB) ..

For internal customers, we could allow hooking up custom implementation ..

For open source version, linking again the Windows packaged icu.dll is the most reasonable choice. For Android Intl implementation, we made the same decision to call into ICU APIs exported by the operating system, at the expense of some support difference across Android versions.

We don't have a bug filed in hermes-windows repo. We don't yet have a process to manage bugs there :(

mganandraj avatar Sep 21 '21 18:09 mganandraj

Hi, I'm still running into this issue. I'm currently using Luxon to format a DateTime object I have and then calling toLocaleString() and this crashes on windows. Is there a workaround that is documented somewhere? I see the comment about icu.dll but that was from 2021, is that still a valid option?

LyDawei avatar Feb 21 '24 22:02 LyDawei