Skip to content

Commit

Permalink
ICU-20558 Fix regression in DateTimePatternGenerator (Backport for 63)
Browse files Browse the repository at this point in the history
Backporting to ICU 63. This is manually cherry-picked from
commit: 693adf3

This fixes a regression introduced by commit
b12a927 for issue ICU-13778.

The above commit improved the error checking in the
DateTimePatternGenerator class, adding checks for errors/failures
where there previously was none at all. This was done in order to
catch catastrophic errors like out-of-memory (OOM), and properly
report them to the caller, rather than ignoring/hiding these errors.

However, in doing so it exposed a case where the code was depending
on ignoring errors in order to fall-back to the Gregorian calendar
when the default ICU locale is set to root.

This restores the previous behavior, by allowing the error of
U_MISSING_RESOURCE_ERROR to fall-though and continue without
reporting back an error to the caller.

Note: This regression was technically introduced in ICU 63, and
also effects ICU 64 as well.
  • Loading branch information
jefgen committed Apr 25, 2019
1 parent 4f715ae commit 5df4d7d
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 6 deletions.
14 changes: 9 additions & 5 deletions icu4c/source/i18n/dtptngen.cpp
Expand Up @@ -756,6 +756,7 @@ void
DateTimePatternGenerator::getCalendarTypeToUse(const Locale& locale, CharString& destination, UErrorCode& err) {
destination.clear().append(DT_DateTimeGregorianTag, -1, err); // initial default
if ( U_SUCCESS(err) ) {
UErrorCode localStatus = U_ZERO_ERROR;
char localeWithCalendarKey[ULOC_LOCALE_IDENTIFIER_CAPACITY];
// obtain a locale that always has the calendar key value that should be used
ures_getFunctionalEquivalent(
Expand All @@ -767,8 +768,7 @@ DateTimePatternGenerator::getCalendarTypeToUse(const Locale& locale, CharString&
locale.getName(),
nullptr,
FALSE,
&err);
if (U_FAILURE(err)) { return; }
&localStatus);
localeWithCalendarKey[ULOC_LOCALE_IDENTIFIER_CAPACITY-1] = 0; // ensure null termination
// now get the calendar key value from that locale
char calendarType[ULOC_KEYWORDS_CAPACITY];
Expand All @@ -777,13 +777,17 @@ DateTimePatternGenerator::getCalendarTypeToUse(const Locale& locale, CharString&
"calendar",
calendarType,
ULOC_KEYWORDS_CAPACITY,
&err);
if (U_FAILURE(err)) { return; }
&localStatus);
// If the input locale was invalid, don't fail with missing resource error, instead
// continue with default of Gregorian.
if (U_FAILURE(localStatus) && localStatus != U_MISSING_RESOURCE_ERROR) {
err = localStatus;
return;
}
if (calendarTypeLen < ULOC_KEYWORDS_CAPACITY) {
destination.clear().append(calendarType, -1, err);
if (U_FAILURE(err)) { return; }
}
err = U_ZERO_ERROR;
}
}

Expand Down
30 changes: 29 additions & 1 deletion icu4c/source/test/intltest/dtptngts.cpp
Expand Up @@ -20,9 +20,9 @@
#include "unicode/dtptngen.h"
#include "unicode/ustring.h"
#include "cmemory.h"
#include "cstring.h"
#include "loctest.h"


// This is an API test, not a unit test. It doesn't test very many cases, and doesn't
// try to test the full functionality. It just calls each function in the class and
// verifies that it works on a basic level.
Expand All @@ -38,6 +38,7 @@ void IntlTestDateTimePatternGeneratorAPI::runIndexedTest( int32_t index, UBool e
TESTCASE(4, testC);
TESTCASE(5, testSkeletonsWithDayPeriods);
TESTCASE(6, testGetFieldDisplayNames);
TESTCASE(7, testFallbackWithDefaultRootLocale);
default: name = ""; break;
}
}
Expand Down Expand Up @@ -1259,4 +1260,31 @@ void IntlTestDateTimePatternGeneratorAPI::testGetFieldDisplayNames() {
}
}

void IntlTestDateTimePatternGeneratorAPI::testFallbackWithDefaultRootLocale() {
UErrorCode status = U_ZERO_ERROR;
char original[ULOC_FULLNAME_CAPACITY];

uprv_strcpy(original, uloc_getDefault());
uloc_setDefault("root", &status);
if (U_FAILURE(status)) {
errln("ERROR: Failed to change the default locale to root! Default is: %s\n", uloc_getDefault());
}

DateTimePatternGenerator* dtpg = icu::DateTimePatternGenerator::createInstance("abcdedf", status);

if (U_FAILURE(status)) {
errln("ERROR: expected createInstance with invalid locale to succeed. Status: %s", u_errorName(status));
}
if (status != U_USING_DEFAULT_WARNING) {
errln("ERROR: expected createInstance to return U_USING_DEFAULT_WARNING for invalid locale and default root locale. Status: %s", u_errorName(status));
}

delete dtpg;

uloc_setDefault(original, &status);
if (U_FAILURE(status)) {
errln("ERROR: Failed to change the default locale back to %s\n", original);
}
}

#endif /* #if !UCONFIG_NO_FORMATTING */
1 change: 1 addition & 0 deletions icu4c/source/test/intltest/dtptngts.h
Expand Up @@ -32,6 +32,7 @@ class IntlTestDateTimePatternGeneratorAPI : public IntlTest {
void testC();
void testSkeletonsWithDayPeriods();
void testGetFieldDisplayNames();
void testFallbackWithDefaultRootLocale();
};

#endif /* #if !UCONFIG_NO_FORMATTING */
Expand Down

0 comments on commit 5df4d7d

Please sign in to comment.