You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/06/15 19:48:57 UTC

[tomcat] branch 8.5.x updated: Use the standard utility method for List to comma separated string

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

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
     new 66f7cac  Use the standard utility method for List to comma separated string
66f7cac is described below

commit 66f7cac48f06b03c16d8933a69d24b4b8e59064d
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jun 15 20:46:27 2021 +0100

    Use the standard utility method for List to comma separated string
---
 java/org/apache/catalina/valves/RemoteIpValve.java |  9 ++++--
 .../apache/catalina/valves/TestRemoteIpValve.java  | 32 ++++------------------
 webapps/docs/changelog.xml                         |  8 ++++++
 3 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java b/java/org/apache/catalina/valves/RemoteIpValve.java
index 0dbd186..7655888 100644
--- a/java/org/apache/catalina/valves/RemoteIpValve.java
+++ b/java/org/apache/catalina/valves/RemoteIpValve.java
@@ -33,6 +33,7 @@ import org.apache.catalina.connector.Request;
 import org.apache.catalina.connector.Response;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.buf.StringUtils;
 import org.apache.tomcat.util.http.MimeHeaders;
 import org.apache.tomcat.util.http.parser.Host;
 
@@ -379,7 +380,11 @@ public class RemoteIpValve extends ValveBase {
      * Convert an array of strings in a comma delimited string
      * @param stringList The string list to convert
      * @return The concatenated string
+     *
+     * @deprecated Unused. This will be removed in Tomcat 10.1.x onwards.
+     *             Use {@link StringUtils#join(java.util.Collection)} instead
      */
+    @Deprecated
     protected static String listToCommaDelimitedString(List<String> stringList) {
         if (stringList == null) {
             return "";
@@ -680,13 +685,13 @@ public class RemoteIpValve extends ValveBase {
                 if (proxiesHeaderValue.size() == 0) {
                     request.getCoyoteRequest().getMimeHeaders().removeHeader(proxiesHeader);
                 } else {
-                    String commaDelimitedListOfProxies = listToCommaDelimitedString(proxiesHeaderValue);
+                    String commaDelimitedListOfProxies = StringUtils.join(proxiesHeaderValue);
                     request.getCoyoteRequest().getMimeHeaders().setValue(proxiesHeader).setString(commaDelimitedListOfProxies);
                 }
                 if (newRemoteIpHeaderValue.size() == 0) {
                     request.getCoyoteRequest().getMimeHeaders().removeHeader(remoteIpHeader);
                 } else {
-                    String commaDelimitedRemoteIpHeaderValue = listToCommaDelimitedString(newRemoteIpHeaderValue);
+                    String commaDelimitedRemoteIpHeaderValue = StringUtils.join(newRemoteIpHeaderValue);
                     request.getCoyoteRequest().getMimeHeaders().setValue(remoteIpHeader).setString(commaDelimitedRemoteIpHeaderValue);
                 }
             }
diff --git a/test/org/apache/catalina/valves/TestRemoteIpValve.java b/test/org/apache/catalina/valves/TestRemoteIpValve.java
index ae5783c..83cc4a8 100644
--- a/test/org/apache/catalina/valves/TestRemoteIpValve.java
+++ b/test/org/apache/catalina/valves/TestRemoteIpValve.java
@@ -110,26 +110,6 @@ public class TestRemoteIpValve {
     }
 
     @Test
-    public void testListToCommaDelimitedString() {
-        List<String> elements = Arrays.asList("element1", "element2", "element3");
-        String actual = RemoteIpValve.listToCommaDelimitedString(elements);
-        Assert.assertEquals("element1, element2, element3", actual);
-    }
-
-    @Test
-    public void testListToCommaDelimitedStringEmptyList() {
-        List<String> elements = new ArrayList<>();
-        String actual = RemoteIpValve.listToCommaDelimitedString(elements);
-        Assert.assertEquals("", actual);
-    }
-
-    @Test
-    public void testCommaDelimitedListToStringArrayNullList() {
-        String actual = RemoteIpValve.listToCommaDelimitedString(null);
-        Assert.assertEquals("", actual);
-    }
-
-    @Test
     public void testInvokeAllowedRemoteAddrWithNullRemoteIpHeader() throws Exception {
         // PREPARE
         RemoteIpValve remoteIpValve = new RemoteIpValve();
@@ -195,7 +175,7 @@ public class TestRemoteIpValve {
         Assert.assertNull("all proxies are trusted, x-forwarded-for must be null", actualXForwardedFor);
 
         String actualXForwardedBy = remoteAddrAndHostTrackerValve.getForwardedBy();
-        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1, proxy2", actualXForwardedBy);
+        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1,proxy2", actualXForwardedBy);
 
         String actualRemoteAddr = remoteAddrAndHostTrackerValve.getRemoteAddr();
         Assert.assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr);
@@ -236,7 +216,7 @@ public class TestRemoteIpValve {
         Assert.assertNull("all proxies are trusted, x-forwarded-for must be null", actualXForwardedFor);
 
         String actualXForwardedBy = remoteAddrAndHostTrackerValve.getForwardedBy();
-        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1, proxy2, proxy3", actualXForwardedBy);
+        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1,proxy2,proxy3", actualXForwardedBy);
 
         String actualRemoteAddr = remoteAddrAndHostTrackerValve.getRemoteAddr();
         Assert.assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr);
@@ -276,7 +256,7 @@ public class TestRemoteIpValve {
         Assert.assertNull("all proxies are trusted, x-forwarded-for must be null", actualXForwardedFor);
 
         String actualXForwardedBy = remoteAddrAndHostTrackerValve.getForwardedBy();
-        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1, proxy2, proxy3", actualXForwardedBy);
+        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1,proxy2,proxy3", actualXForwardedBy);
 
         String actualRemoteAddr = remoteAddrAndHostTrackerValve.getRemoteAddr();
         Assert.assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr);
@@ -318,7 +298,7 @@ public class TestRemoteIpValve {
         Assert.assertNull("all proxies are trusted, x-forwarded-for must be null", actualXForwardedFor);
 
         String actualXForwardedBy = remoteAddrAndHostTrackerValve.getForwardedBy();
-        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1, proxy2", actualXForwardedBy);
+        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1,proxy2", actualXForwardedBy);
 
         String actualRemoteAddr = remoteAddrAndHostTrackerValve.getRemoteAddr();
         Assert.assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr);
@@ -402,7 +382,7 @@ public class TestRemoteIpValve {
         Assert.assertNull("all proxies are trusted, x-forwarded-for must be null", actualXForwardedFor);
 
         String actualXForwardedBy = remoteAddrAndHostTrackerValve.getForwardedBy();
-        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1, proxy2", actualXForwardedBy);
+        Assert.assertEquals("all proxies are trusted, they must appear in x-forwarded-by", "proxy1,proxy2", actualXForwardedBy);
 
         String actualRemoteAddr = remoteAddrAndHostTrackerValve.getRemoteAddr();
         Assert.assertEquals("remoteAddr", "140.211.11.130", actualRemoteAddr);
@@ -1032,7 +1012,7 @@ public class TestRemoteIpValve {
 
         // VERIFY
         String actualXForwardedFor = remoteAddrAndHostTrackerValve.getForwardedFor();
-        Assert.assertEquals("ip/host before untrusted-proxy must appear in x-forwarded-for", "140.211.11.130, proxy1", actualXForwardedFor);
+        Assert.assertEquals("ip/host before untrusted-proxy must appear in x-forwarded-for", "140.211.11.130,proxy1", actualXForwardedFor);
 
         String actualXForwardedBy = remoteAddrAndHostTrackerValve.getForwardedBy();
         Assert.assertEquals("ip/host after untrusted-proxy must appear in  x-forwarded-by", "proxy2", actualXForwardedBy);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index eb34b00..e8874de 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,14 @@
   issues do not "pop up" wrt. others).
 -->
 <section name="Tomcat 8.5.69 (schultz)" rtext="in development">
+  <subsection name="Catalina">
+    <changelog>
+      <scode>
+        Refactor the <code>RemoteIpValve</code> to use the common utility method
+        for list to comma separated string conversion. (markt)
+      </scode>
+    </changelog>
+  </subsection>
   <subsection name="Coyote">
     <changelog>
       <fix>

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