From e318c0c374330906a561933cd54304a665d1b22e Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Tue, 12 Mar 2019 18:06:21 -0700 Subject: [PATCH] ICU-20494 Fixes to very large magnitude exponents in number parsing. - Do not depend on ArithmeticException string in ICU4J. - Return correct string in ICU4C. - Fix related issue in applyMaxInteger. --- icu4c/source/i18n/fmtable.cpp | 6 ++++- icu4c/source/i18n/number_decimalquantity.cpp | 6 +++++ icu4c/source/test/intltest/numbertest_api.cpp | 25 ++++++++++++++++++ icu4c/source/test/intltest/numfmtst.cpp | 26 +++++++++++++++++++ .../number/DecimalQuantity_AbstractBCD.java | 7 +++++ .../DecimalQuantity_DualStorageBCD.java | 16 ++++++------ .../icu/dev/test/format/NumberFormatTest.java | 26 +++++++++++++++++++ .../test/number/NumberFormatterApiTest.java | 25 ++++++++++++++++++ 8 files changed, 128 insertions(+), 9 deletions(-) diff --git a/icu4c/source/i18n/fmtable.cpp b/icu4c/source/i18n/fmtable.cpp index 82456f94df1..d813424494f 100644 --- a/icu4c/source/i18n/fmtable.cpp +++ b/icu4c/source/i18n/fmtable.cpp @@ -732,7 +732,11 @@ CharString *Formattable::internalGetCharString(UErrorCode &status) { // Older ICUs called uprv_decNumberToString here, which is not exactly the same as // DecimalQuantity::toScientificString(). The biggest difference is that uprv_decNumberToString does // not print scientific notation for magnitudes greater than -5 and smaller than some amount (+5?). - if (fDecimalQuantity->isZero()) { + if (fDecimalQuantity->isInfinite()) { + fDecimalStr->append("Infinity", status); + } else if (fDecimalQuantity->isNaN()) { + fDecimalStr->append("NaN", status); + } else if (fDecimalQuantity->isZero()) { fDecimalStr->append("0", -1, status); } else if (fType==kLong || fType==kInt64 || // use toPlainString for integer types (fDecimalQuantity->getMagnitude() != INT32_MIN && std::abs(fDecimalQuantity->getMagnitude()) < 5)) { diff --git a/icu4c/source/i18n/number_decimalquantity.cpp b/icu4c/source/i18n/number_decimalquantity.cpp index 2fde292a080..d899c276711 100644 --- a/icu4c/source/i18n/number_decimalquantity.cpp +++ b/icu4c/source/i18n/number_decimalquantity.cpp @@ -160,6 +160,11 @@ void DecimalQuantity::applyMaxInteger(int32_t maxInt) { return; } + if (maxInt <= scale) { + setBcdToZero(); + return; + } + int32_t magnitude = getMagnitude(); if (maxInt <= magnitude) { popFromLeft(magnitude - maxInt + 1); @@ -983,6 +988,7 @@ void DecimalQuantity::shiftRight(int32_t numDigits) { } void DecimalQuantity::popFromLeft(int32_t numDigits) { + U_ASSERT(numDigits <= precision); if (usingBytes) { int i = precision - 1; for (; i >= precision - numDigits; i--) { diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp index 902d27f1271..68a0e7ba1c5 100644 --- a/icu4c/source/test/intltest/numbertest_api.cpp +++ b/icu4c/source/test/intltest/numbertest_api.cpp @@ -1657,6 +1657,31 @@ void NumberFormatterApiTest::integerWidth() { u"00.08765", u"00.008765", u"00"); + + assertFormatSingle( + u"Integer Width Remove All A", + u"integer-width/00", + NumberFormatter::with().integerWidth(IntegerWidth::zeroFillTo(2).truncateAt(2)), + "en", + 2500, + u"00"); + + assertFormatSingle( + u"Integer Width Remove All B", + u"integer-width/00", + NumberFormatter::with().integerWidth(IntegerWidth::zeroFillTo(2).truncateAt(2)), + "en", + 25000, + u"00"); + + assertFormatSingle( + u"Integer Width Remove All B, Bytes Mode", + u"integer-width/00", + NumberFormatter::with().integerWidth(IntegerWidth::zeroFillTo(2).truncateAt(2)), + "en", + // Note: this double produces all 17 significant digits + 10000000000000002000.0, + u"00"); } void NumberFormatterApiTest::symbols() { diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp index 7dd66e580a1..5c0e8490767 100644 --- a/icu4c/source/test/intltest/numfmtst.cpp +++ b/icu4c/source/test/intltest/numfmtst.cpp @@ -9443,6 +9443,32 @@ void NumberFormatTest::Test20037_ScientificIntegerOverflow() { assertEquals(u"Should not overflow", u"3E-2147483648", {sp.data(), sp.length(), US_INV}); + + // Test largest parseable exponent + result = Formattable(); + nf->parse(u"9876e2147483643", result, status); + sp = result.getDecimalNumber(status); + assertEquals(u"Should not overflow", + u"9.876E+2147483646", + {sp.data(), sp.length(), US_INV}); + + // Test max value as well + const char16_t* infinityInputs[] = { + u"9876e2147483644", + u"9876e2147483645", + u"9876e2147483646", + u"9876e2147483647", + u"9876e2147483648", + u"9876e2147483649", + }; + for (const auto& input : infinityInputs) { + result = Formattable(); + nf->parse(input, result, status); + sp = result.getDecimalNumber(status); + assertEquals(UnicodeString("Should become Infinity: ") + input, + u"Infinity", + {sp.data(), sp.length(), US_INV}); + } } void NumberFormatTest::Test13840_ParseLongStringCrash() { diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java index 87434932a85..58a04c8452e 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java @@ -141,6 +141,11 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { return; } + if (maxInt <= scale) { + setBcdToZero(); + return; + } + int magnitude = getMagnitude(); if (maxInt <= magnitude) { popFromLeft(magnitude - maxInt + 1); @@ -205,6 +210,8 @@ public abstract class DecimalQuantity_AbstractBCD implements DecimalQuantity { if (precision != 0) { scale = Utility.addExact(scale, delta); origDelta = Utility.addExact(origDelta, delta); + // Make sure that precision + scale won't overflow, either + Utility.addExact(scale, precision); } } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java index 791538c9386..47f0d978c0b 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java @@ -156,6 +156,7 @@ public final class DecimalQuantity_DualStorageBCD extends DecimalQuantity_Abstra @Override protected void popFromLeft(int numDigits) { + assert numDigits <= precision; if (usingBytes) { int i = precision - 1; for (; i >= precision - numDigits; i--) { @@ -252,17 +253,16 @@ public final class DecimalQuantity_DualStorageBCD extends DecimalQuantity_Abstra tempLong = tempLong * 10 + getDigitPos(shift); } BigDecimal result = BigDecimal.valueOf(tempLong); - try { + // Test that the new scale fits inside the BigDecimal + long newScale = result.scale() + scale; + if (newScale <= Integer.MIN_VALUE) { + result = BigDecimal.ZERO; + } else { result = result.scaleByPowerOfTen(scale); - } catch (ArithmeticException e) { - if (e.getMessage().contains("Underflow")) { - result = BigDecimal.ZERO; - } else { - throw e; - } } - if (isNegative()) + if (isNegative()) { result = result.negate(); + } return result; } } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java index 0c76bd002e8..f6b57523149 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java @@ -6511,6 +6511,32 @@ public class NumberFormatTest extends TestFmwk { result = nf.parse(".0003e-2147483644"); assertEquals("Should not overflow", "0", result.toString()); + + // Test largest parseable exponent + // This is limited by ICU's BigDecimal implementation + result = nf.parse("1e999999999"); + assertEquals("Should not overflow", + "1E+999999999", result.toString()); + + // Test max value as well + String[] infinityInputs = { + "9876e1000000000", + "9876e2147483640", + "9876e2147483641", + "9876e2147483642", + "9876e2147483643", + "9876e2147483644", + "9876e2147483645", + "9876e2147483646", + "9876e2147483647", + "9876e2147483648", + "9876e2147483649", + }; + for (String input : infinityInputs) { + result = nf.parse(input); + assertEquals("Should become Infinity: " + input, + "Infinity", result.toString()); + } } @Test diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java index 9421097eca6..e0ad131b362 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java @@ -1591,6 +1591,31 @@ public class NumberFormatterApiTest { "00.08765", "00.008765", "00"); + + assertFormatSingle( + "Integer Width Remove All A", + "integer-width/00", + NumberFormatter.with().integerWidth(IntegerWidth.zeroFillTo(2).truncateAt(2)), + ULocale.ENGLISH, + 2500, + "00"); + + assertFormatSingle( + "Integer Width Remove All B", + "integer-width/00", + NumberFormatter.with().integerWidth(IntegerWidth.zeroFillTo(2).truncateAt(2)), + ULocale.ENGLISH, + 25000, + "00"); + + assertFormatSingle( + "Integer Width Remove All B, Bytes Mode", + "integer-width/00", + NumberFormatter.with().integerWidth(IntegerWidth.zeroFillTo(2).truncateAt(2)), + ULocale.ENGLISH, + // Note: this double produces all 17 significant digits + 10000000000000002000.0, + "00"); } @Test