From 2273f341162d25aca76717e24b422a2a7561619a Mon Sep 17 00:00:00 2001 From: Frank Tang Date: Thu, 8 Jul 2021 17:19:16 -0700 Subject: [PATCH] [Intl] throw instead of DCHECK while long locale This is a temp fix to throw instead of DCHECK in debug build. The correct fix depends on the landing of https://github.com/unicode-org/icu/pull/1762 Once that land I will cherrypick into chrome to fix the function correctly. But the current (before this CL) behavior is not harmful in release build. It basically does not do the max nor min just return itself. Bug: chromium:1224869 Change-Id: Iebce2ab0a5ce047e83e8fce05db8290212e64509 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3017300 Reviewed-by: Shu-yu Guo Reviewed-by: Jakob Kummerow Commit-Queue: Frank Tang Cr-Commit-Position: refs/heads/master@{#76047} --- src/objects/js-locale.cc | 18 ++++++++++++++---- test/intl/regress-1224869.js | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 test/intl/regress-1224869.js diff --git a/src/objects/js-locale.cc b/src/objects/js-locale.cc index 98ce154336..6c3f284e71 100644 --- a/src/objects/js-locale.cc +++ b/src/objects/js-locale.cc @@ -425,8 +425,13 @@ MaybeHandle JSLocale::Maximize(Isolate* isolate, // Base name is not changed result = source; } - DCHECK(U_SUCCESS(status)); - DCHECK(!result.isBogus()); + if (U_FAILURE(status) || result.isBogus()) { + // Due to https://unicode-org.atlassian.net/browse/ICU-21639 + // Valid but super long locale will fail. Just throw here for now. + THROW_NEW_ERROR(isolate, + NewRangeError(MessageTemplate::kLocaleBadParameters), + JSLocale); + } return Construct(isolate, result); } @@ -455,8 +460,13 @@ MaybeHandle JSLocale::Minimize(Isolate* isolate, // Base name is not changed result = source; } - DCHECK(U_SUCCESS(status)); - DCHECK(!result.isBogus()); + if (U_FAILURE(status) || result.isBogus()) { + // Due to https://unicode-org.atlassian.net/browse/ICU-21639 + // Valid but super long locale will fail. Just throw here for now. + THROW_NEW_ERROR(isolate, + NewRangeError(MessageTemplate::kLocaleBadParameters), + JSLocale); + } return Construct(isolate, result); } diff --git a/test/intl/regress-1224869.js b/test/intl/regress-1224869.js new file mode 100644 index 0000000000..2d30ac5dc5 --- /dev/null +++ b/test/intl/regress-1224869.js @@ -0,0 +1,18 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +var l = new Intl.Locale("en-US-4106-4104-4102-4100-4098-4096-4094-4092-4090-4088-4086-4084-4082-4080-4078-4076-4074-4072-4070-4068-4066-4064-4062-4060-4058-4056-4054-4052-4050-4048-4049"); + +// Ensure won't DCHECK in debug build +try { + l.maximize(); +} catch(e) { +} + +l2 = new Intl.Locale("en-US-4106-4104-4102-4100-4098-4096-4094-4092-4090-4088-4086-4084-4082-4080-4078-4076-4074-4072-4070-4068-4066-4064-4062-4060-4058-4056-4054-4052-4050-4048-4049"); +// Ensure won't DCHECK in debug build +try { + l2.minimize(); +} catch(e) { +}