ICU-21683 ICU-21674 Clear state from UFormattedNumber[Range] in C API

See #1862

diff --git a/icu4c/source/i18n/formattedval_impl.h b/icu4c/source/i18n/formattedval_impl.h
index 4ff25dcfb879227caabb80fdae85fe5c4f675299..2b9a3970d2e4ebaed20a7f3c9ca8dade9dfe0e7c 100644 (file)
--- a/icu4c/source/i18n/formattedval_impl.h
+++ b/icu4c/source/i18n/formattedval_impl.h
@@ -169,6 +169,7 @@ public:
     inline const FormattedStringBuilder& getStringRef() const {
         return fString;
     }
+    void resetString();
 
     /**
      * Adds additional metadata used for span fields.

diff --git a/icu4c/source/i18n/formattedval_sbimpl.cpp b/icu4c/source/i18n/formattedval_sbimpl.cpp
index 9ec06daf3ea41e8ba794becdd3c1de5609878b5b..70ffacac4b7416433e9cff4895dd77c1276c3a3d 100644 (file)
--- a/icu4c/source/i18n/formattedval_sbimpl.cpp
+++ b/icu4c/source/i18n/formattedval_sbimpl.cpp
@@ -96,6 +96,11 @@ void FormattedValueStringBuilderImpl::getAllFieldPositions(FieldPositionIterator
     }
 }
 
+void FormattedValueStringBuilderImpl::resetString() {
+    fString.clear();
+    spanIndicesCount = 0;
+}
+
 // Signal the end of the string using a field that doesn't exist and that is
 // different from kUndefinedField, which is used for "null field".
 static constexpr Field kEndField = Field(0xf, 0xf);
@@ -283,27 +288,27 @@ bool FormattedValueStringBuilderImpl::nextPositionImpl(ConstrainedFieldPosition&
 
 void FormattedValueStringBuilderImpl::appendSpanInfo(UFieldCategory category, int32_t spanValue, int32_t start, int32_t length, UErrorCode& status) {
     if (U_FAILURE(status)) { return; }
-    U_ASSERT(spanIndices.getCapacity() >= spanValue);
-    if (spanIndices.getCapacity() == spanValue) {
-        if (!spanIndices.resize(spanValue * 2, spanValue)) {
+    U_ASSERT(spanIndices.getCapacity() >= spanIndicesCount);
+    if (spanIndices.getCapacity() == spanIndicesCount) {
+        if (!spanIndices.resize(spanIndicesCount * 2, spanIndicesCount)) {
             status = U_MEMORY_ALLOCATION_ERROR;
             return;
         }
     }
-    spanIndices[spanValue] = {category, spanValue, start, length};
+    spanIndices[spanIndicesCount] = {category, spanValue, start, length};
     spanIndicesCount++;
 }
 
 void FormattedValueStringBuilderImpl::prependSpanInfo(UFieldCategory category, int32_t spanValue, int32_t start, int32_t length, UErrorCode& status) {
     if (U_FAILURE(status)) { return; }
-    U_ASSERT(spanIndices.getCapacity() >= spanValue);
-    if (spanIndices.getCapacity() == spanValue) {
-        if (!spanIndices.resize(spanValue * 2, spanValue)) {
+    U_ASSERT(spanIndices.getCapacity() >= spanIndicesCount);
+    if (spanIndices.getCapacity() == spanIndicesCount) {
+        if (!spanIndices.resize(spanIndicesCount * 2, spanIndicesCount)) {
             status = U_MEMORY_ALLOCATION_ERROR;
             return;
         }
     }
-    for (int32_t i = spanValue - 1; i >= 0; i--) {
+    for (int32_t i = spanIndicesCount - 1; i >= 0; i--) {
         spanIndices[i+1] = spanIndices[i];
     }
     spanIndices[0] = {category, spanValue, start, length};

diff --git a/icu4c/source/i18n/number_capi.cpp b/icu4c/source/i18n/number_capi.cpp
index 4cb56253628d7491d1861242b16e871151a3be9b..b87dbd93e5fcde6b8e97f08e895ea59b1b6d8797 100644 (file)
--- a/icu4c/source/i18n/number_capi.cpp
+++ b/icu4c/source/i18n/number_capi.cpp
@@ -116,7 +116,8 @@ unumf_formatInt(const UNumberFormatter* uformatter, int64_t value, UFormattedNum
     auto* result = UFormattedNumberApiHelper::validate(uresult, *ec);
     if (U_FAILURE(*ec)) { return; }
 
-    result->fData.getStringRef().clear();
+    result->fData.resetString();
+    result->fData.quantity.clear();
     result->fData.quantity.setToLong(value);
     formatter->fFormatter.formatImpl(&result->fData, *ec);
 }
@@ -128,7 +129,8 @@ unumf_formatDouble(const UNumberFormatter* uformatter, double value, UFormattedN
     auto* result = UFormattedNumberApiHelper::validate(uresult, *ec);
     if (U_FAILURE(*ec)) { return; }
 
-    result->fData.getStringRef().clear();
+    result->fData.resetString();
+    result->fData.quantity.clear();
     result->fData.quantity.setToDouble(value);
     formatter->fFormatter.formatImpl(&result->fData, *ec);
 }
@@ -140,7 +142,8 @@ unumf_formatDecimal(const UNumberFormatter* uformatter, const char* value, int32
     auto* result = UFormattedNumberApiHelper::validate(uresult, *ec);
     if (U_FAILURE(*ec)) { return; }
 
-    result->fData.getStringRef().clear();
+    result->fData.resetString();
+    result->fData.quantity.clear();
     result->fData.quantity.setToDecNumber({value, valueLen}, *ec);
     if (U_FAILURE(*ec)) { return; }
     formatter->fFormatter.formatImpl(&result->fData, *ec);

diff --git a/icu4c/source/i18n/numrange_capi.cpp b/icu4c/source/i18n/numrange_capi.cpp
index a440a53fe6b173e0456773b381b207956c219071..bd3a9ef5e82528c65aac0eafead899f82b9d5dc0 100644 (file)
--- a/icu4c/source/i18n/numrange_capi.cpp
+++ b/icu4c/source/i18n/numrange_capi.cpp
@@ -115,7 +115,9 @@ unumrf_formatDoubleRange(
     auto* result = UFormattedNumberRangeApiHelper::validate(uresult, *ec);
     if (U_FAILURE(*ec)) { return; }
 
-    result->fData.getStringRef().clear();
+    result->fData.resetString();
+    result->fData.quantity1.clear();
+    result->fData.quantity2.clear();
     result->fData.quantity1.setToDouble(first);
     result->fData.quantity2.setToDouble(second);
     formatter->fFormatter.formatImpl(result->fData, first == second, *ec);
@@ -132,7 +134,9 @@ unumrf_formatDecimalRange(
     auto* result = UFormattedNumberRangeApiHelper::validate(uresult, *ec);
     if (U_FAILURE(*ec)) { return; }
 
-    result->fData.getStringRef().clear();
+    result->fData.resetString();
+    result->fData.quantity1.clear();
+    result->fData.quantity2.clear();
     result->fData.quantity1.setToDecNumber({first, firstLen}, *ec);
     result->fData.quantity2.setToDecNumber({second, secondLen}, *ec);
     formatter->fFormatter.formatImpl(result->fData, first == second, *ec);

diff --git a/icu4c/source/test/cintltst/unumberformattertst.c b/icu4c/source/test/cintltst/unumberformattertst.c
index 75f1cca4c446c15ac146264b45a63e085c9d3746..ddba7fb059c83f3d00ef6de6998b66899e9e2cdc 100644 (file)
--- a/icu4c/source/test/cintltst/unumberformattertst.c
+++ b/icu4c/source/test/cintltst/unumberformattertst.c
@@ -32,6 +32,8 @@ static void TestToDecimalNumber(void);
 
 static void TestPerUnitInArabic(void);
 
+static void Test21674_State(void);
+
 void addUNumberFormatterTest(TestNode** root);
 
 #define TESTCASE(x) addTest(root, &x, "tsformat/unumberformatter/" #x)
@@ -44,6 +46,7 @@ void addUNumberFormatterTest(TestNode** root) {
     TESTCASE(TestSkeletonParseError);
     TESTCASE(TestToDecimalNumber);
     TESTCASE(TestPerUnitInArabic);
+    TESTCASE(Test21674_State);
 }
 
 
@@ -375,4 +378,47 @@ static void TestPerUnitInArabic() {
     }
     unumf_closeResult(formatted);
 }
+
+
+static void Test21674_State() {
+    UErrorCode status = U_ZERO_ERROR;
+    UNumberFormatter* nf = NULL;
+    UFormattedNumber* result = NULL;
+
+    nf = unumf_openForSkeletonAndLocale(u"precision-increment/0.05/w", -1, "en", &status);
+    if (!assertSuccess("unumf_openForSkeletonAndLocale", &status)) { goto cleanup; }
+
+    result = unumf_openResult(&status);
+    if (!assertSuccess("unumf_openResult", &status)) { goto cleanup; }
+
+    typedef struct TestCase {
+        double num;
+        const UChar* expected;
+    } TestCase;
+    TestCase cases[] = {
+        { 1.975, u"2" },
+        { 1.97, u"1.95" },
+        { 1.975, u"2" },
+    };
+    for (int i=0; i<3; i++) {
+        unumf_formatDouble(nf, cases[i].num, result, &status);
+        if (!assertSuccess("unumf_formatDouble", &status)) { goto cleanup; }
+
+        const UFormattedValue* formattedValue = unumf_resultAsValue(result, &status);
+        if (!assertSuccess("unumf_resultAsValue", &status)) { goto cleanup; }
+
+        int32_t length;
+        const UChar* str = ufmtval_getString(formattedValue, &length, &status);
+        if (!assertSuccess("ufmtval_getString", &status)) { goto cleanup; }
+
+        char message[] = {i + '0', '\0'};
+        assertUEquals(message, cases[i].expected, str);
+    }
+
+cleanup:
+    unumf_close(nf);
+    unumf_closeResult(result);
+}
+
+
 #endif /* #if !UCONFIG_NO_FORMATTING */

