[Intl] Fix CHECK fail in Intl::ToLanguageTag()

Make the function return Maybe<std::string> to propagate error.

Bug: chromium:917151
Change-Id: I4330b0c54c122b1eddd3b37e21e0f387cb5d803a
Reviewed-on: https://chromium-review.googlesource.com/c/1392205
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Frank Tang <ftang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58535}

src/objects/intl-objects.cc

@@ -499,24 +499,13 @@ bool RemoveLocaleScriptTag(const std::string& icu_locale,
 std::set<std::string> Intl::BuildLocaleSet(
     const icu::Locale* icu_available_locales, int32_t count) {
   std::set<std::string> locales;
-  UErrorCode error = U_ZERO_ERROR;
-  char result[ULOC_FULLNAME_CAPACITY];
-
   for (int32_t i = 0; i < count; ++i) {
-    const char* icu_name = icu_available_locales[i].getName();
-
-    error = U_ZERO_ERROR;
-    // No need to force strict BCP47 rules.
-    uloc_toLanguageTag(icu_name, result, ULOC_FULLNAME_CAPACITY, FALSE, &error);
-    if (U_FAILURE(error) || error == U_STRING_NOT_TERMINATED_WARNING) {
-      // This shouldn't happen, but lets not break the user.
-      continue;
-    }
-    std::string locale(result);
+    std::string locale =
+        Intl::ToLanguageTag(icu_available_locales[i]).FromJust();
     locales.insert(locale);
 
     std::string shortened_locale;
-    if (RemoveLocaleScriptTag(icu_name, &shortened_locale)) {
+    if (RemoveLocaleScriptTag(locale, &shortened_locale)) {
       std::replace(shortened_locale.begin(), shortened_locale.end(), '_', '-');
       locales.insert(shortened_locale);
     }
@@ -525,9 +514,12 @@ std::set<std::string> Intl::BuildLocaleSet(
   return locales;
 }
 
-std::string Intl::ToLanguageTag(const icu::Locale& locale) {
+Maybe<std::string> Intl::ToLanguageTag(const icu::Locale& locale) {
   UErrorCode status = U_ZERO_ERROR;
   std::string res = locale.toLanguageTag<std::string>(status);
+  if (U_FAILURE(status)) {
+    return Nothing<std::string>();
+  }
   CHECK(U_SUCCESS(status));
 
   // Hack to remove -true from unicode extensions
@@ -543,7 +535,7 @@ std::string Intl::ToLanguageTag(const icu::Locale& locale) {
       res.erase(sep_true, 5 /* strlen(kSepTrue) == 5 */);
     }
   }
-  return res;
+  return Just(res);
 }
 
 namespace {
@@ -555,13 +547,10 @@ std::string DefaultLocale(Isolate* isolate) {
       isolate->set_default_locale("en-US");
     } else {
       // Set the locale
-      char result[ULOC_FULLNAME_CAPACITY];
-      UErrorCode status = U_ZERO_ERROR;
-      int32_t length =
-          uloc_toLanguageTag(default_locale.getName(), result,
-                             ULOC_FULLNAME_CAPACITY, FALSE, &status);
       isolate->set_default_locale(
-          U_SUCCESS(status) ? std::string(result, length) : "und");
+          default_locale.isBogus()
+              ? "und"
+              : Intl::ToLanguageTag(default_locale).FromJust());
     }
     DCHECK(!isolate->default_locale().empty());
   }
@@ -768,36 +757,26 @@ Maybe<std::string> Intl::CanonicalizeLanguageTag(Isolate* isolate,
   // handle long locale names better. See
   // https://unicode-org.atlassian.net/browse/ICU-13417
   UErrorCode error = U_ZERO_ERROR;
-  char icu_result[ULOC_FULLNAME_CAPACITY];
   // uloc_forLanguageTag checks the structrual validity. If the input BCP47
   // language tag is parsed all the way to the end, it indicates that the input
   // is structurally valid. Due to a couple of bugs, we can't use it
   // without Chromium patches or ICU 62 or earlier.
-  int parsed_length;
-  uloc_forLanguageTag(locale.c_str(), icu_result, ULOC_FULLNAME_CAPACITY,
-                      &parsed_length, &error);
-  if (U_FAILURE(error) ||
-      static_cast<size_t>(parsed_length) < locale.length() ||
-      error == U_STRING_NOT_TERMINATED_WARNING) {
+  icu::Locale icu_locale = icu::Locale::forLanguageTag(locale.c_str(), error);
+  if (U_FAILURE(error) || icu_locale.isBogus()) {
+    THROW_NEW_ERROR_RETURN_VALUE(
+        isolate,
+        NewRangeError(MessageTemplate::kInvalidLanguageTag, locale_str),
+        Nothing<std::string>());
+  }
+  Maybe<std::string> maybe_to_language_tag = Intl::ToLanguageTag(icu_locale);
+  if (maybe_to_language_tag.IsNothing()) {
     THROW_NEW_ERROR_RETURN_VALUE(
         isolate,
         NewRangeError(MessageTemplate::kInvalidLanguageTag, locale_str),
         Nothing<std::string>());
   }
 
-  // Force strict BCP47 rules.
-  char result[ULOC_FULLNAME_CAPACITY];
-  int32_t result_len = uloc_toLanguageTag(icu_result, result,
-                                          ULOC_FULLNAME_CAPACITY, TRUE, &error);
-
-  if (U_FAILURE(error)) {
-    THROW_NEW_ERROR_RETURN_VALUE(
-        isolate,
-        NewRangeError(MessageTemplate::kInvalidLanguageTag, locale_str),
-        Nothing<std::string>());
-  }
-
-  return Just(std::string(result, result_len));
+  return maybe_to_language_tag;
 }
 
 Maybe<std::vector<std::string>> Intl::CanonicalizeLocaleList(
@@ -1591,11 +1570,7 @@ Intl::ResolvedLocale Intl::ResolveLocale(
   std::map<std::string, std::string> extensions =
       LookupAndValidateUnicodeExtensions(&icu_locale, relevant_extension_keys);
 
-  char canonicalized_locale[ULOC_FULLNAME_CAPACITY];
-  UErrorCode status = U_ZERO_ERROR;
-  uloc_toLanguageTag(icu_locale.getName(), canonicalized_locale,
-                     ULOC_FULLNAME_CAPACITY, true, &status);
-  CHECK(U_SUCCESS(status));
+  std::string canonicalized_locale = Intl::ToLanguageTag(icu_locale).FromJust();
 
   // TODO(gsathya): Remove privateuse subtags from extensions.
 

src/objects/intl-objects.h

@@ -51,7 +51,7 @@ class Intl {
   static std::set<std::string> BuildLocaleSet(
       const icu::Locale* icu_available_locales, int32_t count);
 
-  static std::string ToLanguageTag(const icu::Locale& locale);
+  static Maybe<std::string> ToLanguageTag(const icu::Locale& locale);
 
   // Get the name of the numbering system from locale.
   // ICU doesn't expose numbering system in any way, so we have to assume that

src/objects/js-date-time-format.cc

@@ -285,7 +285,9 @@ MaybeHandle<JSObject> JSDateTimeFormat::ResolvedOptions(
   CHECK(!date_time_format->icu_locale().is_null());
   CHECK_NOT_NULL(date_time_format->icu_locale()->raw());
   icu::Locale* icu_locale = date_time_format->icu_locale()->raw();
-  std::string locale_str = Intl::ToLanguageTag(*icu_locale);
+  Maybe<std::string> maybe_locale_str = Intl::ToLanguageTag(*icu_locale);
+  MAYBE_RETURN(maybe_locale_str, MaybeHandle<JSObject>());
+  std::string locale_str = maybe_locale_str.FromJust();
   Handle<String> locale =
       factory->NewStringFromAsciiChecked(locale_str.c_str());
 

src/objects/js-locale.cc

@@ -391,7 +391,7 @@ Handle<String> MorphLocale(Isolate* isolate, String locale,
   (*morph_func)(&icu_locale, &status);
   CHECK(U_SUCCESS(status));
   CHECK(!icu_locale.isBogus());
-  std::string locale_str = Intl::ToLanguageTag(icu_locale);
+  std::string locale_str = Intl::ToLanguageTag(icu_locale).FromJust();
   return isolate->factory()->NewStringFromAsciiChecked(locale_str.c_str());
 }
 
@@ -435,7 +435,7 @@ Handle<Object> JSLocale::Region(Isolate* isolate, Handle<JSLocale> locale) {
 Handle<String> JSLocale::BaseName(Isolate* isolate, Handle<JSLocale> locale) {
   icu::Locale icu_locale =
       icu::Locale::createFromName(locale->icu_locale()->raw()->getBaseName());
-  std::string base_name = Intl::ToLanguageTag(icu_locale);
+  std::string base_name = Intl::ToLanguageTag(icu_locale).FromJust();
   return isolate->factory()->NewStringFromAsciiChecked(base_name.c_str());
 }
 
@@ -471,7 +471,7 @@ Handle<Object> JSLocale::NumberingSystem(Isolate* isolate,
 
 Handle<String> JSLocale::ToString(Isolate* isolate, Handle<JSLocale> locale) {
   icu::Locale* icu_locale = locale->icu_locale()->raw();
-  std::string locale_str = Intl::ToLanguageTag(*icu_locale);
+  std::string locale_str = Intl::ToLanguageTag(*icu_locale).FromJust();
   return isolate->factory()->NewStringFromAsciiChecked(locale_str.c_str());
 }
 

test/intl/regress-917151.js

@@ -0,0 +1,11 @@
+// Copyright 2016 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.
+
+// Regression test for 917151
+
+assertThrows(
+    () => Number.prototype.toLocaleString.call(
+          -22,
+          "x-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6-6"),
+    RangeError)