[Intl] Handle invalid values in locale

Fix the bug that we call setKeywordValue w/o convert the key to legacy key.

Bug: v8:7481
Change-Id: I5a743e41d02b64385c5050fc1d83279741e24659
Reviewed-on: https://chromium-review.googlesource.com/c/1339243
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57886}

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

@@ -846,9 +846,7 @@ MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::Initialize(
   // The value of the [[RelevantExtensionKeys]] internal slot is
   // « "ca", "nu", "hc" ».
   //
-  // TODO(ftang): Add "hc" to this list of keys:
-  // https://bugs.chromium.org/p/v8/issues/detail?id=7482
-  std::set<std::string> relevant_extension_keys = {"nu", "ca"};
+  std::set<std::string> relevant_extension_keys = {"nu", "ca", "hc"};
 
   // 10. Let localeData be %DateTimeFormat%.[[LocaleData]].
   // 11. Let r be ResolveLocale( %DateTimeFormat%.[[AvailableLocales]],
@@ -859,8 +857,6 @@ MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::Initialize(
       isolate, JSDateTimeFormat::GetAvailableLocales(), requested_locales,
       locale_matcher, relevant_extension_keys);
 
-  // TODO(ftang): Make sure that "nu" key doesn't have "native",
-  // "traditio" or "finance" values.
   icu::Locale icu_locale = r.icu_locale;
   DCHECK(!icu_locale.isBogus());
 
@@ -916,6 +912,40 @@ MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::Initialize(
     }
   }
 
+  // The list that is the value of the "nu" field of any locale field of
+  // [[LocaleData]] must not include the values "native",  "traditio", or
+  // "finance".
+  //
+  // See https://tc39.github.io/ecma402/#sec-intl.datetimeformat-internal-slots
+  UErrorCode status = U_ZERO_ERROR;
+  const std::map<std::string, std::string>& extensions = r.extensions;
+  if (extensions.find("nu") != extensions.end()) {
+    const std::string& value = extensions.at("nu");
+    if (value == "native" || value == "traditio" || value == "finance") {
+      // 10. Set numberFormat.[[NumberingSystem]] to r.[[nu]].
+      // Note: setUnicodeKeywordValue is buggy with nullptr or "" as 2nd
+      // parameter now See https://unicode-org.atlassian.net/browse/ICU-20276
+      // TODO(ftang): change to use icu_locale.setUnicodeKeywordValue in ICU 64
+      // after ICU-20276 got fixed.
+      icu_locale.setKeywordValue(uloc_toLegacyKey(("nu")), nullptr, status);
+      CHECK(U_SUCCESS(status));
+    }
+  }
+  // The list that is the value of the "hc" field of any locale field of
+  // [[LocaleData]] must not include the values other than "h11", "h12", "h23",
+  // "h24"
+  if (extensions.find("hc") != extensions.end()) {
+    const std::string& value = extensions.at("hc");
+    if (value != "h11" && value != "h12" && value != "h23" && value != "h24") {
+      // Note: setUnicodeKeywordValue is buggy with nullptr or "" as 2nd
+      // parameter now See https://unicode-org.atlassian.net/browse/ICU-20276
+      // TODO(ftang): change to use icu_locale.setUnicodeKeywordValue in ICU 64
+      // after ICU-20276 got fixed.
+      icu_locale.setKeywordValue(uloc_toLegacyKey(("hc")), nullptr, status);
+      CHECK(U_SUCCESS(status));
+    }
+  }
+
   // Set the locale
   // 12. Set dateTimeFormat.[[Locale]] to r.[[locale]].
   icu::Locale* cloned_locale = icu_locale.clone();

src/objects/js-number-format.cc

