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

Also adds expectError to IcuTestErrorCode.
This commit is contained in:
Shane 2018-09-20 14:44:48 -07:00 committed by Shane Carr
parent f89a1d9d8a
commit 71ad5adf4a
No known key found for this signature in database
GPG key ID: FCED3B24AAB18B5C
6 changed files with 107 additions and 32 deletions

View file

@ -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;

View file

@ -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);
}
}

View file

@ -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);

View file

@ -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 */

View file

@ -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);

View file

@ -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