From 596647c0c34bf19d90d7c90d4f3827876fef688f Mon Sep 17 00:00:00 2001
From: Frank Tang <ftang@chromium.org>
Date: Thu, 17 Oct 2019 22:42:14 +0000
Subject: [PATCH] ICU-20862 Fix setKeywordValue U_BUFFER_OVERFLOW_ERROR bug.

See #885
---
 icu4c/source/common/locid.cpp          | 26 +++++++++-
 icu4c/source/test/intltest/loctest.cpp | 68 ++++++++++++++++++++++++++
 icu4c/source/test/intltest/loctest.h   |  2 +
 3 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/icu4c/source/common/locid.cpp b/icu4c/source/common/locid.cpp
index c6d3f88fc3..c6c976394d 100644
--- a/icu4c/source/common/locid.cpp
+++ b/icu4c/source/common/locid.cpp
@@ -1340,7 +1340,31 @@ Locale::getUnicodeKeywordValue(StringPiece keywordName,
 void
 Locale::setKeywordValue(const char* keywordName, const char* keywordValue, UErrorCode &status)
 {
-    uloc_setKeywordValue(keywordName, keywordValue, fullName, ULOC_FULLNAME_CAPACITY, &status);
+    if (U_FAILURE(status)) {
+        return;
+    }
+    int32_t bufferLength = uprv_max((int32_t)(uprv_strlen(fullName) + 1), ULOC_FULLNAME_CAPACITY);
+    int32_t newLength = uloc_setKeywordValue(keywordName, keywordValue, fullName,
+                                             bufferLength, &status) + 1;
+    /* Handle the case the current buffer is not enough to hold the new id */
+    if (status == U_BUFFER_OVERFLOW_ERROR) {
+        U_ASSERT(newLength > bufferLength);
+        char* newFullName = (char *)uprv_malloc(newLength);
+        if (newFullName == nullptr) {
+            status = U_MEMORY_ALLOCATION_ERROR;
+            return;
+        }
+        uprv_strcpy(newFullName, fullName);
+        if (fullName != fullNameBuffer) {
+            // if full Name is already on the heap, need to free it.
+            uprv_free(fullName);
+        }
+        fullName = newFullName;
+        status = U_ZERO_ERROR;
+        uloc_setKeywordValue(keywordName, keywordValue, fullName, newLength, &status);
+    } else {
+        U_ASSERT(newLength <= bufferLength);
+    }
     if (U_SUCCESS(status) && baseName == fullName) {
         // May have added the first keyword, meaning that the fullName is no longer also the baseName.
         initBaseName(status);
diff --git a/icu4c/source/test/intltest/loctest.cpp b/icu4c/source/test/intltest/loctest.cpp
index 98612a1a37..73f1b73794 100644
--- a/icu4c/source/test/intltest/loctest.cpp
+++ b/icu4c/source/test/intltest/loctest.cpp
@@ -271,6 +271,8 @@ void LocaleTest::runIndexedTest( int32_t index, UBool exec, const char* &name, c
     TESTCASE_AUTO(TestPointerConvertingIterator);
     TESTCASE_AUTO(TestTagConvertingIterator);
     TESTCASE_AUTO(TestCapturingTagConvertingIterator);
+    TESTCASE_AUTO(TestSetUnicodeKeywordValueInLongLocale);
+    TESTCASE_AUTO(TestSetUnicodeKeywordValueNullInLongLocale);
     TESTCASE_AUTO_END;
 }
 
@@ -3952,3 +3954,69 @@ void LocaleTest::TestCapturingTagConvertingIterator() {
 
     assertFalse("3.hasNext()", iter.hasNext());
 }
+
+void LocaleTest::TestSetUnicodeKeywordValueInLongLocale() {
+    IcuTestErrorCode status(*this, "TestSetUnicodeKeywordValueInLongLocale");
+    const char* value = "efghijkl";
+    icu::Locale l("de");
+    char keyword[3];
+    CharString expected("de-u", status);
+    keyword[2] = '\0';
+    for (char i = 'a'; i < 's'; i++) {
+        keyword[0] = keyword[1] = i;
+        expected.append("-", status);
+        expected.append(keyword, status);
+        expected.append("-", status);
+        expected.append(value, status);
+        l.setUnicodeKeywordValue(keyword, value, status);
+        if (status.errIfFailureAndReset(
+            "setUnicodeKeywordValue(\"%s\", \"%s\") fail while locale is \"%s\"",
+            keyword, value, l.getName())) {
+            return;
+        }
+        std::string tag = l.toLanguageTag<std::string>(status);
+        if (status.errIfFailureAndReset(
+            "toLanguageTag fail on \"%s\"", l.getName())) {
+            return;
+        }
+        if (tag != expected.data()) {
+            errln("Expected to get \"%s\" bug got \"%s\"", tag.c_str(),
+                  expected.data());
+            return;
+        }
+    }
+}
+
+void LocaleTest::TestSetUnicodeKeywordValueNullInLongLocale() {
+    IcuTestErrorCode status(*this, "TestSetUnicodeKeywordValueNullInLongLocale");
+    const char *exts[] = {"cf", "cu", "em", "kk", "kr", "ks", "kv", "lb", "lw",
+      "ms", "nu", "rg", "sd", "ss", "tz"};
+    for (int32_t i = 0; i < UPRV_LENGTHOF(exts); i++) {
+        CharString tag("de-u", status);
+        for (int32_t j = 0; j <= i; j++) {
+            tag.append("-", status).append(exts[j], status);
+        }
+        if (status.errIfFailureAndReset(
+                "Cannot create tag \"%s\"", tag.data())) {
+            continue;
+        }
+        Locale l = Locale::forLanguageTag(tag.data(), status);
+        if (status.errIfFailureAndReset(
+                "Locale::forLanguageTag(\"%s\") failed", tag.data())) {
+            continue;
+        }
+        for (int32_t j = 0; j <= i; j++) {
+            l.setUnicodeKeywordValue(exts[j], nullptr, status);
+            if (status.errIfFailureAndReset(
+                    "Locale(\"%s\").setUnicodeKeywordValue(\"%s\", nullptr) failed",
+                    tag.data(), exts[j])) {
+                 continue;
+            }
+        }
+        if (strcmp("de", l.getName()) != 0) {
+            errln("setUnicodeKeywordValue should remove all extensions from "
+                  "\"%s\" and only have \"de\", but is \"%s\" instead.",
+                  tag.data(), l.getName());
+        }
+    }
+}
diff --git a/icu4c/source/test/intltest/loctest.h b/icu4c/source/test/intltest/loctest.h
index 065213e922..b62cd26016 100644
--- a/icu4c/source/test/intltest/loctest.h
+++ b/icu4c/source/test/intltest/loctest.h
@@ -145,6 +145,8 @@ public:
     void TestPointerConvertingIterator();
     void TestTagConvertingIterator();
     void TestCapturingTagConvertingIterator();
+    void TestSetUnicodeKeywordValueInLongLocale();
+    void TestSetUnicodeKeywordValueNullInLongLocale();
 
 private:
     void _checklocs(const char* label,