Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ICU-20065 Prevent crash on Collator::makeInstance fail when optimized… #54

Merged
merged 2 commits into from
Aug 13, 2018
Merged

ICU-20065 Prevent crash on Collator::makeInstance fail when optimized… #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8e7c9a
ICU-20065 Prevent crash on Collator::makeInstance fail when optimized…
pedberg Aug 12, 2018
c71b9e0
ICU-20065 Fix bad wording in comment
pedberg Aug 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions icu4c/source/i18n/coll.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,13 @@ Collator* U_EXPORT2 Collator::createInstance(const Locale& desiredLocale,
#endif
{
coll = makeInstance(desiredLocale, status);
// Either returns NULL with U_FAILURE(status), or non-NULL with U_SUCCESS(status)
}
// The use of *coll in setAttributesFromKeywords can cause the NULL check to be
// optimized out of the delete even though setAttributesFromKeywords returns
// immediately if U_FAILURE(status), so we add a check here.
if (U_FAILURE(status)) {
return NULL;
}
setAttributesFromKeywords(desiredLocale, *coll, status);
if (U_FAILURE(status)) {
Expand Down
35 changes: 35 additions & 0 deletions icu4c/source/test/cintltst/callcoll.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ static void TestFCDCrash(void);

static void TestJ5298(void);

static void TestBadKey(void);

const UCollationResult results[] = {
UCOL_LESS,
UCOL_LESS, /*UCOL_GREATER,*/
Expand Down Expand Up @@ -210,6 +212,7 @@ void addAllCollTest(TestNode** root)
addTest(root, &TestJitterbug1098, "tscoll/callcoll/TestJitterbug1098");
addTest(root, &TestFCDCrash, "tscoll/callcoll/TestFCDCrash");
addTest(root, &TestJ5298, "tscoll/callcoll/TestJ5298");
addTest(root, &TestBadKey, "tscoll/callcoll/TestBadKey");
}

UBool hasCollationElements(const char *locName) {
Expand Down Expand Up @@ -1343,4 +1346,36 @@ static void TestJ5298(void)
uenum_close(values);
log_verbose("\n");
}

static const char* badKeyLocales[] = {
"@calendar=japanese;collation=search", // ucol_open OK
"@calendar=japanese", // ucol_open OK
"en@calendar=x", // ucol_open OK
"ja@calendar=x", // ucol_open OK
"en@collation=x", // ucol_open OK
"ja@collation=x", // ucol_open OK
"ja@collation=private-kana", // ucol_open fails, verify it does not crash
"en@collation=\x80", // (x80 undef in ASCII,EBCDIC) ucol_open fails, verify it does not crash
NULL
};

// Mainly this is to check that we don't have a crash, but we check
// for correct NULL return and FAILURE/SUCCESS status as a bonus.
static void TestBadKey(void)
{
const char* badLoc;
const char** badLocsPtr = badKeyLocales;
while ((badLoc = *badLocsPtr++) != NULL) {
UErrorCode status = U_ZERO_ERROR;
UCollator* uc = ucol_open(badLoc, &status);
if ( U_SUCCESS(status) ) {
if (uc == NULL) {
log_err("ucol_open sets SUCCESS but returns NULL, locale: %s\n", badLoc);
}
ucol_close(uc);
} else if (uc != NULL) {
log_err("ucol_open sets FAILURE but returns non-NULL, locale: %s\n", badLoc);
}
}
}
#endif /* #if !UCONFIG_NO_COLLATION */