This is an automated email from the ASF dual-hosted git repository.

schultz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
     new 4c5f4b06b2 Unify comma-separated-value code and optimize the 
implementation (#707)
4c5f4b06b2 is described below

commit 4c5f4b06b202765940a7f8eaad1c6aaf59d942f8
Author: Christopher Schultz <ch...@christopherschultz.net>
AuthorDate: Sun Mar 17 10:33:00 2024 -0400

    Unify comma-separated-value code and optimize the implementation (#707)
    
    Add new StringUtils.splitCommaSeparated utility method which uses a simple 
search pattern for comma-separated values instead of a regular expression.
---
 java/org/apache/catalina/connector/Connector.java  |  3 ++-
 .../org/apache/catalina/filters/ExpiresFilter.java | 21 ++-----------------
 .../apache/catalina/filters/RemoteCIDRFilter.java  |  3 ++-
 .../apache/catalina/filters/RemoteIpFilter.java    | 21 ++-----------------
 java/org/apache/catalina/realm/JNDIRealm.java      |  6 ++++--
 java/org/apache/catalina/util/NetMaskSet.java      |  4 +++-
 .../apache/catalina/valves/RemoteCIDRValve.java    |  3 ++-
 java/org/apache/catalina/valves/RemoteIpValve.java | 22 ++------------------
 java/org/apache/tomcat/util/buf/StringUtils.java   | 24 ++++++++++++++++++++++
 .../catalina/filters/TestRemoteIpFilter.java       |  4 ++--
 .../apache/catalina/valves/TestRemoteIpValve.java  |  5 +++--
 11 files changed, 48 insertions(+), 68 deletions(-)

diff --git a/java/org/apache/catalina/connector/Connector.java 
b/java/org/apache/catalina/connector/Connector.java
index 240a83d614..fac5f33bf8 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -41,6 +41,7 @@ import org.apache.tomcat.util.IntrospectionUtils;
 import org.apache.tomcat.util.buf.B2CConverter;
 import org.apache.tomcat.util.buf.CharsetUtil;
 import org.apache.tomcat.util.buf.EncodedSolidusHandling;
+import org.apache.tomcat.util.buf.StringUtils;
 import org.apache.tomcat.util.compat.JreCompat;
 import org.apache.tomcat.util.net.SSLHostConfig;
 import org.apache.tomcat.util.net.openssl.OpenSSLImplementation;
@@ -528,7 +529,7 @@ public class Connector extends LifecycleMBeanBase {
         HashSet<String> methodSet = new HashSet<>();
 
         if (null != methods) {
-            methodSet.addAll(Arrays.asList(methods.split("\\s*,\\s*")));
+            
methodSet.addAll(Arrays.asList(StringUtils.splitCommaSeparated(methods)));
         }
 
         if (methodSet.contains("TRACE")) {
diff --git a/java/org/apache/catalina/filters/ExpiresFilter.java 
b/java/org/apache/catalina/filters/ExpiresFilter.java
index fe80faa521..6e1d9fdc9b 100644
--- a/java/org/apache/catalina/filters/ExpiresFilter.java
+++ b/java/org/apache/catalina/filters/ExpiresFilter.java
@@ -28,7 +28,6 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.StringTokenizer;
-import java.util.regex.Pattern;
 
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.FilterConfig;
@@ -44,6 +43,7 @@ import jakarta.servlet.http.MappingMatch;
 
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.buf.StringUtils;
 
 /**
  * <p>
@@ -1010,11 +1010,6 @@ public class ExpiresFilter extends FilterBase {
 
     }
 
-    /**
-     * {@link Pattern} for a comma delimited string that support whitespace 
characters
-     */
-    private static final Pattern commaSeparatedValuesPattern = 
Pattern.compile("\\s*,\\s*");
-
     private static final String HEADER_CACHE_CONTROL = "Cache-Control";
 
     private static final String HEADER_EXPIRES = "Expires";
@@ -1039,7 +1034,7 @@ public class ExpiresFilter extends FilterBase {
      * @return never {@code null} array
      */
     protected static int[] commaDelimitedListToIntArray(String 
commaDelimitedInts) {
-        String[] intsAsStrings = 
commaDelimitedListToStringArray(commaDelimitedInts);
+        String[] intsAsStrings = 
StringUtils.splitCommaSeparated(commaDelimitedInts);
         int[] ints = new int[intsAsStrings.length];
         for (int i = 0; i < intsAsStrings.length; i++) {
             String intAsString = intsAsStrings[i];
@@ -1053,18 +1048,6 @@ public class ExpiresFilter extends FilterBase {
         return ints;
     }
 
-    /**
-     * Convert a given comma delimited list of strings into an array of String
-     *
-     * @param commaDelimitedStrings the string to be split
-     *
-     * @return array of patterns (non {@code null})
-     */
-    protected static String[] commaDelimitedListToStringArray(String 
commaDelimitedStrings) {
-        return (commaDelimitedStrings == null || 
commaDelimitedStrings.length() == 0) ? new String[0] :
-                commaSeparatedValuesPattern.split(commaDelimitedStrings);
-    }
-
     /**
      * @return {@code true} if the given {@code str} contains the given {@code 
searchStr}.
      *
diff --git a/java/org/apache/catalina/filters/RemoteCIDRFilter.java 
b/java/org/apache/catalina/filters/RemoteCIDRFilter.java
index 2187c49bec..3c308356f3 100644
--- a/java/org/apache/catalina/filters/RemoteCIDRFilter.java
+++ b/java/org/apache/catalina/filters/RemoteCIDRFilter.java
@@ -33,6 +33,7 @@ import jakarta.servlet.http.HttpServletResponse;
 import org.apache.catalina.util.NetMask;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.buf.StringUtils;
 
 public final class RemoteCIDRFilter extends FilterBase {
 
@@ -221,7 +222,7 @@ public final class RemoteCIDRFilter extends FilterBase {
         final List<String> messages = new ArrayList<>();
         NetMask nm;
 
-        for (final String s : input.split("\\s*,\\s*")) {
+        for (final String s : StringUtils.splitCommaSeparated(input)) {
             try {
                 nm = new NetMask(s);
                 target.add(nm);
diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java 
b/java/org/apache/catalina/filters/RemoteIpFilter.java
index 0e3a3e74df..34af331dcf 100644
--- a/java/org/apache/catalina/filters/RemoteIpFilter.java
+++ b/java/org/apache/catalina/filters/RemoteIpFilter.java
@@ -640,11 +640,6 @@ public class RemoteIpFilter extends GenericFilter {
     }
 
 
-    /**
-     * {@link Pattern} for a comma delimited string that support whitespace 
characters
-     */
-    private static final Pattern commaSeparatedValuesPattern = 
Pattern.compile("\\s*,\\s*");
-
     protected static final String HTTP_SERVER_PORT_PARAMETER = 
"httpServerPort";
 
     protected static final String HTTPS_SERVER_PORT_PARAMETER = 
"httpsServerPort";
@@ -676,18 +671,6 @@ public class RemoteIpFilter extends GenericFilter {
 
     protected static final String ENABLE_LOOKUPS_PARAMETER = "enableLookups";
 
-    /**
-     * Convert a given comma delimited list of regular expressions into an 
array of String
-     *
-     * @param commaDelimitedStrings The string to split
-     *
-     * @return array of patterns (non <code>null</code>)
-     */
-    protected static String[] commaDelimitedListToStringArray(String 
commaDelimitedStrings) {
-        return (commaDelimitedStrings == null || 
commaDelimitedStrings.length() == 0) ? new String[0] :
-                commaSeparatedValuesPattern.split(commaDelimitedStrings);
-    }
-
     /**
      * @see #setHttpServerPort(int)
      */
@@ -764,7 +747,7 @@ public class RemoteIpFilter extends GenericFilter {
                 concatRemoteIpHeaderValue.append(e.nextElement());
             }
 
-            String[] remoteIpHeaderValue = 
commaDelimitedListToStringArray(concatRemoteIpHeaderValue.toString());
+            String[] remoteIpHeaderValue = 
StringUtils.splitCommaSeparated(concatRemoteIpHeaderValue.toString());
             int idx;
             if (!isInternal) {
                 proxiesHeaderValue.addFirst(request.getRemoteAddr());
@@ -898,7 +881,7 @@ public class RemoteIpFilter extends GenericFilter {
         if (!protocolHeaderValue.contains(",")) {
             return 
protocolHeaderHttpsValue.equalsIgnoreCase(protocolHeaderValue);
         }
-        String[] forwardedProtocols = 
commaDelimitedListToStringArray(protocolHeaderValue);
+        String[] forwardedProtocols = 
StringUtils.splitCommaSeparated(protocolHeaderValue);
         if (forwardedProtocols.length == 0) {
             return false;
         }
diff --git a/java/org/apache/catalina/realm/JNDIRealm.java 
b/java/org/apache/catalina/realm/JNDIRealm.java
index c4da4e27cd..aa4e0ac8ce 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -64,6 +64,7 @@ import javax.net.ssl.SSLSession;
 import javax.net.ssl.SSLSocketFactory;
 
 import org.apache.catalina.LifecycleException;
+import org.apache.tomcat.util.buf.StringUtils;
 import org.apache.tomcat.util.collections.SynchronizedStack;
 import org.ietf.jgss.GSSContext;
 import org.ietf.jgss.GSSCredential;
@@ -962,11 +963,12 @@ public class JNDIRealm extends RealmBase {
         if (cipherSuites == null || cipherSuitesArray != null) {
             return cipherSuitesArray;
         }
-        if (this.cipherSuites.trim().isEmpty()) {
+        this.cipherSuites = this.cipherSuites.trim();
+        if (this.cipherSuites.isEmpty()) {
             containerLog.warn(sm.getString("jndiRealm.emptyCipherSuites"));
             this.cipherSuitesArray = null;
         } else {
-            this.cipherSuitesArray = cipherSuites.trim().split("\\s*,\\s*");
+            this.cipherSuitesArray = 
StringUtils.splitCommaSeparated(cipherSuites);
             if (containerLog.isTraceEnabled()) {
                 containerLog.trace(sm.getString("jndiRealm.cipherSuites", 
Arrays.toString(this.cipherSuitesArray)));
             }
diff --git a/java/org/apache/catalina/util/NetMaskSet.java 
b/java/org/apache/catalina/util/NetMaskSet.java
index dfaf23fc11..6365dc8f85 100644
--- a/java/org/apache/catalina/util/NetMaskSet.java
+++ b/java/org/apache/catalina/util/NetMaskSet.java
@@ -25,6 +25,8 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
+import org.apache.tomcat.util.buf.StringUtils;
+
 
 /**
  * This class maintains a Set of NetMask objects and allows to check if a 
given IP address is matched by any of the
@@ -124,7 +126,7 @@ public class NetMaskSet {
 
         List<String> errMessages = new ArrayList<>();
 
-        for (String s : input.split("\\s*,\\s*")) {
+        for (String s : StringUtils.splitCommaSeparated(input)) {
             try {
                 this.add(s);
             } catch (IllegalArgumentException e) {
diff --git a/java/org/apache/catalina/valves/RemoteCIDRValve.java 
b/java/org/apache/catalina/valves/RemoteCIDRValve.java
index 9cd998aefb..7b9638beb9 100644
--- a/java/org/apache/catalina/valves/RemoteCIDRValve.java
+++ b/java/org/apache/catalina/valves/RemoteCIDRValve.java
@@ -30,6 +30,7 @@ import org.apache.catalina.connector.Response;
 import org.apache.catalina.util.NetMask;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.buf.StringUtils;
 
 public final class RemoteCIDRValve extends RequestFilterValve {
 
@@ -236,7 +237,7 @@ public final class RemoteCIDRValve extends 
RequestFilterValve {
         final List<String> messages = new ArrayList<>();
         NetMask nm;
 
-        for (final String s : input.split("\\s*,\\s*")) {
+        for (final String s : StringUtils.splitCommaSeparated(input)) {
             try {
                 nm = new NetMask(s);
                 target.add(nm);
diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java 
b/java/org/apache/catalina/valves/RemoteIpValve.java
index 13a745467f..27ee43cda4 100644
--- a/java/org/apache/catalina/valves/RemoteIpValve.java
+++ b/java/org/apache/catalina/valves/RemoteIpValve.java
@@ -357,29 +357,11 @@ import org.apache.tomcat.util.http.parser.Host;
  * </p>
  */
 public class RemoteIpValve extends ValveBase {
-
-    /**
-     * {@link Pattern} for a comma delimited string that support whitespace 
characters
-     */
-    private static final Pattern commaSeparatedValuesPattern = 
Pattern.compile("\\s*,\\s*");
-
     /**
      * Logger
      */
     private static final Log log = LogFactory.getLog(RemoteIpValve.class);
 
-    /**
-     * Convert a given comma delimited String into an array of String
-     *
-     * @param commaDelimitedStrings The string to convert
-     *
-     * @return array of String (non <code>null</code>)
-     */
-    protected static String[] commaDelimitedListToStringArray(String 
commaDelimitedStrings) {
-        return (commaDelimitedStrings == null || 
commaDelimitedStrings.length() == 0) ? new String[0]
-                : commaSeparatedValuesPattern.split(commaDelimitedStrings);
-    }
-
     private String hostHeader = null;
 
     private boolean changeLocalName = false;
@@ -608,7 +590,7 @@ public class RemoteIpValve extends ValveBase {
                 concatRemoteIpHeaderValue.append(e.nextElement());
             }
 
-            String[] remoteIpHeaderValue = 
commaDelimitedListToStringArray(concatRemoteIpHeaderValue.toString());
+            String[] remoteIpHeaderValue = 
StringUtils.splitCommaSeparated(concatRemoteIpHeaderValue.toString());
             int idx;
             if (!isInternal) {
                 proxiesHeaderValue.addFirst(originalRemoteAddr);
@@ -769,7 +751,7 @@ public class RemoteIpValve extends ValveBase {
         if (!protocolHeaderValue.contains(",")) {
             return 
protocolHeaderHttpsValue.equalsIgnoreCase(protocolHeaderValue);
         }
-        String[] forwardedProtocols = 
commaDelimitedListToStringArray(protocolHeaderValue);
+        String[] forwardedProtocols = 
StringUtils.splitCommaSeparated(protocolHeaderValue);
         if (forwardedProtocols.length == 0) {
             return false;
         }
diff --git a/java/org/apache/tomcat/util/buf/StringUtils.java 
b/java/org/apache/tomcat/util/buf/StringUtils.java
index a50535295a..f339021ce0 100644
--- a/java/org/apache/tomcat/util/buf/StringUtils.java
+++ b/java/org/apache/tomcat/util/buf/StringUtils.java
@@ -94,4 +94,28 @@ public final class StringUtils {
             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) {
+            splits[i] = splits[i].trim();
+        }
+
+        return splits;
+    }
 }
diff --git a/test/org/apache/catalina/filters/TestRemoteIpFilter.java 
b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
index c15023a62a..4effc5b3df 100644
--- a/test/org/apache/catalina/filters/TestRemoteIpFilter.java
+++ b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
@@ -638,7 +638,7 @@ public class TestRemoteIpFilter extends TomcatBaseTest {
 
     @Test
     public void testListToCommaDelimitedString() {
-        String[] actual = 
RemoteIpFilter.commaDelimitedListToStringArray("element1, element2, element3");
+        String[] actual = StringUtils.splitCommaSeparated("element1, element2, 
element3");
         String[] expected = new String[] { "element1", "element2", "element3" 
};
         Assert.assertEquals(expected.length, actual.length);
         for (int i = 0; i < actual.length; i++) {
@@ -648,7 +648,7 @@ public class TestRemoteIpFilter extends TomcatBaseTest {
 
     @Test
     public void testListToCommaDelimitedStringMixedSpaceChars() {
-        String[] actual = 
RemoteIpFilter.commaDelimitedListToStringArray("element1  , element2,\t 
element3");
+        String[] actual = StringUtils.splitCommaSeparated("element1  , 
element2,\t element3");
         String[] expected = new String[] { "element1", "element2", "element3" 
};
         Assert.assertEquals(expected.length, actual.length);
         for (int i = 0; i < actual.length; i++) {
diff --git a/test/org/apache/catalina/valves/TestRemoteIpValve.java 
b/test/org/apache/catalina/valves/TestRemoteIpValve.java
index f8c0b72a1b..41f934e37a 100644
--- a/test/org/apache/catalina/valves/TestRemoteIpValve.java
+++ b/test/org/apache/catalina/valves/TestRemoteIpValve.java
@@ -32,6 +32,7 @@ import org.apache.catalina.Globals;
 import org.apache.catalina.connector.Connector;
 import org.apache.catalina.connector.Request;
 import org.apache.catalina.connector.Response;
+import org.apache.tomcat.util.buf.StringUtils;
 
 /**
  * {@link RemoteIpValve} Tests
@@ -1031,14 +1032,14 @@ public class TestRemoteIpValve {
 
     @Test
     public void testCommaDelimitedListToStringArray() {
-        String[] actual = 
RemoteIpValve.commaDelimitedListToStringArray("element1, element2, element3");
+        String[] actual = StringUtils.splitCommaSeparated("element1, element2, 
element3");
         String[] expected = new String[] { "element1", "element2", "element3" 
};
         assertArrayEquals(expected, actual);
     }
 
     @Test
     public void testCommaDelimitedListToStringArrayMixedSpaceChars() {
-        String[] actual = 
RemoteIpValve.commaDelimitedListToStringArray("element1  , element2,\t 
element3");
+        String[] actual = StringUtils.splitCommaSeparated("element1  , 
element2,\t element3");
         String[] expected = new String[] { "element1", "element2", "element3" 
};
         assertArrayEquals(expected, actual);
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to