From: Shane F. Carr Date: Tue, 21 Sep 2021 07:56:49 +0000 (+0000) Subject: ICU-21683 ICU-21674 Clear state from UFormattedNumber[Range] in C API X-Git-Tag: release-70-rc~18 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=63637445d9702f096608733aef3881fa674bb76f;p=icu 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 4ff25dcfb87..2b9a3970d2e 100644 --- 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 9ec06daf3ea..70ffacac4b7 100644 --- 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 4cb56253628..b87dbd93e5f 100644 --- 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 a440a53fe6b..bd3a9ef5e82 100644 --- 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 75f1cca4c44..ddba7fb059c 100644 --- 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 f5a4ba75f1b..be6fe89eb15 100644 --- 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 2f59f43ab6c..08e74df8f52 100644 --- 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,