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: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to