From bfee7c2da9a9578e2f8c75ee57e9223438e58aff Mon Sep 17 00:00:00 2001 From: GWphua Date: Tue, 11 Mar 2025 11:31:29 +0800 Subject: [PATCH 1/4] Update StringUtils.replace() after fix in JDK9 --- .../druid/java/util/common/StringUtils.java | 28 +------------------ 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java b/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java index d8ef9956d9bf..1cc03071a696 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java @@ -508,33 +508,7 @@ public static String replaceChar(String s, char c, String replacement) */ public static String replace(String s, String target, String replacement) { - // String.replace() is suboptimal in JDK8, but is fixed in JDK9+. When the minimal JDK version supported by Druid is - // JDK9+, the implementation of this method should be replaced with simple delegation to String.replace(). However, - // the method should still be prohibited to use in all other places except this method body, because it's easy to - // suboptimally call String.replace("a", "b"), String.replace("a", ""), String.replace("a", "abc"), which have - // better alternatives String.replace('a', 'b'), removeChar() and replaceChar() respectively. - int pos = s.indexOf(target); - if (pos < 0) { - return s; - } - int sLength = s.length(); - int targetLength = target.length(); - // This is needed to work correctly with empty target string and mimic String.replace() behavior - int searchSkip = Math.max(targetLength, 1); - StringBuilder sb = new StringBuilder(sLength - targetLength + replacement.length()); - int prevPos = 0; - do { - sb.append(s, prevPos, pos); - sb.append(replacement); - prevPos = pos + targetLength; - // Break from the loop if the target is empty - if (pos == sLength) { - break; - } - pos = s.indexOf(target, pos + searchSkip); - } while (pos > 0); - sb.append(s, prevPos, sLength); - return sb.toString(); + return s.replace(target, replacement); } /** From e17e64148c3f499f5d81e82399746f606d9b5369 Mon Sep 17 00:00:00 2001 From: GWphua Date: Tue, 11 Mar 2025 14:39:07 +0800 Subject: [PATCH 2/4] Upgrade optimized string replace algorithm --- .../druid/java/util/common/StringUtils.java | 42 ++++--------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java b/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java index 1cc03071a696..db5f590d3e59 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java @@ -20,6 +20,7 @@ package org.apache.druid.java.util.common; import com.google.common.base.Strings; +import io.netty.util.SuppressForbidden; import org.apache.commons.io.IOUtils; import javax.annotation.Nonnull; @@ -460,52 +461,27 @@ public static String maybeAppendTrailingSlash(String s) } /** - * Removes all occurrences of the given char from the given string. This method is an optimal version of - * {@link String#replace(CharSequence, CharSequence) s.replace("c", "")}. + * Removes all occurrences of the given char from the given string. */ + @SuppressForbidden(reason = "String#replace") public static String removeChar(String s, char c) { - int pos = s.indexOf(c); - if (pos < 0) { - return s; - } - StringBuilder sb = new StringBuilder(s.length() - 1); - int prevPos = 0; - do { - sb.append(s, prevPos, pos); - prevPos = pos + 1; - pos = s.indexOf(c, pos + 1); - } while (pos > 0); - sb.append(s, prevPos, s.length()); - return sb.toString(); + return s.replace(String.valueOf(c), ""); } /** - * Replaces all occurrences of the given char in the given string with the given replacement string. This method is an - * optimal version of {@link String#replace(CharSequence, CharSequence) s.replace("c", replacement)}. + * Replaces all occurrences of the given char in the given string with the given replacement string. */ + @SuppressForbidden(reason = "String#replace") public static String replaceChar(String s, char c, String replacement) { - int pos = s.indexOf(c); - if (pos < 0) { - return s; - } - StringBuilder sb = new StringBuilder(s.length() - 1 + replacement.length()); - int prevPos = 0; - do { - sb.append(s, prevPos, pos); - sb.append(replacement); - prevPos = pos + 1; - pos = s.indexOf(c, pos + 1); - } while (pos > 0); - sb.append(s, prevPos, s.length()); - return sb.toString(); + return s.replace(String.valueOf(c), replacement); } /** - * Replaces all occurrences of the given target substring in the given string with the given replacement string. This - * method is an optimal version of {@link String#replace(CharSequence, CharSequence) s.replace(target, replacement)}. + * Replaces all occurrences of the given target substring in the given string with the given replacement string. */ + @SuppressForbidden(reason = "String#replace") public static String replace(String s, String target, String replacement) { return s.replace(target, replacement); From 59ddb878ff2a5879dd5292da71b9e3173f3f0c09 Mon Sep 17 00:00:00 2001 From: GWphua Date: Wed, 12 Mar 2025 17:32:21 +0800 Subject: [PATCH 3/4] Update methods by re-using declared StringUtils#replace method --- .../java/org/apache/druid/java/util/common/StringUtils.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java b/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java index db5f590d3e59..29ace9dad706 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java @@ -463,19 +463,17 @@ public static String maybeAppendTrailingSlash(String s) /** * Removes all occurrences of the given char from the given string. */ - @SuppressForbidden(reason = "String#replace") public static String removeChar(String s, char c) { - return s.replace(String.valueOf(c), ""); + return StringUtils.replace(s, String.valueOf(c), ""); } /** * Replaces all occurrences of the given char in the given string with the given replacement string. */ - @SuppressForbidden(reason = "String#replace") public static String replaceChar(String s, char c, String replacement) { - return s.replace(String.valueOf(c), replacement); + return StringUtils.replace(s, String.valueOf(c), replacement); } /** From e7d081c24785e077614bc15335c36cafffc7494b Mon Sep 17 00:00:00 2001 From: GWphua Date: Mon, 17 Mar 2025 14:17:12 +0800 Subject: [PATCH 4/4] Replace hard-coded UTF-8 encodings with StandardCharsets --- .../druid/java/util/common/StringUtils.java | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java b/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java index 29ace9dad706..4c3f334636fa 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/StringUtils.java @@ -262,7 +262,7 @@ public static String fromUtf8(final ByteBuffer buffer) } /** - * If buffer is Decodes a UTF-8 string from the remaining bytes of a buffer. + * If buffer is not null, decodes a UTF-8 string from the remaining bytes of a buffer. * Advances the position of the buffer by {@link ByteBuffer#remaining()}. * * If the value is null, this method returns null. If the buffer will never be null, use {@link #fromUtf8(ByteBuffer)} @@ -422,12 +422,7 @@ public static String urlEncode(@Nullable String s) return null; } - try { - return StringUtils.replace(URLEncoder.encode(s, "UTF-8"), "+", "%20"); - } - catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); - } + return StringUtils.replace(URLEncoder.encode(s, StandardCharsets.UTF_8), "+", "%20"); } @Nullable @@ -437,12 +432,7 @@ public static String urlDecode(String s) return null; } - try { - return URLDecoder.decode(s, "UTF-8"); - } - catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); - } + return URLDecoder.decode(s, StandardCharsets.UTF_8); } public static String maybeRemoveLeadingSlash(String s)