Mark,
On 3/15/24 13:27, markt-asf (via GitHub) wrote:
markt-asf commented on code in PR #707:
URL: https://github.com/apache/tomcat/pull/707#discussion_r1526610303
##########
java/org/apache/catalina/realm/JNDIRealm.java:
##########
@@ -966,7 +967,7 @@ private String[] getCipherSuitesArray() {
containerLog.warn(sm.getString("jndiRealm.emptyCipherSuites"));
this.cipherSuitesArray = null;
} else {
- this.cipherSuitesArray = cipherSuites.trim().split("\\s*,\\s*");
+ this.cipherSuitesArray =
StringUtils.splitCommaSeparated(cipherSuites.trim());
if (containerLog.isTraceEnabled()) {
Review Comment:
Is this trim necessary here? I know it was there before but it looks
redundant.
Looking back, it does make sense to remove this. I was thinking that as
I wrote it, but the trim was technically necessary before the change
from regex to split-then-trim.
I'll shave off this sharp edge.
##########
java/org/apache/tomcat/util/buf/StringUtils.java:
##########
@@ -94,4 +94,28 @@ public static <T> void join(Iterable<T> iterable, char
separator, Function<T,Str
sb.append(function.apply(value));
}
}
+
+ /**
+ * Splits a comma-separated string into an array of String values.
+ *
+ * Whitespace around the commas is removed.
+ *
+ * Null or empty values will return a zero-element array.
+ *
+ * @param s The string to split by commas.
+ *
+ * @return An array of String values.
+ */
+ public static String[] splitCommaSeparated(String s) {
+ if (s == null || s.length() == 0) {
+ return new String[0];
+ }
+
+ String[] splits = s.split(",");
+ for (int i=0; i<splits.length; ++i) {
Review Comment:
The formatting police are going to complain about spaces around operators
here.
Checkstyle was happy enough. :)
I'll get that corrected.
-chris
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