diff --git a/icu4c/source/test/intltest/numbertest.h b/icu4c/source/test/intltest/numbertest.h
index f5a4ba75f1b3d9048582485976fdd5e5c11baf97..be6fe89eb150964eb882f2562b35b28bb6001ba2 100644 (file)
--- a/icu4c/source/test/intltest/numbertest.h
+++ b/icu4c/source/test/intltest/numbertest.h
@@ -320,6 +320,7 @@ class NumberRangeFormatterTest : public IntlTestWithFieldPosition {
     void testGetDecimalNumbers();
     void test21684_Performance();
     void test21358_SignPosition();
+    void test21683_StateLeak();
 
     void runIndexedTest(int32_t index, UBool exec, const char *&name, char *par = 0) override;
 

diff --git a/icu4c/source/test/intltest/numbertest_range.cpp b/icu4c/source/test/intltest/numbertest_range.cpp
index 2f59f43ab6c67487879c9e2d5a304bab3faed9d4..08e74df8f528ccdacfcc15fd9d0d85f43fb90808 100644 (file)
--- a/icu4c/source/test/intltest/numbertest_range.cpp
+++ b/icu4c/source/test/intltest/numbertest_range.cpp
@@ -56,6 +56,7 @@ void NumberRangeFormatterTest::runIndexedTest(int32_t index, UBool exec, const c
         TESTCASE_AUTO(testGetDecimalNumbers);
         TESTCASE_AUTO(test21684_Performance);
         TESTCASE_AUTO(test21358_SignPosition);
+        TESTCASE_AUTO(test21683_StateLeak);
     TESTCASE_AUTO_END;
 }
 
@@ -1008,6 +1009,70 @@ void NumberRangeFormatterTest::test21358_SignPosition() {
     }
 }
 
