From 85af08c09dd7cb406cf6a7f950b989763b8e1e1d Mon Sep 17 00:00:00 2001 From: Markus Scherer Date: Fri, 5 May 2017 23:48:40 +0000 Subject: [PATCH] ICU-13189 ucurr_forLocale() do not succeed without writing output; also make it more readable and fix other issues X-SVN-Rev: 40114 --- icu4c/source/common/ucurr.cpp | 161 +++++++++++++------------ icu4c/source/i18n/dcfmtsym.cpp | 13 +- icu4c/source/test/intltest/loctest.cpp | 13 ++ 3 files changed, 103 insertions(+), 84 deletions(-) diff --git a/icu4c/source/common/ucurr.cpp b/icu4c/source/common/ucurr.cpp index 885ca3a922..f2cae13ce3 100644 --- a/icu4c/source/common/ucurr.cpp +++ b/icu4c/source/common/ucurr.cpp @@ -25,6 +25,7 @@ #include "uenumimp.h" #include "uhash.h" #include "hash.h" +#include "uinvchar.h" #include "uresimp.h" #include "ulist.h" #include "ureslocs.h" @@ -545,93 +546,97 @@ U_CAPI int32_t U_EXPORT2 ucurr_forLocale(const char* locale, UChar* buff, int32_t buffCapacity, - UErrorCode* ec) -{ - int32_t resLen = 0; - const UChar* s = NULL; - if (ec != NULL && U_SUCCESS(*ec)) { - if ((buff && buffCapacity) || !buffCapacity) { - UErrorCode localStatus = U_ZERO_ERROR; - char id[ULOC_FULLNAME_CAPACITY]; - if ((resLen = uloc_getKeywordValue(locale, "currency", id, ULOC_FULLNAME_CAPACITY, &localStatus))) { - // there is a currency keyword. Try to see if it's valid - if(buffCapacity > resLen) { - /* Normalize the currency keyword value to upper case. */ - T_CString_toUpperCase(id); - u_charsToUChars(id, buff, resLen); - } - } else { - // get country or country_variant in `id' - uint32_t variantType = idForLocale(locale, id, sizeof(id), ec); + UErrorCode* ec) { + if (U_FAILURE(*ec)) { return 0; } + if (buffCapacity < 0 || (buff == nullptr && buffCapacity > 0)) { + *ec = U_ILLEGAL_ARGUMENT_ERROR; + return 0; + } - if (U_FAILURE(*ec)) { - return 0; - } + char currency[4]; // ISO currency codes are alpha3 codes. + UErrorCode localStatus = U_ZERO_ERROR; + int32_t resLen = uloc_getKeywordValue(locale, "currency", + currency, UPRV_LENGTHOF(currency), &localStatus); + if (U_SUCCESS(localStatus) && resLen == 3 && uprv_isInvariantString(currency, resLen)) { + if (resLen < buffCapacity) { + T_CString_toUpperCase(currency); + u_charsToUChars(currency, buff, resLen); + } + return u_terminateUChars(buff, buffCapacity, resLen, ec); + } + + // get country or country_variant in `id' + char id[ULOC_FULLNAME_CAPACITY]; + uint32_t variantType = idForLocale(locale, id, UPRV_LENGTHOF(id), ec); + if (U_FAILURE(*ec)) { + return 0; + } #if !UCONFIG_NO_SERVICE - const UChar* result = CReg::get(id); - if (result) { - if(buffCapacity > u_strlen(result)) { - u_strcpy(buff, result); - } - return u_strlen(result); - } + const UChar* result = CReg::get(id); + if (result) { + if(buffCapacity > u_strlen(result)) { + u_strcpy(buff, result); + } + resLen = u_strlen(result); + return u_terminateUChars(buff, buffCapacity, resLen, ec); + } #endif - // Remove variants, which is only needed for registration. - char *idDelim = strchr(id, VAR_DELIM); - if (idDelim) { - idDelim[0] = 0; - } + // Remove variants, which is only needed for registration. + char *idDelim = uprv_strchr(id, VAR_DELIM); + if (idDelim) { + idDelim[0] = 0; + } - // Look up the CurrencyMap element in the root bundle. - UResourceBundle *rb = ures_openDirect(U_ICUDATA_CURR, CURRENCY_DATA, &localStatus); - UResourceBundle *cm = ures_getByKey(rb, CURRENCY_MAP, rb, &localStatus); - UResourceBundle *countryArray = ures_getByKey(rb, id, cm, &localStatus); - UResourceBundle *currencyReq = ures_getByIndex(countryArray, 0, NULL, &localStatus); + const UChar* s; // Currency code from data file. + if (id[0] == 0) { + // No point looking in the data for an empty string. + // This is what we would get. + localStatus = U_MISSING_RESOURCE_ERROR; + } else { + // Look up the CurrencyMap element in the root bundle. + localStatus = U_ZERO_ERROR; + UResourceBundle *rb = ures_openDirect(U_ICUDATA_CURR, CURRENCY_DATA, &localStatus); + UResourceBundle *cm = ures_getByKey(rb, CURRENCY_MAP, rb, &localStatus); + UResourceBundle *countryArray = ures_getByKey(rb, id, cm, &localStatus); + UResourceBundle *currencyReq = ures_getByIndex(countryArray, 0, NULL, &localStatus); + s = ures_getStringByKey(currencyReq, "id", &resLen, &localStatus); + + // Get the second item when PREEURO is requested, and this is a known Euro country. + // If the requested variant is PREEURO, and this isn't a Euro country, + // assume that the country changed over to the Euro in the future. + // This is probably an old version of ICU that hasn't been updated yet. + // The latest currency is probably correct. + if (U_SUCCESS(localStatus)) { + if ((variantType & VARIANT_IS_PREEURO) && u_strcmp(s, EUR_STR) == 0) { + currencyReq = ures_getByIndex(countryArray, 1, currencyReq, &localStatus); s = ures_getStringByKey(currencyReq, "id", &resLen, &localStatus); - - /* - Get the second item when PREEURO is requested, and this is a known Euro country. - If the requested variant is PREEURO, and this isn't a Euro country, assume - that the country changed over to the Euro in the future. This is probably - an old version of ICU that hasn't been updated yet. The latest currency is - probably correct. - */ - if (U_SUCCESS(localStatus)) { - if ((variantType & VARIANT_IS_PREEURO) && u_strcmp(s, EUR_STR) == 0) { - currencyReq = ures_getByIndex(countryArray, 1, currencyReq, &localStatus); - s = ures_getStringByKey(currencyReq, "id", &resLen, &localStatus); - } - else if ((variantType & VARIANT_IS_EURO)) { - s = EUR_STR; - } - } - ures_close(countryArray); - ures_close(currencyReq); - - if ((U_FAILURE(localStatus)) && strchr(id, '_') != 0) - { - // We don't know about it. Check to see if we support the variant. - uloc_getParent(locale, id, sizeof(id), ec); - *ec = U_USING_FALLBACK_WARNING; - return ucurr_forLocale(id, buff, buffCapacity, ec); - } - else if (*ec == U_ZERO_ERROR || localStatus != U_ZERO_ERROR) { - // There is nothing to fallback to. Report the failure/warning if possible. - *ec = localStatus; - } - if (U_SUCCESS(*ec)) { - if(buffCapacity > resLen) { - u_strcpy(buff, s); - } - } + } else if ((variantType & VARIANT_IS_EURO)) { + s = EUR_STR; } - return u_terminateUChars(buff, buffCapacity, resLen, ec); - } else { - *ec = U_ILLEGAL_ARGUMENT_ERROR; + } + ures_close(currencyReq); + ures_close(countryArray); + } + + if ((U_FAILURE(localStatus)) && strchr(id, '_') != 0) { + // We don't know about it. Check to see if we support the variant. + uloc_getParent(locale, id, UPRV_LENGTHOF(id), ec); + *ec = U_USING_FALLBACK_WARNING; + // TODO: Loop over the shortened id rather than recursing and + // looking again for a currency keyword. + return ucurr_forLocale(id, buff, buffCapacity, ec); + } + if (*ec == U_ZERO_ERROR || localStatus != U_ZERO_ERROR) { + // There is nothing to fallback to. Report the failure/warning if possible. + *ec = localStatus; + } + if (U_SUCCESS(*ec)) { + if(buffCapacity > resLen) { + u_strcpy(buff, s); } } - return resLen; + return u_terminateUChars(buff, buffCapacity, resLen, ec); } // end registration diff --git a/icu4c/source/i18n/dcfmtsym.cpp b/icu4c/source/i18n/dcfmtsym.cpp index 08a85b545e..43eea49ea5 100644 --- a/icu4c/source/i18n/dcfmtsym.cpp +++ b/icu4c/source/i18n/dcfmtsym.cpp @@ -433,12 +433,13 @@ DecimalFormatSymbols::initialize(const Locale& loc, UErrorCode& status, UBool us UErrorCode internalStatus = U_ZERO_ERROR; // don't propagate failures out UChar curriso[4]; UnicodeString tempStr; - ucurr_forLocale(locStr, curriso, 4, &internalStatus); - - uprv_getStaticCurrencyName(curriso, locStr, tempStr, internalStatus); - if (U_SUCCESS(internalStatus)) { - fSymbols[kIntlCurrencySymbol].setTo(curriso, -1); - fSymbols[kCurrencySymbol] = tempStr; + int32_t currisoLength = ucurr_forLocale(locStr, curriso, UPRV_LENGTHOF(curriso), &internalStatus); + if (U_SUCCESS(internalStatus) && currisoLength == 3) { + uprv_getStaticCurrencyName(curriso, locStr, tempStr, internalStatus); + if (U_SUCCESS(internalStatus)) { + fSymbols[kIntlCurrencySymbol].setTo(curriso, currisoLength); + fSymbols[kCurrencySymbol] = tempStr; + } } /* else use the default values. */ diff --git a/icu4c/source/test/intltest/loctest.cpp b/icu4c/source/test/intltest/loctest.cpp index e849b26fa1..5bb2dc24dd 100644 --- a/icu4c/source/test/intltest/loctest.cpp +++ b/icu4c/source/test/intltest/loctest.cpp @@ -1257,6 +1257,19 @@ LocaleTest::TestEuroSupport() if (invalidLen || U_SUCCESS(status)) { errln("Fail: en_QQ didn't return NULL"); } + + // The currency keyword value is as long as the destination buffer. + // It should detect the overflow internally, and default to the locale's currency. + tmp[0] = u'¤'; + status = U_ZERO_ERROR; + int32_t length = ucurr_forLocale("en_US@currency=euro", tmp, 4, &status); + if (U_FAILURE(status) || dollarStr != UnicodeString(tmp, length)) { + if (U_SUCCESS(status) && tmp[0] == u'¤') { + errln("Fail: ucurr_forLocale(en_US@currency=euro) succeeded without writing output"); + } else { + errln("Fail: ucurr_forLocale(en_US@currency=euro) != USD - %s", u_errorName(status)); + } + } } #endif