From 7046e684b89fdae92fd5ea83fab12c5634cf3ba3 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Thu, 23 Mar 2023 21:50:47 -0700
Subject: [PATCH v8 1/4] Avoid potential UCollator leak for older ICU versions.

ICU versions 53 and earlier rely on icu_set_collation_attributes() to
process the attributes in the locale string. Refactor slightly to
avoid leaking the already-opened UCollator object if an error is
encountered.

Also centralize error reporting in pg_ucol_open() and make consistent
between versions.
---
 src/backend/utils/adt/pg_locale.c | 59 ++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 42b6ad45cb..f9bc30b85f 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -147,7 +147,8 @@ static size_t uchar_length(UConverter *converter,
 static int32_t uchar_convert(UConverter *converter,
 							 UChar *dest, int32_t destlen,
 							 const char *str, int32_t srclen);
-static void icu_set_collation_attributes(UCollator *collator, const char *loc);
+static void icu_set_collation_attributes(UCollator *collator, const char *loc,
+										 UErrorCode *status);
 #endif
 
 /*
@@ -2503,6 +2504,7 @@ pg_ucol_open(const char *loc_str)
 {
 	UCollator  *collator;
 	UErrorCode	status;
+	const char *orig_str = loc_str;
 	char	   *fixed_str = NULL;
 
 	/*
@@ -2552,11 +2554,27 @@ pg_ucol_open(const char *loc_str)
 	collator = ucol_open(loc_str, &status);
 	if (U_FAILURE(status))
 		ereport(ERROR,
+				/* use original string for error report */
 				(errmsg("could not open collator for locale \"%s\": %s",
-						loc_str, u_errorName(status))));
+						orig_str, u_errorName(status))));
 
 	if (U_ICU_VERSION_MAJOR_NUM < 54)
-		icu_set_collation_attributes(collator, loc_str);
+	{
+		status = U_ZERO_ERROR;
+		icu_set_collation_attributes(collator, loc_str, &status);
+
+		/*
+		 * Pretend the error came from ucol_open(), for consistent error
+		 * message across ICU versions.
+		 */
+		if (U_FAILURE(status))
+		{
+			ucol_close(collator);
+			ereport(ERROR,
+					(errmsg("could not open collator for locale \"%s\": %s",
+							orig_str, u_errorName(status))));
+		}
+	}
 
 	if (fixed_str != NULL)
 		pfree(fixed_str);
@@ -2706,9 +2724,9 @@ icu_from_uchar(char **result, const UChar *buff_uchar, int32_t len_uchar)
  */
 pg_attribute_unused()
 static void
-icu_set_collation_attributes(UCollator *collator, const char *loc)
+icu_set_collation_attributes(UCollator *collator, const char *loc,
+							 UErrorCode *status)
 {
-	UErrorCode	status;
 	int32_t		len;
 	char	   *icu_locale_id;
 	char	   *lower_str;
@@ -2721,15 +2739,15 @@ icu_set_collation_attributes(UCollator *collator, const char *loc)
 	 * locale ID, e.g. "und@colcaselevel=yes;colstrength=primary", by
 	 * uloc_canonicalize().
 	 */
-	status = U_ZERO_ERROR;
-	len = uloc_canonicalize(loc, NULL, 0, &status);
+	*status = U_ZERO_ERROR;
+	len = uloc_canonicalize(loc, NULL, 0, status);
 	icu_locale_id = palloc(len + 1);
-	status = U_ZERO_ERROR;
-	len = uloc_canonicalize(loc, icu_locale_id, len + 1, &status);
-	if (U_FAILURE(status))
+	*status = U_ZERO_ERROR;
+	len = uloc_canonicalize(loc, icu_locale_id, len + 1, status);
+	if (U_FAILURE(*status))
 		ereport(ERROR,
 				(errmsg("canonicalization failed for locale string \"%s\": %s",
-						loc, u_errorName(status))));
+						loc, u_errorName(*status))));
 
 	lower_str = asc_tolower(icu_locale_id, strlen(icu_locale_id));
 
@@ -2751,7 +2769,7 @@ icu_set_collation_attributes(UCollator *collator, const char *loc)
 			UColAttribute uattr;
 			UColAttributeValue uvalue;
 
-			status = U_ZERO_ERROR;
+			*status = U_ZERO_ERROR;
 
 			*e = '\0';
 			name = token;
@@ -2801,19 +2819,12 @@ icu_set_collation_attributes(UCollator *collator, const char *loc)
 			else if (strcmp(value, "upper") == 0)
 				uvalue = UCOL_UPPER_FIRST;
 			else
-				status = U_ILLEGAL_ARGUMENT_ERROR;
-
-			if (status == U_ZERO_ERROR)
-				ucol_setAttribute(collator, uattr, uvalue, &status);
+			{
+				*status = U_ILLEGAL_ARGUMENT_ERROR;
+				break;
+			}
 
-			/*
-			 * Pretend the error came from ucol_open(), for consistent error
-			 * message across ICU versions.
-			 */
-			if (U_FAILURE(status))
-				ereport(ERROR,
-						(errmsg("could not open collator for locale \"%s\": %s",
-								loc, u_errorName(status))));
+			ucol_setAttribute(collator, uattr, uvalue, status);
 		}
 	}
 
-- 
2.34.1