+void NumberRangeFormatterTest::test21683_StateLeak() {
+    IcuTestErrorCode status(*this, "test21683_StateLeak");
+    UNumberRangeFormatter* nrf = nullptr;
+    UFormattedNumberRange* result = nullptr;
+    UConstrainedFieldPosition* fpos = nullptr;
+
+    struct Range {
+        double start;
+        double end;
+        const char16_t* expected;
+        int numFields;
+    } ranges[] = {
+        {1, 2, u"1\u20132", 4},
+        {1, 1, u"~1", 2},
+    };
+
+    UParseError* perror = nullptr;
+    nrf = unumrf_openForSkeletonWithCollapseAndIdentityFallback(
+        u"", -1,
+        UNUM_RANGE_COLLAPSE_AUTO,
+        UNUM_IDENTITY_FALLBACK_APPROXIMATELY,
+        "en", perror, status);
+    if (status.errIfFailureAndReset("unumrf_openForSkeletonWithCollapseAndIdentityFallback")) {
+        goto cleanup;
+    }
+
+    result = unumrf_openResult(status);
+    if (status.errIfFailureAndReset("unumrf_openResult")) { goto cleanup; }
+
+    for (auto range : ranges) {
+        unumrf_formatDoubleRange(nrf, range.start, range.end, result, status);
+        if (status.errIfFailureAndReset("unumrf_formatDoubleRange")) { goto cleanup; }
+
+        auto* formattedValue = unumrf_resultAsValue(result, status);
+        if (status.errIfFailureAndReset("unumrf_resultAsValue")) { goto cleanup; }
+
+        int32_t utf16Length;
+        const char16_t* utf16Str = ufmtval_getString(formattedValue, &utf16Length, status);
+        if (status.errIfFailureAndReset("ufmtval_getString")) { goto cleanup; }
+
+        assertEquals("Format", range.expected, utf16Str);
+
+        ucfpos_close(fpos);
+        fpos = ucfpos_open(status);
+        if (status.errIfFailureAndReset("ucfpos_open")) { goto cleanup; }
+
+        int numFields = 0;
+        while (true) {
+            bool hasMore = ufmtval_nextPosition(formattedValue, fpos, status);
+            if (status.errIfFailureAndReset("ufmtval_nextPosition")) { goto cleanup; }
+            if (!hasMore) {
+                break;
+            }
+            numFields++;
+        }
+        assertEquals("numFields", range.numFields, numFields);
+    }
+
+cleanup:
+    unumrf_close(nrf);
+    unumrf_closeResult(result);
+    ucfpos_close(fpos);
+}
+
 void  NumberRangeFormatterTest::assertFormatRange(
       const char16_t* message,
       const UnlocalizedNumberRangeFormatter& f,