From 9ddda243d7d4fba4beb1868f6285f51369f6ceda Mon Sep 17 00:00:00 2001 From: Jeff Genovy <29107334+jefgen@users.noreply.github.com> Date: Mon, 9 Aug 2021 18:37:22 -0700 Subject: [PATCH] ICU-21705 ICU-21706 Fix crash if ICU's default locale has BCP47 Unicode Extensions, and fix ures_openDirect crash with NULL locale ID. Add test case for ures_openDirect with NULL locale ID. --- icu4c/source/common/uresbund.cpp | 26 +++++++++++++++++++------- icu4c/source/test/cintltst/crestst.c | 8 +++++++- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/icu4c/source/common/uresbund.cpp b/icu4c/source/common/uresbund.cpp index 18f474ea5fa..4ca3a348910 100644 --- a/icu4c/source/common/uresbund.cpp +++ b/icu4c/source/common/uresbund.cpp @@ -461,11 +461,10 @@ getPoolEntry(const char *path, UErrorCode *status) { /* INTERNAL: */ /* CAUTION: resbMutex must be locked when calling this function! */ static UResourceDataEntry * -findFirstExisting(const char* path, char* name, +findFirstExisting(const char* path, char* name, const char* defaultLocale, UBool *isRoot, UBool *hasChopped, UBool *isDefault, UErrorCode* status) { UResourceDataEntry *r = NULL; UBool hasRealData = FALSE; - const char *defaultLoc = uloc_getDefault(); *hasChopped = TRUE; /* we're starting with a fresh name */ while(*hasChopped && !hasRealData) { @@ -474,7 +473,7 @@ findFirstExisting(const char* path, char* name, if (U_FAILURE(*status)) { return NULL; } - *isDefault = (UBool)(uprv_strncmp(name, defaultLoc, uprv_strlen(name)) == 0); + *isDefault = (UBool)(uprv_strncmp(name, defaultLocale, uprv_strlen(name)) == 0); hasRealData = (UBool)(r->fBogus == U_ZERO_ERROR); if(!hasRealData) { /* this entry is not real. We will discard it. */ @@ -669,10 +668,13 @@ static UResourceDataEntry *entryOpen(const char* path, const char* localeID, } } + // Note: We need to query the default locale *before* locking resbMutex. + const char *defaultLocale = uloc_getDefault(); + Mutex lock(&resbMutex); // Lock resbMutex until the end of this function. /* We're going to skip all the locales that do not have any data */ - r = findFirstExisting(path, name, &isRoot, &hasChopped, &isDefault, &intStatus); + r = findFirstExisting(path, name, defaultLocale, &isRoot, &hasChopped, &isDefault, &intStatus); // If we failed due to out-of-memory, report the failure and exit early. if (intStatus == U_MEMORY_ALLOCATION_ERROR) { @@ -712,8 +714,8 @@ static UResourceDataEntry *entryOpen(const char* path, const char* localeID, /* if that is the case, we need to chain in the default locale */ if(r==NULL && openType == URES_OPEN_LOCALE_DEFAULT_ROOT && !isDefault && !isRoot) { /* insert default locale */ - uprv_strcpy(name, uloc_getDefault()); - r = findFirstExisting(path, name, &isRoot, &hasChopped, &isDefault, &intStatus); + uprv_strcpy(name, defaultLocale); + r = findFirstExisting(path, name, defaultLocale, &isRoot, &hasChopped, &isDefault, &intStatus); // If we failed due to out-of-memory, report the failure and exit early. if (intStatus == U_MEMORY_ALLOCATION_ERROR) { *status = intStatus; @@ -737,7 +739,7 @@ static UResourceDataEntry *entryOpen(const char* path, const char* localeID, /* present */ if(r == NULL) { uprv_strcpy(name, kRootLocaleName); - r = findFirstExisting(path, name, &isRoot, &hasChopped, &isDefault, &intStatus); + r = findFirstExisting(path, name, defaultLocale, &isRoot, &hasChopped, &isDefault, &intStatus); // If we failed due to out-of-memory, report the failure and exit early. if (intStatus == U_MEMORY_ALLOCATION_ERROR) { *status = intStatus; @@ -791,7 +793,17 @@ entryOpenDirect(const char* path, const char* localeID, UErrorCode* status) { return NULL; } + // Note: We need to query the default locale *before* locking resbMutex. + // If the localeID is NULL, then we want to use the default locale. + if (localeID == NULL) { + localeID = uloc_getDefault(); + } else if (*localeID == 0) { + // If the localeID is "", then we want to use the root locale. + localeID = kRootLocaleName; + } + Mutex lock(&resbMutex); + // findFirstExisting() without fallbacks. UResourceDataEntry *r = init_entry(localeID, path, status); if(U_SUCCESS(*status)) { diff --git a/icu4c/source/test/cintltst/crestst.c b/icu4c/source/test/cintltst/crestst.c index 080301090d1..d79ff8f1e50 100644 --- a/icu4c/source/test/cintltst/crestst.c +++ b/icu4c/source/test/cintltst/crestst.c @@ -496,7 +496,7 @@ static void TestFallback() static void TestOpenDirect(void) { - UResourceBundle *idna_rules, *casing, *te_IN, *ne, *item; + UResourceBundle *idna_rules, *casing, *te_IN, *ne, *item, *defaultLocale; UErrorCode errorCode; /* @@ -641,6 +641,12 @@ TestOpenDirect(void) { ures_close(item); } ures_close(te_IN); + + // ICU-21705 + // Verify that calling ures_openDirect() with a NULL localeID doesn't crash or assert. + errorCode = U_ZERO_ERROR; + defaultLocale = ures_openDirect(NULL, NULL, &errorCode); + ures_close(defaultLocale); } static void