deps: backport ICU-20558 to fix Intl crasher #27415

srl295 · 2019-04-25T16:36:54Z

Floating patch for ICU 63.x and 64.x
fixing crash in Intl when ICU data not found.

Background:

ICU-13778 (landed in ICU 63.1) fixed a bug but
added a regression.
a recent v8 land in Node v12 (which one?) exposes
this bug to cause a crash when ICU data is not found.

ICU Bug: https://unicode-org.atlassian.net/browse/ICU-20558
Backport of: unicode-org/icu#632
(Commit not landed yet in ICU)
Fixes: #27379

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

nodejs-github-bot · 2019-04-25T16:36:57Z

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3366

srl295 · 2019-04-25T16:42:43Z

fyi @jefgen

srl295 · 2019-04-25T16:43:57Z

Notes:

the '64' patch is direct from the PR
the '63' patch is https://github.com/unicode-org/icu/pull/632.diff applied to the dtptngen.cpp file from the node.js embedded small-icu (which is version 63)

targos

RSLGTM

refack

RSLGTM

refack · 2019-04-25T19:19:11Z

Added a regression test for this. @srl295 obviously feel free to push it out or change it.

srl295

nice test, thanks @refack !

- Floating patch for ICU 63.x and 64.x - fixing crash in Intl when ICU data not found. - Regression test from refack included. Background: - ICU-13778 (landed in ICU 63.1) fixed a bug but added a regression. - a recent v8 land in Node v12 (which one?) exposes this bug to cause a crash when ICU data is not found. ICU Bug: https://unicode-org.atlassian.net/browse/ICU-20558 Backport of: unicode-org/icu#632 (Commit not landed yet in ICU) Fixes: nodejs#27379 Co-authored-by: Refael Ackermann <refack@gmail.com>

srl295 · 2019-04-25T19:40:18Z

Rebased and squashed.

nodejs-github-bot · 2019-04-25T19:40:59Z

CI: https://ci.nodejs.org/job/node-test-pull-request/22720/

srl295 · 2019-04-25T20:46:46Z

@refack do you want me to squash again?

refack · 2019-04-25T21:03:42Z

@refack do you want me to squash again?

No rush, we can squash while landing (I could also force push myself). I'm waiting for CI job 22720 to finish so we could start a resume job for the failing platforms.

nodejs-github-bot · 2019-04-25T21:15:50Z

CI: https://ci.nodejs.org/job/node-test-pull-request/22722/

srl295 · 2019-04-25T21:48:18Z

@refack want to take this over ?

refack · 2019-04-25T22:16:28Z

@refack want to take this over ?

Yeah, I'm on it.

nodejs-github-bot · 2019-04-25T22:19:00Z

CI: https://ci.nodejs.org/job/node-test-pull-request/22723/

refack · 2019-04-25T22:20:20Z

@here now that #27361 landed, we probably should fast-track this since it solves #27379 which is kind of harsh.

Please 👍 if you concur.

ryzokuken

RSLGTM

targos · 2019-04-27T09:40:54Z

Landed in 617c55e

- Floating patch for ICU 63.x and 64.x - fixing crash in Intl when ICU data not found. - Regression test from refack included. Background: - ICU-13778 (landed in ICU 63.1) fixed a bug but added a regression. - a recent v8 land in Node v12 (which one?) exposes this bug to cause a crash when ICU data is not found. ICU Bug: https://unicode-org.atlassian.net/browse/ICU-20558 Backport of: unicode-org/icu#632 Fixes: nodejs#27379 Co-authored-by: Refael Ackermann <refack@gmail.com> PR-URL: nodejs#27415 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>

- Floating patch for ICU 63.x and 64.x - fixing crash in Intl when ICU data not found. - Regression test from refack included. Background: - ICU-13778 (landed in ICU 63.1) fixed a bug but added a regression. - a recent v8 land in Node v12 (which one?) exposes this bug to cause a crash when ICU data is not found. ICU Bug: https://unicode-org.atlassian.net/browse/ICU-20558 Backport of: unicode-org/icu#632 Fixes: #27379 Co-authored-by: Refael Ackermann <refack@gmail.com> PR-URL: #27415 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>

Notable changes: * intl: * Update ICU to 64.2. This adds support for Japanese Era (Reiwa). #27361 * Fixes a bug in ICU that affected Node.js 12.0.0 in the case where `new Date().toLocaleString()` was called with a non-default locale. #27415 * C++ API: * Added an `Environment` overload of `EmitAsyncDestroy`. #27255 PR-URL: TODO

Notable changes: * intl: * Update ICU to 64.2. This adds support for Japanese Era (Reiwa). #27361 * Fixes a bug in ICU that affected Node.js 12.0.0 in the case where `new Date().toLocaleString()` was called with a non-default locale. #27415 * C++ API: * Added an overload of `EmitAsyncDestroy` that can be used during garbage collection. #27255 PR-URL: #27440