@@ -259,31 +259,38 @@ MaybeHandle<JSNumberFormat> JSNumberFormat::Initialize(
       Intl::ResolveLocale(isolate, JSNumberFormat::GetAvailableLocales(),
                           requested_locales, matcher, relevant_extension_keys);
 
-  // 9. Set numberFormat.[[Locale]] to r.[[locale]].
-  Handle<String> locale_str =
-      isolate->factory()->NewStringFromAsciiChecked(r.locale.c_str());
-  number_format->set_locale(*locale_str);
 
   icu::Locale icu_locale = r.icu_locale;
   DCHECK(!icu_locale.isBogus());
 
-  std::map<std::string, std::string> extensions = r.extensions;
+  const std::map<std::string, std::string>& extensions = r.extensions;
 
   // The list that is the value of the "nu" field of any locale field of
   // [[LocaleData]] must not include the values "native",  "traditio", or
   // "finance".
   //
   // See https://tc39.github.io/ecma402/#sec-intl.numberformat-internal-slots
+  UErrorCode status = U_ZERO_ERROR;
   if (extensions.find("nu") != extensions.end()) {
-    const std::string value = extensions.at("nu");
+    const std::string& value = extensions.at("nu");
     if (value == "native" || value == "traditio" || value == "finance") {
       // 10. Set numberFormat.[[NumberingSystem]] to r.[[nu]].
-      UErrorCode status = U_ZERO_ERROR;
-      icu_locale.setKeywordValue("nu", nullptr, status);
+      // Note: setUnicodeKeywordValue is buggy with nullptr or "" as 2nd
+      // parameter now See https://unicode-org.atlassian.net/browse/ICU-20276
+      // TODO(ftang): change to use icu_locale.setUnicodeKeywordValue in ICU 64
+      // after ICU-20276 got fixed.
+      icu_locale.setKeywordValue(uloc_toLegacyKey(("nu")), nullptr, status);
       CHECK(U_SUCCESS(status));
     }
   }
 
+  // 9. Set numberFormat.[[Locale]] to r.[[locale]].
+  status = U_ZERO_ERROR;
+  std::string locale_str = icu_locale.toLanguageTag<std::string>(status);
+  CHECK(U_SUCCESS(status));
+  number_format->set_locale(
+      *factory->NewStringFromAsciiChecked(locale_str.c_str()));
+
   // 11. Let dataLocale be r.[[dataLocale]].
   //
   // 12. Let style be ? GetOption(options, "style", "string",  « "decimal",
@@ -353,8 +360,8 @@ MaybeHandle<JSNumberFormat> JSNumberFormat::Initialize(
   CurrencyDisplay currency_display = maybe_currencyDisplay.FromJust();
   UNumberFormatStyle format_style = ToNumberFormatStyle(currency_display);
 
-  UErrorCode status = U_ZERO_ERROR;
   std::unique_ptr<icu::NumberFormat> icu_number_format;
+  status = U_ZERO_ERROR;
   if (style == Style::DECIMAL) {
     icu_number_format.reset(
         icu::NumberFormat::createInstance(icu_locale, status));
@@ -385,9 +392,9 @@ MaybeHandle<JSNumberFormat> JSNumberFormat::Initialize(
     // currencyDisplay.
     number_format->set_currency_display(currency_display);
 
+    status = U_ZERO_ERROR;
     // 17.b. Set numberFormat.[[Currency]] to currency.
     if (!currency_ustr.isEmpty()) {
-      status = U_ZERO_ERROR;
       icu_number_format->setCurrency(currency_ustr.getBuffer(), status);
       CHECK(U_SUCCESS(status));
     }

test/intl/regress-7481.js

+// Copyright 2018 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.
+assertEquals(
+    "en-u-hc-h11-nu-arab",
+    new Intl.DateTimeFormat(["en-u-hc-h11-nu-arab"]).resolvedOptions().locale
+);
+assertEquals(
+    "en-u-hc-h12-nu-arab",
+    new Intl.DateTimeFormat(["en-u-hc-h12-nu-arab"]).resolvedOptions().locale
+);
+assertEquals(
+    "en-u-hc-h23-nu-arab",
+    new Intl.DateTimeFormat(["en-u-hc-h23-nu-arab"]).resolvedOptions().locale
+);
+assertEquals(
+    "en-u-hc-h24-nu-arab",
+    new Intl.DateTimeFormat(["en-u-hc-h24-nu-arab"]).resolvedOptions().locale
+);
+
+// https://tc39.github.io/ecma402/#sec-intl.datetimeformat-internal-slots
+// invalid hc should be removed
+// [[LocaleData]][locale].hc must be « null, "h11", "h12", "h23", "h24" » for all locale values.
+assertEquals(
+    "en-u-nu-arab",
+    new Intl.DateTimeFormat(["en-u-hc-h10-nu-arab"]).resolvedOptions().locale
+);
+assertEquals(
+    "en-u-nu-arab",
+    new Intl.DateTimeFormat(["en-u-hc-h13-nu-arab"]).resolvedOptions().locale
+);
+assertEquals(
+    "en-u-nu-arab",
+    new Intl.DateTimeFormat(["en-u-hc-h22-nu-arab"]).resolvedOptions().locale
+);
+assertEquals(
+    "en-u-nu-arab",
+    new Intl.DateTimeFormat(["en-u-hc-h25-nu-arab"]).resolvedOptions().locale
+);

test/test262/test262.status

@@ -591,6 +591,8 @@
 
   # https://bugs.chromium.org/p/v8/issues/detail?id=7481
   'intl402/NumberFormat/ignore-invalid-unicode-ext-values': [FAIL],
+
+  # https://crbug.com/v8/8469
   'intl402/DateTimeFormat/ignore-invalid-unicode-ext-values': [FAIL],
 
   # https://bugs.chromium.org/p/v8/issues/detail?id=7482