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