ICU-13850 Make CurrencyUnit safe with 1-length and 2-length strings. (#133)

Also adds expectError to IcuTestErrorCode.

diff --git a/icu4c/source/i18n/currunit.cpp b/icu4c/source/i18n/currunit.cpp
index bd6059366cd35b73ada5fb0830bc1e8f950d0159..7f3490d406b23d7930c709f1d2101270e10bcc91 100644 (file)
--- a/icu4c/source/i18n/currunit.cpp
+++ b/icu4c/source/i18n/currunit.cpp
@@ -28,9 +28,13 @@ CurrencyUnit::CurrencyUnit(ConstChar16Ptr _isoCode, UErrorCode& ec) {
     // 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.
     // However, we allow a NUL-terminated empty string, which should have the same behavior as nullptr.
+    // Consider NUL-terminated strings of length 1 or 2 as invalid.
     const char16_t* isoCodeToUse;
     if (U_FAILURE(ec) || _isoCode == nullptr || _isoCode[0] == 0) {
         isoCodeToUse = kDefaultCurrency;
+    } else if (_isoCode[1] == 0 || _isoCode[2] == 0) {
+        isoCodeToUse = kDefaultCurrency;
+        ec = U_ILLEGAL_ARGUMENT_ERROR;
     } else if (!uprv_isInvariantUString(_isoCode, 3)) {
         // TODO: Perform a more strict ASCII check like in ICU4J isAlpha3Code?
         isoCodeToUse = kDefaultCurrency;

diff --git a/icu4c/source/test/intltest/itutil.cpp b/icu4c/source/test/intltest/itutil.cpp
index 7b2995f7ccdcdaef777f9a442953363710d441be..4d3466120cf557e8677092c124dd147b6268ee82 100644 (file)
--- a/icu4c/source/test/intltest/itutil.cpp
+++ b/icu4c/source/test/intltest/itutil.cpp
@@ -300,7 +300,7 @@ void ErrorCodeTest::TestIcuTestErrorCode() {
     helper.test = this;
 
     // Test destructor message
-    helper.expectedErrln = u"AAA failure: U_ILLEGAL_PAD_POSITION";
+    helper.expectedErrln = u"AAA destructor: expected success but got error: U_ILLEGAL_PAD_POSITION";
     helper.expectedDataErr = FALSE;
     helper.seenError = FALSE;
     {
@@ -310,7 +310,7 @@ void ErrorCodeTest::TestIcuTestErrorCode() {
     assertTrue("Should have seen an error", helper.seenError);
 
     // Test destructor message with scope
-    helper.expectedErrln = u"BBB failure: U_ILLEGAL_PAD_POSITION scope: foo";
+    helper.expectedErrln = u"BBB destructor: expected success but got error: U_ILLEGAL_PAD_POSITION scope: foo";
     helper.expectedDataErr = FALSE;
     helper.seenError = FALSE;
     {
@@ -321,7 +321,7 @@ void ErrorCodeTest::TestIcuTestErrorCode() {
     assertTrue("Should have seen an error", helper.seenError);
 
     // Check errIfFailure message with scope
-    helper.expectedErrln = u"CCC failure: U_ILLEGAL_PAD_POSITION scope: foo";
+    helper.expectedErrln = u"CCC expected success but got error: U_ILLEGAL_PAD_POSITION scope: foo";
     helper.expectedDataErr = FALSE;
     helper.seenError = FALSE;
     {
@@ -331,14 +331,14 @@ void ErrorCodeTest::TestIcuTestErrorCode() {
         testStatus.errIfFailureAndReset();
         assertTrue("Should have seen an error", helper.seenError);
         helper.seenError = FALSE;
-        helper.expectedErrln = u"CCC failure: U_ILLEGAL_CHAR_FOUND scope: foo - 5.4300";
+        helper.expectedErrln = u"CCC expected success but got error: U_ILLEGAL_CHAR_FOUND scope: foo - 5.4300";
         testStatus.set(U_ILLEGAL_CHAR_FOUND);
         testStatus.errIfFailureAndReset("%6.4f", 5.43);
         assertTrue("Should have seen an error", helper.seenError);
     }
 
     // Check errDataIfFailure message without scope
-    helper.expectedErrln = u"DDD failure: U_ILLEGAL_PAD_POSITION";
+    helper.expectedErrln = u"DDD data: expected success but got error: U_ILLEGAL_PAD_POSITION";
     helper.expectedDataErr = TRUE;
     helper.seenError = FALSE;
     {
@@ -347,11 +347,33 @@ void ErrorCodeTest::TestIcuTestErrorCode() {
         testStatus.errDataIfFailureAndReset();
         assertTrue("Should have seen an error", helper.seenError);
         helper.seenError = FALSE;
-        helper.expectedErrln = u"DDD failure: U_ILLEGAL_CHAR_FOUND - 5.4300";
+        helper.expectedErrln = u"DDD data: expected success but got error: U_ILLEGAL_CHAR_FOUND - 5.4300";
         testStatus.set(U_ILLEGAL_CHAR_FOUND);
         testStatus.errDataIfFailureAndReset("%6.4f", 5.43);
         assertTrue("Should have seen an error", helper.seenError);
     }
+
+    // Check expectFailure
+    helper.expectedErrln = u"EEE expected: U_ILLEGAL_CHAR_FOUND but got error: U_ILLEGAL_PAD_POSITION";
+    helper.expectedDataErr = FALSE;
+    helper.seenError = FALSE;
+    {
+        IcuTestErrorCode testStatus(helper, "EEE");
+        testStatus.set(U_ILLEGAL_PAD_POSITION);
+        testStatus.expectErrorAndReset(U_ILLEGAL_PAD_POSITION);
+        assertFalse("Should NOT have seen an error", helper.seenError);
+        testStatus.set(U_ILLEGAL_PAD_POSITION);
+        testStatus.expectErrorAndReset(U_ILLEGAL_CHAR_FOUND);
+        assertTrue("Should have seen an error", helper.seenError);
+        helper.seenError = FALSE;
+        helper.expectedErrln = u"EEE expected: U_ILLEGAL_CHAR_FOUND but got error: U_ZERO_ERROR scope: scopety scope - 5.4300";
+        testStatus.setScope("scopety scope");
+        testStatus.set(U_ILLEGAL_PAD_POSITION);
+        testStatus.expectErrorAndReset(U_ILLEGAL_PAD_POSITION, "%6.4f", 5.43);
+        assertFalse("Should NOT have seen an error", helper.seenError);
+        testStatus.expectErrorAndReset(U_ILLEGAL_CHAR_FOUND, "%6.4f", 5.43);
+        assertTrue("Should have seen an error", helper.seenError);
+    }
 }
 
 

diff --git a/icu4c/source/test/intltest/measfmttest.cpp b/icu4c/source/test/intltest/measfmttest.cpp
index 8d7ec8165b03163a651f1495510a7fdf9ca9ee8b..4928962d3952c5c1560e2dbff613bd1daa6482e5 100644 (file)
--- a/icu4c/source/test/intltest/measfmttest.cpp
+++ b/icu4c/source/test/intltest/measfmttest.cpp
@@ -1857,9 +1857,14 @@ void MeasureFormatTest::TestGram() {
 }
 
 void MeasureFormatTest::TestCurrencies() {
-    UChar USD[4];
+    UChar USD[4] = {};
     u_uastrcpy(USD, "USD");
     UErrorCode status = U_ZERO_ERROR;
+    CurrencyUnit USD_unit(USD, status);
+    assertEquals("Currency Unit", USD, USD_unit.getISOCurrency());
+    if (!assertSuccess("Error creating CurrencyUnit", status)) {
+        return;
+    }
     CurrencyAmount USD_1(1.0, USD, status);
     assertEquals("Currency Code", USD, USD_1.getISOCurrency());
     CurrencyAmount USD_2(2.0, USD, status);

diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp
index 77d4283441b21055fcb2ffc5c76cbc7e451775fe..ecd6f95380db59b8f64293463159c31647330be9 100644 (file)
--- a/icu4c/source/test/intltest/numfmtst.cpp
+++ b/icu4c/source/test/intltest/numfmtst.cpp
@@ -9241,22 +9241,40 @@ void NumberFormatTest::Test13840_ParseLongStringCrash() {
 void NumberFormatTest::Test13850_EmptyStringCurrency() {
     IcuTestErrorCode status(*this, "Test13840_EmptyStringCurrency");
 
-    LocalPointer<NumberFormat> nf(NumberFormat::createCurrencyInstance("en-US", status), status);
-    if (status.errIfFailureAndReset()) { return; }
-    UnicodeString actual;
-    nf->format(1, actual, status);
-    assertEquals("Should format with US currency", u"$1.00", actual);
-    nf->setCurrency(u"", status);
-    nf->format(1, actual.remove(), status);
-    assertEquals("Should unset the currency on empty string", u"XXX\u00A01.00", actual);
-
-    // Try with nullptr
-    nf.adoptInstead(NumberFormat::createCurrencyInstance("en-US", status));
-    nf->format(1, actual.remove(), status);
-    assertEquals("Should format with US currency", u"$1.00", actual);
-    nf->setCurrency(nullptr, status);
-    nf->format(1, actual.remove(), status);
-    assertEquals("Should unset the currency on nullptr", u"XXX\u00A01.00", actual);
+    struct TestCase {
+        const char16_t* currencyArg;
+        UErrorCode expectedError;
+    } cases[] = {
+        {u"", U_ZERO_ERROR},
+        {u"U", U_ILLEGAL_ARGUMENT_ERROR},
+        {u"Us", U_ILLEGAL_ARGUMENT_ERROR},
+        {nullptr, U_ZERO_ERROR},
+        {u"U$D", U_INVARIANT_CONVERSION_ERROR},
+        {u"Xxx", U_ZERO_ERROR}
+    };
+    for (const auto& cas : cases) {
+        UnicodeString message(u"with currency arg: ");
+        if (cas.currencyArg == nullptr) {
+            message += u"nullptr";
+        } else {
+            message += UnicodeString(cas.currencyArg);
+        }
+        status.setScope(message);
+        LocalPointer<NumberFormat> nf(NumberFormat::createCurrencyInstance("en-US", status), status);
+        if (status.errIfFailureAndReset()) { return; }
+        UnicodeString actual;
+        nf->format(1, actual, status);
+        status.errIfFailureAndReset();
+        assertEquals(u"Should format with US currency " + message, u"$1.00", actual);
+        nf->setCurrency(cas.currencyArg, status);
+        if (status.expectErrorAndReset(cas.expectedError)) {
+            // If an error occurred, do not check formatting.
+            continue;
+        }
+        nf->format(1, actual.remove(), status);
+        assertEquals(u"Should unset the currency " + message, u"XXX\u00A01.00", actual);
+        status.errIfFailureAndReset();
+    }
 }
 
 #endif /* #if !UCONFIG_NO_FORMATTING */

diff --git a/icu4c/source/tools/ctestfw/tstdtmod.cpp b/icu4c/source/tools/ctestfw/tstdtmod.cpp
index 797803c5cd9d3c3d8adba566cfc4d6d874a13042..f4580b165da8f74a57fdea77425dfbb9529ed71b 100644 (file)
--- a/icu4c/source/tools/ctestfw/tstdtmod.cpp
+++ b/icu4c/source/tools/ctestfw/tstdtmod.cpp
@@ -21,13 +21,13 @@ TestLog::~TestLog() {}
 IcuTestErrorCode::~IcuTestErrorCode() {
     // Safe because our errlog() does not throw exceptions.
     if(isFailure()) {
-        errlog(FALSE, nullptr);
+        errlog(FALSE, u"destructor: expected success", nullptr);
     }
 }
 
 UBool IcuTestErrorCode::errIfFailureAndReset() {
     if(isFailure()) {
-        errlog(FALSE, nullptr);
+        errlog(FALSE, u"expected success", nullptr);
         reset();
         return TRUE;
     } else {
@@ -43,7 +43,7 @@ UBool IcuTestErrorCode::errIfFailureAndReset(const char *fmt, ...) {
         va_start(ap, fmt);
         vsprintf(buffer, fmt, ap);
         va_end(ap);
-        errlog(FALSE, buffer);
+        errlog(FALSE, u"expected success", buffer);
         reset();
         return TRUE;
     } else {
@@ -54,7 +54,7 @@ UBool IcuTestErrorCode::errIfFailureAndReset(const char *fmt, ...) {
 
 UBool IcuTestErrorCode::errDataIfFailureAndReset() {
     if(isFailure()) {
-        errlog(TRUE, nullptr);
+        errlog(TRUE, u"data: expected success", nullptr);
         reset();
         return TRUE;
     } else {
@@ -70,7 +70,7 @@ UBool IcuTestErrorCode::errDataIfFailureAndReset(const char *fmt, ...) {
         va_start(ap, fmt);
         vsprintf(buffer, fmt, ap);
         va_end(ap);
-        errlog(TRUE, buffer);
+        errlog(TRUE, u"data: expected success", buffer);
         reset();
         return TRUE;
     } else {
@@ -79,6 +79,29 @@ UBool IcuTestErrorCode::errDataIfFailureAndReset(const char *fmt, ...) {
     }
 }
 
+UBool IcuTestErrorCode::expectErrorAndReset(UErrorCode expectedError) {
+    if(get() != expectedError) {
+        errlog(FALSE, UnicodeString(u"expected: ") + u_errorName(expectedError), nullptr);
+    }
+    UBool retval = isFailure();
+    reset();
+    return retval;
+}
+
+UBool IcuTestErrorCode::expectErrorAndReset(UErrorCode expectedError, const char *fmt, ...) {
+    if(get() != expectedError) {
+        char buffer[4000];
+        va_list ap;
+        va_start(ap, fmt);
+        vsprintf(buffer, fmt, ap);
+        va_end(ap);
+        errlog(FALSE, UnicodeString(u"expected: ") + u_errorName(expectedError), buffer);
+    }
+    UBool retval = isFailure();
+    reset();
+    return retval;
+}
+
 void IcuTestErrorCode::setScope(const char* message) {
     scopeMessage.remove().append({ message, -1, US_INV });
 }
@@ -88,12 +111,13 @@ void IcuTestErrorCode::setScope(const UnicodeString& message) {
 }
 
 void IcuTestErrorCode::handleFailure() const {
-    errlog(FALSE, nullptr);
+    errlog(FALSE, u"(handleFailure)", nullptr);
 }
 
-void IcuTestErrorCode::errlog(UBool dataErr, const char* extraMessage) const {
+void IcuTestErrorCode::errlog(UBool dataErr, const UnicodeString& mainMessage, const char* extraMessage) const {
     UnicodeString msg(testName, -1, US_INV);
-    msg.append(u" failure: ").append(UnicodeString(errorName(), -1, US_INV));
+    msg.append(u' ').append(mainMessage);
+    msg.append(u" but got error: ").append(UnicodeString(errorName(), -1, US_INV));
 
     if (!scopeMessage.isEmpty()) {
         msg.append(u" scope: ").append(scopeMessage);

diff --git a/icu4c/source/tools/ctestfw/unicode/testlog.h b/icu4c/source/tools/ctestfw/unicode/testlog.h
index 11b1c6e3b11fdf5528a45ad3368063a020cbe9f9..9db35b9ca853986a2c5c157a841fd99c38bc4a41 100644 (file)
--- a/icu4c/source/tools/ctestfw/unicode/testlog.h
+++ b/icu4c/source/tools/ctestfw/unicode/testlog.h
@@ -41,6 +41,8 @@ public:
     UBool errIfFailureAndReset(const char *fmt, ...);
     UBool errDataIfFailureAndReset();
     UBool errDataIfFailureAndReset(const char *fmt, ...);
+    UBool expectErrorAndReset(UErrorCode expectedError);
+    UBool expectErrorAndReset(UErrorCode expectedError, const char *fmt, ...);
 
     /** Sets an additional message string to be appended to failure output. */
     void setScope(const char* message);
@@ -54,7 +56,7 @@ private:
     const char *const testName;
     UnicodeString scopeMessage;
 
-    void errlog(UBool dataErr, const char* extraMessage) const;
+    void errlog(UBool dataErr, const UnicodeString& mainMessage, const char* extraMessage) const;
 };
 
 #endif