- Floating patch for ICU 63.x and 64.x - fixing crash in Intl when ICU data not found. - Regression test from refack included. Background: - ICU-13778 (landed in ICU 63.1) fixed a bug but added a regression. - a recent v8 land in Node v12 (which one?) exposes this bug to cause a crash when ICU data is not found. ICU Bug: https://unicode-org.atlassian.net/browse/ICU-20558 Backport of: unicode-org/icu#632 Fixes: #27379 Co-authored-by: Refael Ackermann <refack@gmail.com> PR-URL: #27415 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>

srl295 added the i18n-api Issues and PRs related to the i18n implementation. label Apr 25, 2019

srl295 requested review from refack, targos and ryzokuken April 25, 2019 16:36

srl295 self-assigned this Apr 25, 2019

nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Apr 25, 2019

targos approved these changes Apr 25, 2019

View reviewed changes

refack approved these changes Apr 25, 2019

View reviewed changes

srl295 marked this pull request as ready for review April 25, 2019 19:12

srl295 commented Apr 25, 2019

View reviewed changes

srl295 force-pushed the patch-ICU-20558 branch from 5400049 to c369ef0 Compare April 25, 2019 19:39

fixup! Some platforms need the full env

d93152d

refack force-pushed the patch-ICU-20558 branch from 5912d0c to d93152d Compare April 25, 2019 20:54

cjihrig approved these changes Apr 25, 2019

View reviewed changes

refack assigned refack and unassigned srl295 Apr 25, 2019

refack added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 25, 2019

jasnell approved these changes Apr 26, 2019

View reviewed changes

ryzokuken approved these changes Apr 26, 2019

View reviewed changes

This was referenced Apr 26, 2019

Node.js crash with a fatal error on Date#toLocaleString #27379

Closed

deps: backport ICU-20575 to fix err/crasher #27435

Closed

targos closed this Apr 27, 2019

targos mentioned this pull request Apr 27, 2019

v12.1.0 proposal #27440

Merged

targos mentioned this pull request May 1, 2019

v10.16.0 proposal #27514

Merged

elvisbegovic mentioned this pull request May 7, 2019

ng xi18n fails with Node 12.0 (FIXED with Node.js 12.1+) angular/angular-cli#14351

Closed

refack removed their assignment Jun 6, 2019

deepak1556 mentioned this pull request Jan 30, 2020

fix: crash with icu when input locale is invalid electron/electron#21969

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: backport ICU-20558 to fix Intl crasher #27415

deps: backport ICU-20558 to fix Intl crasher #27415

srl295 commented Apr 25, 2019 •

edited

Loading

nodejs-github-bot commented Apr 25, 2019

srl295 commented Apr 25, 2019

srl295 commented Apr 25, 2019 •

edited

Loading

targos left a comment

refack left a comment

refack commented Apr 25, 2019

srl295 left a comment

srl295 commented Apr 25, 2019

nodejs-github-bot commented Apr 25, 2019

srl295 commented Apr 25, 2019

refack commented Apr 25, 2019 •

edited

Loading

nodejs-github-bot commented Apr 25, 2019

srl295 commented Apr 25, 2019 via email •

edited

Loading

refack commented Apr 25, 2019

nodejs-github-bot commented Apr 25, 2019

refack commented Apr 25, 2019

ryzokuken left a comment

targos commented Apr 27, 2019 •

edited

Loading

deps: backport ICU-20558 to fix Intl crasher #27415

deps: backport ICU-20558 to fix Intl crasher #27415

Conversation

srl295 commented Apr 25, 2019 • edited Loading

Checklist

nodejs-github-bot commented Apr 25, 2019

srl295 commented Apr 25, 2019

srl295 commented Apr 25, 2019 • edited Loading

targos left a comment

Choose a reason for hiding this comment

refack left a comment

Choose a reason for hiding this comment

refack commented Apr 25, 2019

srl295 left a comment

Choose a reason for hiding this comment

srl295 commented Apr 25, 2019

nodejs-github-bot commented Apr 25, 2019

srl295 commented Apr 25, 2019

refack commented Apr 25, 2019 • edited Loading

nodejs-github-bot commented Apr 25, 2019

srl295 commented Apr 25, 2019 via email • edited Loading

refack commented Apr 25, 2019

nodejs-github-bot commented Apr 25, 2019

refack commented Apr 25, 2019

ryzokuken left a comment

Choose a reason for hiding this comment

targos commented Apr 27, 2019 • edited Loading

srl295 commented Apr 25, 2019 •

edited

Loading

srl295 commented Apr 25, 2019 •

edited

Loading

refack commented Apr 25, 2019 •

edited

Loading

srl295 commented Apr 25, 2019 via email •

edited

Loading

targos commented Apr 27, 2019 •

edited

Loading