ICU-13634 Fixing address sanitizer issue involving backwards-compatible UChar* behavi...

X-SVN-Rev: 41248

diff --git a/icu4c/source/i18n/currunit.cpp b/icu4c/source/i18n/currunit.cpp
index 003ce612109e78990e5a70c5fa7a8392149b21d7..5a6ff6e7e19ffc0294e7294e10b4e0dcbeb6b050 100644 (file)
--- a/icu4c/source/i18n/currunit.cpp
+++ b/icu4c/source/i18n/currunit.cpp
@@ -17,6 +17,7 @@
 #include "unicode/currunit.h"
 #include "unicode/ustring.h"
 #include "cstring.h"
+#include "uinvchar.h"
 
 static constexpr char16_t kDefaultCurrency[] = u"XXX";
 
@@ -24,14 +25,20 @@ U_NAMESPACE_BEGIN
 
 CurrencyUnit::CurrencyUnit(ConstChar16Ptr _isoCode, UErrorCode& ec) {
     // The constructor always leaves the CurrencyUnit in a valid state (with a 3-character currency code).
+    // Note: in ICU4J Currency.getInstance(), we check string length for 3, but in ICU4C we allow a
+    // non-NUL-terminated string to be passed as an argument, so it is not possible to check length.
+    const char16_t* isoCodeToUse;
     if (U_FAILURE(ec) || _isoCode == nullptr) {
-        u_strcpy(isoCode, kDefaultCurrency);
-    } else if (u_strlen(_isoCode) != 3) {
-        u_strcpy(isoCode, kDefaultCurrency);
-        ec = U_ILLEGAL_ARGUMENT_ERROR;
+        isoCodeToUse = kDefaultCurrency;
+    } else if (!uprv_isInvariantUString(_isoCode, 3)) {
+        // TODO: Perform a more strict ASCII check like in ICU4J isAlpha3Code?
+        isoCodeToUse = kDefaultCurrency;
+        ec = U_INVARIANT_CONVERSION_ERROR;
     } else {
-        u_strcpy(isoCode, _isoCode);
+        isoCodeToUse = _isoCode;
     }
+    u_strncpy(isoCode, isoCodeToUse, 3);
+    isoCode[3] = 0;
     char simpleIsoCode[4];
     u_UCharsToChars(isoCode, simpleIsoCode, 4);
     initCurrency(simpleIsoCode);

diff --git a/icu4c/source/i18n/decimfmt.cpp b/icu4c/source/i18n/decimfmt.cpp
index c7192dc46911b3c073f7eb9b84bc887d9b2dceb5..42cd90233bf6dc11b447a37874a7a1423f079f00 100644 (file)
--- a/icu4c/source/i18n/decimfmt.cpp
+++ b/icu4c/source/i18n/decimfmt.cpp
@@ -992,7 +992,6 @@ void DecimalFormat::setCurrency(const char16_t* theCurrency, UErrorCode& ec) {
 
 void DecimalFormat::setCurrency(const char16_t* theCurrency) {
     ErrorCode localStatus;
-    NumberFormat::setCurrency(theCurrency, localStatus); // to set field for compatibility
     setCurrency(theCurrency, localStatus);
 }
 

diff --git a/icu4c/source/i18n/number_skeletons.cpp b/icu4c/source/i18n/number_skeletons.cpp
index d921b547e1309d6c8d4fc58cfbc1a6cd290bff92..e998e8e51c358abe715c2178f92dc1155ced5d0f 100644 (file)
--- a/icu4c/source/i18n/number_skeletons.cpp
+++ b/icu4c/source/i18n/number_skeletons.cpp
@@ -800,8 +800,12 @@ blueprint_helpers::parseExponentSignOption(const StringSegment& segment, MacroPr
 
 void blueprint_helpers::parseCurrencyOption(const StringSegment& segment, MacroProps& macros,
                                             UErrorCode& status) {
-    // Can't use toTempUnicodeString() because getTerminatedBuffer is non-const
-    const UChar* currencyCode = segment.toUnicodeString().getTerminatedBuffer();
+    // Unlike ICU4J, have to check length manually because ICU4C CurrencyUnit does not check it for us
+    if (segment.length() != 3) {
+        status = U_NUMBER_SKELETON_SYNTAX_ERROR;
+        return;
+    }
+    const UChar* currencyCode = segment.toTempUnicodeString().getBuffer();
     UErrorCode localStatus = U_ZERO_ERROR;
     CurrencyUnit currency(currencyCode, localStatus);
     if (U_FAILURE(localStatus)) {

diff --git a/icu4c/source/i18n/unicode/currunit.h b/icu4c/source/i18n/unicode/currunit.h
index 5ad23b1f58a3a85c5208a9cbab2fb26c43cf27fa..ca90acb79183325f1e0eb8964b8dc63b1d66a988 100644 (file)
--- a/icu4c/source/i18n/unicode/currunit.h
+++ b/icu4c/source/i18n/unicode/currunit.h
@@ -44,8 +44,9 @@ class U_I18N_API CurrencyUnit: public MeasureUnit {
 
     /**
      * Construct an object with the given ISO currency code.
-     * @param isoCode the 3-letter ISO 4217 currency code; must not be
-     * NULL and must have length 3
+     * @param isoCode the 3-letter ISO 4217 currency code; must have
+     * length 3 and need not be NUL-terminated. If NULL, the currency
+     * is initialized to the unknown currency XXX.
      * @param ec input-output error code. If the isoCode is invalid,
      * then this will be set to a failing value.
      * @stable ICU 3.0

diff --git a/icu4c/source/test/intltest/numbertest_skeletons.cpp b/icu4c/source/test/intltest/numbertest_skeletons.cpp
index 33de5c311b2d6ef641c4363249ce3377e9372f9d..59ad630ce9c6bac60dfc88a98173a2113413a043 100644 (file)
--- a/icu4c/source/test/intltest/numbertest_skeletons.cpp
+++ b/icu4c/source/test/intltest/numbertest_skeletons.cpp
@@ -68,6 +68,7 @@ void NumberSkeletonTest::validTokens() {
             u"measure-unit/energy-joule per-measure-unit/length-meter",
             u"currency/XXX",
             u"currency/ZZZ",
+            u"currency/usd",
             u"group-off",
             u"group-min2",
             u"group-auto",
@@ -137,6 +138,7 @@ void NumberSkeletonTest::invalidTokens() {
             u"scale/0.1.2",
             u"scale/français", // non-invariant characters for C++
             u"currency/dummy",
+            u"currency/ççç", // three characters but not ASCII
             u"measure-unit/foo",
             u"integer-width/xxx",
             u"integer-width/0+",

diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberSkeletonTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberSkeletonTest.java
index 7c7349b5e99484c15842ad7418b08e22a90aac8b..6e854dfbf2c9d39204e4cce4434b86c526721e9f 100644 (file)
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberSkeletonTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberSkeletonTest.java
@@ -60,6 +60,7 @@ public class NumberSkeletonTest {
                 "measure-unit/energy-joule per-measure-unit/length-meter",
                 "currency/XXX",
                 "currency/ZZZ",
+                "currency/usd",
                 "group-off",
                 "group-min2",
                 "group-auto",
@@ -131,6 +132,7 @@ public class NumberSkeletonTest {
                 "scale/0.1.2",
                 "scale/français", // non-invariant characters for C++
                 "currency/dummy",
+                "currency/ççç", // three characters but not ASCII
                 "measure-unit/foo",
                 "integer-width/xxx",
                 "integer-width/0+",