mirror of
https://github.com/unicode-org/icu.git
synced 2025-04-13 08:53:20 +00:00
ICU-13847 ICU-20381 Improve handling of errors (Out-of-Memory) in DecimalFormat class.
- Use move assignment for fields->formatter (LocalizedNumberFormatter) instead of creating new heap object every time. - Add test cases for DecimalFormat object in invalid state. - Protect against self-assignment in assignment operator. - Fix segmentation fault when attempting to compare valid and invalid DecimalFormat objects. - Changes based on review feedback from Shane. - Fix minor typos in the public header file.
This commit is contained in:
Jeff Genovy
parent
48776d132a
commit
00596d3027
7 changed files with 778 additions and 70 deletions
File diff suppressed because it is too large
Load diff
|
|
@ -21,6 +21,7 @@ char kRawDefaultProperties[sizeof(DecimalFormatProperties)];
|
|||
icu::UInitOnce gDefaultPropertiesInitOnce = U_INITONCE_INITIALIZER;
|
||||
|
||||
void U_CALLCONV initDefaultProperties(UErrorCode&) {
|
||||
// can't fail, uses placement new into staticly allocated space.
|
||||
new(kRawDefaultProperties) DecimalFormatProperties(); // set to the default instance
|
||||
}
|
||||
|
||||
|
|
@ -142,4 +143,10 @@ bool DecimalFormatProperties::equalsDefaultExceptFastFormat() const {
|
|||
return _equals(*reinterpret_cast<DecimalFormatProperties*>(kRawDefaultProperties), true);
|
||||
}
|
||||
|
||||
const DecimalFormatProperties& DecimalFormatProperties::getDefault() {
|
||||
UErrorCode localStatus = U_ZERO_ERROR;
|
||||
umtx_initOnce(gDefaultPropertiesInitOnce, &initDefaultProperties, localStatus);
|
||||
return *reinterpret_cast<const DecimalFormatProperties*>(kRawDefaultProperties);
|
||||
}
|
||||
|
||||
#endif /* #if !UCONFIG_NO_FORMATTING */
|
||||
|
|
|
|||
|
|
@ -155,6 +155,11 @@ struct U_I18N_API DecimalFormatProperties : public UMemory {
|
|||
*/
|
||||
bool equalsDefaultExceptFastFormat() const;
|
||||
|
||||
/**
|
||||
* Returns the default DecimalFormatProperties instance.
|
||||
*/
|
||||
static const DecimalFormatProperties& getDefault();
|
||||
|
||||
private:
|
||||
bool _equals(const DecimalFormatProperties& other, bool ignoreForFastFormat) const;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -136,7 +136,7 @@ struct DecimalFormatFields : public UMemory {
|
|||
* The pre-computed formatter object. Setters cause this to be re-computed atomically. The {@link
|
||||
* #format} method uses the formatter directly without needing to synchronize.
|
||||
*/
|
||||
LocalPointer<const LocalizedNumberFormatter> formatter;
|
||||
LocalPointer<LocalizedNumberFormatter> formatter;
|
||||
|
||||
/** The lazy-computed parser for .parse() */
|
||||
std::atomic<::icu::numparse::impl::NumberParserImpl*> atomicParser = {};
|
||||
|
|
|
|||
|
|
@ -288,7 +288,7 @@ template class U_I18N_API EnumSet<UNumberFormatAttribute,
|
|||
* <td>Pad escape, precedes pad character
|
||||
* </table>
|
||||
*
|
||||
* <p>A DecimalFormat pattern contains a postive and negative
|
||||
* <p>A DecimalFormat pattern contains a positive and negative
|
||||
* subpattern, for example, "#,##0.00;(#,##0.00)". Each subpattern has a
|
||||
* prefix, a numeric part, and a suffix. If there is no explicit negative
|
||||
* subpattern, the negative subpattern is the localized minus sign prefixed to the
|
||||
|
|
@ -423,7 +423,7 @@ template class U_I18N_API EnumSet<UNumberFormatAttribute,
|
|||
*
|
||||
* <li>If the number of actual fraction digits is less than the
|
||||
* <em>minimum fraction digits</em>, then trailing zeros are added.
|
||||
* For example, 0.125 is formatted as "0.1250" if the mimimum fraction
|
||||
* For example, 0.125 is formatted as "0.1250" if the minimum fraction
|
||||
* digits is set to 4.
|
||||
*
|
||||
* <li>Trailing fractional zeros are not displayed if they occur
|
||||
|
|
@ -588,9 +588,9 @@ template class U_I18N_API EnumSet<UNumberFormatAttribute,
|
|||
* count of <code>getMaximumSignificantDigits() - 1</code>. For example, the
|
||||
* pattern <code>"@@###E0"</code> is equivalent to <code>"0.0###E0"</code>.
|
||||
*
|
||||
* <li>If signficant digits are in use, then the integer and fraction
|
||||
* <li>If significant digits are in use, then the integer and fraction
|
||||
* digit counts, as set via the API, are ignored. If significant
|
||||
* digits are not in use, then the signficant digit counts, as set via
|
||||
* digits are not in use, then the significant digit counts, as set via
|
||||
* the API, are ignored.
|
||||
*
|
||||
* </ul>
|
||||
|
|
@ -644,7 +644,7 @@ template class U_I18N_API EnumSet<UNumberFormatAttribute,
|
|||
* increment in the pattern itself. "#,#50" specifies a rounding increment of
|
||||
* 50. "#,##0.05" specifies a rounding increment of 0.05.
|
||||
*
|
||||
* <p>In the absense of an explicit rounding increment numbers are
|
||||
* <p>In the absence of an explicit rounding increment numbers are
|
||||
* rounded to their formatted width.
|
||||
*
|
||||
* <ul>
|
||||
|
|
@ -849,7 +849,7 @@ class U_I18N_API DecimalFormat : public NumberFormat {
|
|||
* @param pattern a non-localized pattern string
|
||||
* @param symbolsToAdopt the set of symbols to be used. The caller should not
|
||||
* delete this object after making this call.
|
||||
* @param parseError Output param to receive errors occured during parsing
|
||||
* @param parseError Output param to receive errors occurred during parsing
|
||||
* @param status Output param set to success/failure code. If the
|
||||
* pattern is invalid this will be set to a failure code.
|
||||
* @stable ICU 2.0
|
||||
|
|
@ -1127,7 +1127,7 @@ class U_I18N_API DecimalFormat : public NumberFormat {
|
|||
* does string comparisons to try to find an optimal match.
|
||||
* If no object can be parsed, index is unchanged, and NULL is
|
||||
* returned. The result is returned as the most parsimonious
|
||||
* type of Formattable that will accomodate all of the
|
||||
* type of Formattable that will accommodate all of the
|
||||
* necessary precision. For example, if the result is exactly 12,
|
||||
* it will be returned as a long. However, if it is 1.5, it will
|
||||
* be returned as a double.
|
||||
|
|
@ -1464,8 +1464,8 @@ class U_I18N_API DecimalFormat : public NumberFormat {
|
|||
* Set the character used to pad to the format width. If padding
|
||||
* is not enabled, then this will take effect if padding is later
|
||||
* enabled.
|
||||
* @param padChar a string containing the pad charcter. If the string
|
||||
* has length 0, then the pad characer is set to ' '. Otherwise
|
||||
* @param padChar a string containing the pad character. If the string
|
||||
* has length 0, then the pad character is set to ' '. Otherwise
|
||||
* padChar.char32At(0) will be used as the pad character.
|
||||
* @see #setFormatWidth
|
||||
* @see #getFormatWidth
|
||||
|
|
@ -2117,7 +2117,7 @@ class U_I18N_API DecimalFormat : public NumberFormat {
|
|||
/** Rebuilds the formatter object from the property bag. */
|
||||
void touch(UErrorCode& status);
|
||||
|
||||
/** Rebuilds the formatter object, hiding the error code. */
|
||||
/** Rebuilds the formatter object, ignoring any error code. */
|
||||
void touchNoError();
|
||||
|
||||
/**
|
||||
|
|
@ -2156,8 +2156,10 @@ class U_I18N_API DecimalFormat : public NumberFormat {
|
|||
// INSTANCE FIELDS //
|
||||
//=====================================================================================//
|
||||
|
||||
// Only one instance field: keep all fields inside of an implementation class defined in number_mapper.h
|
||||
number::impl::DecimalFormatFields* fields;
|
||||
|
||||
// One instance field for the implementation, keep all fields inside of an implementation
|
||||
// class defined in number_mapper.h
|
||||
number::impl::DecimalFormatFields* fields = nullptr;
|
||||
|
||||
// Allow child class CompactDecimalFormat to access fProperties:
|
||||
friend class CompactDecimalFormat;
|
||||
|
|
|
|||
|
|
@ -95,6 +95,12 @@ void IntlTestDecimalFormatAPI::runIndexedTest( int32_t index, UBool exec, const
|
|||
testErrorCode();
|
||||
}
|
||||
break;
|
||||
case 9: name = "testInvalidObject";
|
||||
if(exec) {
|
||||
logln((UnicodeString) "testInvalidObject ---");
|
||||
testInvalidObject();
|
||||
}
|
||||
break;
|
||||
default: name = ""; break;
|
||||
}
|
||||
}
|
||||
|
|
@ -1077,7 +1083,7 @@ void IntlTestDecimalFormatAPI::testErrorCode() {
|
|||
}
|
||||
|
||||
// Try each DecimalFormat method with an error code parameter, verifying that
|
||||
// an input error is not altered.
|
||||
// an input error is not altered, and that no segmentation faults occur.
|
||||
|
||||
status = U_INTERNAL_PROGRAM_ERROR;
|
||||
DecimalFormat dfBogus(status);
|
||||
|
|
@ -1140,4 +1146,238 @@ void IntlTestDecimalFormatAPI::testErrorCode() {
|
|||
}
|
||||
}
|
||||
|
||||
void IntlTestDecimalFormatAPI::testInvalidObject() {
|
||||
{
|
||||
UErrorCode status = U_INTERNAL_PROGRAM_ERROR;
|
||||
DecimalFormat dfBogus(status);
|
||||
assertEquals(WHERE, U_INTERNAL_PROGRAM_ERROR, status);
|
||||
|
||||
status = U_ZERO_ERROR;
|
||||
DecimalFormat dfGood(status);
|
||||
assertSuccess(WHERE, status);
|
||||
|
||||
// An invalid object should not be equal to a valid object.
|
||||
// This also tests that no segmentation fault occurs in the comparison operator due
|
||||
// to any dangling/nullptr pointers. (ICU-20381).
|
||||
assertTrue(WHERE, dfGood != dfBogus);
|
||||
|
||||
status = U_MEMORY_ALLOCATION_ERROR;
|
||||
DecimalFormat dfBogus2(status);
|
||||
assertEquals(WHERE, U_MEMORY_ALLOCATION_ERROR, status);
|
||||
|
||||
// Two invalid objects should not be equal.
|
||||
// (Also verify that nullptr isn't t dereferenced in the comparision operator.)
|
||||
assertTrue(WHERE, dfBogus != dfBogus2);
|
||||
|
||||
// Verify the comparison operator works for two valid objects.
|
||||
status = U_ZERO_ERROR;
|
||||
DecimalFormat dfGood2(status);
|
||||
assertSuccess(WHERE, status);
|
||||
assertTrue(WHERE, dfGood == dfGood2);
|
||||
|
||||
// Verify that the assignment operator sets the object to an invalid state, and
|
||||
// that no segmentation fault occurs due to any dangling/nullptr pointers.
|
||||
status = U_INTERNAL_PROGRAM_ERROR;
|
||||
DecimalFormat dfAssignmentBogus = DecimalFormat(status);
|
||||
// Verify comparison for the assigned object.
|
||||
assertTrue(WHERE, dfAssignmentBogus != dfGood);
|
||||
assertTrue(WHERE, dfAssignmentBogus != dfGood2);
|
||||
assertTrue(WHERE, dfAssignmentBogus != dfBogus);
|
||||
|
||||
// Verify that cloning our original invalid object gives nullptr.
|
||||
auto dfBogusClone = dfBogus.clone();
|
||||
assertTrue(WHERE, dfBogusClone == nullptr);
|
||||
// Verify that cloning our assigned invalid object gives nullptr.
|
||||
auto dfBogusClone2 = dfAssignmentBogus.clone();
|
||||
assertTrue(WHERE, dfBogusClone2 == nullptr);
|
||||
|
||||
// Verify copy constructing from an invalid object is also invalid.
|
||||
DecimalFormat dfCopy(dfBogus);
|
||||
assertTrue(WHERE, dfCopy != dfGood);
|
||||
assertTrue(WHERE, dfCopy != dfGood2);
|
||||
assertTrue(WHERE, dfCopy != dfBogus);
|
||||
DecimalFormat dfCopyAssign = dfBogus;
|
||||
assertTrue(WHERE, dfCopyAssign != dfGood);
|
||||
assertTrue(WHERE, dfCopyAssign != dfGood2);
|
||||
assertTrue(WHERE, dfCopyAssign != dfBogus);
|
||||
auto dfBogusCopyClone1 = dfCopy.clone();
|
||||
auto dfBogusCopyClone2 = dfCopyAssign.clone();
|
||||
assertTrue(WHERE, dfBogusCopyClone1 == nullptr);
|
||||
assertTrue(WHERE, dfBogusCopyClone2 == nullptr);
|
||||
}
|
||||
|
||||
{
|
||||
// Try each DecimalFormat class method that lacks an error code parameter, verifying
|
||||
// we don't crash (segmentation fault) on invalid objects.
|
||||
|
||||
UErrorCode status = U_ZERO_ERROR;
|
||||
const UnicodeString pattern(u"0.###E0");
|
||||
UParseError pe;
|
||||
DecimalFormatSymbols symbols(Locale::getUS(), status);
|
||||
assertSuccess(WHERE, status);
|
||||
CurrencyPluralInfo currencyPI(status);
|
||||
assertSuccess(WHERE, status);
|
||||
|
||||
status = U_INTERNAL_PROGRAM_ERROR;
|
||||
DecimalFormat dfBogus1(status);
|
||||
assertEquals(WHERE, U_INTERNAL_PROGRAM_ERROR, status);
|
||||
|
||||
status = U_INTERNAL_PROGRAM_ERROR;
|
||||
DecimalFormat dfBogus2(pattern, status);
|
||||
assertEquals(WHERE, U_INTERNAL_PROGRAM_ERROR, status);
|
||||
|
||||
status = U_INTERNAL_PROGRAM_ERROR;
|
||||
DecimalFormat dfBogus3(pattern, new DecimalFormatSymbols(symbols), status);
|
||||
assertEquals(WHERE, U_INTERNAL_PROGRAM_ERROR, status);
|
||||
|
||||
status = U_INTERNAL_PROGRAM_ERROR;
|
||||
DecimalFormat dfBogus4(pattern, new DecimalFormatSymbols(symbols), UNumberFormatStyle::UNUM_CURRENCY, status);
|
||||
assertEquals(WHERE, U_INTERNAL_PROGRAM_ERROR, status);
|
||||
|
||||
status = U_INTERNAL_PROGRAM_ERROR;
|
||||
DecimalFormat dfBogus5(pattern, new DecimalFormatSymbols(symbols), pe, status);
|
||||
assertEquals(WHERE, U_INTERNAL_PROGRAM_ERROR, status);
|
||||
|
||||
for (DecimalFormat *df : {&dfBogus1, &dfBogus2, &dfBogus3, &dfBogus4, &dfBogus5})
|
||||
{
|
||||
df->setGroupingUsed(true);
|
||||
|
||||
df->setParseIntegerOnly(false);
|
||||
|
||||
df->setLenient(true);
|
||||
|
||||
auto dfClone = df->clone();
|
||||
assertTrue(WHERE, dfClone == nullptr);
|
||||
|
||||
UnicodeString dest;
|
||||
FieldPosition fp;
|
||||
df->format(1.2, dest, fp);
|
||||
df->format(static_cast<int32_t>(1234), dest, fp);
|
||||
df->format(static_cast<int64_t>(1234), dest, fp);
|
||||
|
||||
UnicodeString text("-1,234.00");
|
||||
Formattable result;
|
||||
ParsePosition pos(0);
|
||||
df->parse(text, result, pos);
|
||||
|
||||
CurrencyAmount* ca = df->parseCurrency(text, pos);
|
||||
assertTrue(WHERE, ca == nullptr);
|
||||
|
||||
const DecimalFormatSymbols* dfs = df->getDecimalFormatSymbols();
|
||||
assertTrue(WHERE, dfs == nullptr);
|
||||
|
||||
df->adoptDecimalFormatSymbols(nullptr);
|
||||
|
||||
df->setDecimalFormatSymbols(symbols);
|
||||
|
||||
const CurrencyPluralInfo* cpi = df->getCurrencyPluralInfo();
|
||||
assertTrue(WHERE, cpi == nullptr);
|
||||
|
||||
df->adoptCurrencyPluralInfo(nullptr);
|
||||
|
||||
df->setCurrencyPluralInfo(currencyPI);
|
||||
|
||||
UnicodeString prefix("-123");
|
||||
df->getPositivePrefix(dest);
|
||||
df->setPositivePrefix(prefix);
|
||||
df->getNegativePrefix(dest);
|
||||
df->setNegativePrefix(prefix);
|
||||
df->getPositiveSuffix(dest);
|
||||
df->setPositiveSuffix(prefix);
|
||||
df->getNegativeSuffix(dest);
|
||||
df->setNegativeSuffix(prefix);
|
||||
|
||||
df->isSignAlwaysShown();
|
||||
|
||||
df->setSignAlwaysShown(true);
|
||||
|
||||
df->getMultiplier();
|
||||
df->setMultiplier(10);
|
||||
|
||||
df->getMultiplierScale();
|
||||
df->setMultiplierScale(2);
|
||||
|
||||
df->getRoundingIncrement();
|
||||
df->setRoundingIncrement(1.2);
|
||||
|
||||
df->getRoundingMode();
|
||||
df->setRoundingMode(DecimalFormat::ERoundingMode::kRoundDown);
|
||||
|
||||
df->getFormatWidth();
|
||||
df->setFormatWidth(0);
|
||||
|
||||
UnicodeString pad(" ");
|
||||
df->getPadCharacterString();
|
||||
df->setPadCharacter(pad);
|
||||
|
||||
df->getPadPosition();
|
||||
df->setPadPosition(DecimalFormat::EPadPosition::kPadBeforePrefix);
|
||||
|
||||
df->isScientificNotation();
|
||||
df->setScientificNotation(false);
|
||||
|
||||
df->getMinimumExponentDigits();
|
||||
df->setMinimumExponentDigits(1);
|
||||
|
||||
df->isExponentSignAlwaysShown();
|
||||
df->setExponentSignAlwaysShown(true);
|
||||
|
||||
df->getGroupingSize();
|
||||
df->setGroupingSize(3);
|
||||
|
||||
df->getSecondaryGroupingSize();
|
||||
df->setSecondaryGroupingSize(-1);
|
||||
|
||||
df->getMinimumGroupingDigits();
|
||||
df->setMinimumGroupingDigits(-1);
|
||||
|
||||
df->isDecimalSeparatorAlwaysShown();
|
||||
df->setDecimalSeparatorAlwaysShown(true);
|
||||
|
||||
df->isDecimalPatternMatchRequired();
|
||||
df->setDecimalPatternMatchRequired(false);
|
||||
|
||||
df->isParseNoExponent();
|
||||
df->setParseNoExponent(true);
|
||||
|
||||
df->isParseCaseSensitive();
|
||||
df->setParseCaseSensitive(false);
|
||||
|
||||
df->isFormatFailIfMoreThanMaxDigits();
|
||||
df->setFormatFailIfMoreThanMaxDigits(true);
|
||||
|
||||
df->toPattern(dest);
|
||||
df->toLocalizedPattern(dest);
|
||||
|
||||
df->setMaximumIntegerDigits(10);
|
||||
df->setMinimumIntegerDigits(0);
|
||||
|
||||
df->setMaximumFractionDigits(2);
|
||||
df->setMinimumFractionDigits(0);
|
||||
|
||||
df->getMinimumSignificantDigits();
|
||||
df->setMinimumSignificantDigits(0);
|
||||
|
||||
df->getMaximumSignificantDigits();
|
||||
df->setMaximumSignificantDigits(5);
|
||||
|
||||
df->areSignificantDigitsUsed();
|
||||
df->setSignificantDigitsUsed(true);
|
||||
|
||||
df->setCurrency(u"USD");
|
||||
|
||||
df->getCurrencyUsage();
|
||||
|
||||
// TODO: See ICU-20366 and ICU-20380.
|
||||
// This method should return a pointer, or at least have an error code parameter,
|
||||
const number::LocalizedNumberFormatter& lnf = df->toNumberFormatter();
|
||||
(void)lnf; // suppress unused variable warning.
|
||||
// Note: The existance of a null reference is undefined behavior in C++.
|
||||
// Attempting to touch/examine a null reference is also undefined. Doing
|
||||
// so generally will cause a crash or segmentation fault.
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
#endif /* #if !UCONFIG_NO_FORMATTING */
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ public:
|
|||
void TestBadFastpath();
|
||||
void TestRequiredDecimalPoint();
|
||||
void testErrorCode();
|
||||
void testInvalidObject();
|
||||
private:
|
||||
/*Helper functions */
|
||||
void verify(const UnicodeString& message, const UnicodeString& got, double expected);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue