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 2020/12/02 18:52:58 UTC

[tomcat] branch 9.0.x updated: Ensure values are not duplicated when manipulating the vary header

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

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


The following commit(s) were added to refs/heads/9.0.x by this push:
     new a4e061e  Ensure values are not duplicated when manipulating the vary header
a4e061e is described below

commit a4e061e8364ac3413082532d84431e6c896a6127
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Dec 2 18:51:26 2020 +0000

    Ensure values are not duplicated when manipulating the vary header
---
 java/org/apache/tomcat/util/http/ResponseUtil.java | 14 +++---
 .../apache/tomcat/util/http/TestResponseUtil.java  | 50 +++++++++++-----------
 webapps/docs/changelog.xml                         |  4 ++
 3 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/java/org/apache/tomcat/util/http/ResponseUtil.java b/java/org/apache/tomcat/util/http/ResponseUtil.java
index 295e7b7..cdc6ceb 100644
--- a/java/org/apache/tomcat/util/http/ResponseUtil.java
+++ b/java/org/apache/tomcat/util/http/ResponseUtil.java
@@ -21,9 +21,9 @@ import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Enumeration;
-import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 
 import javax.servlet.http.HttpServletResponse;
 
@@ -76,7 +76,7 @@ public class ResponseUtil {
         // the existing values, check if the new value is already present and
         // then add it if not. The good news is field names are tokens which
         // makes parsing simpler.
-        Set<String> fieldNames = new HashSet<>();
+        LinkedHashSet<String> fieldNames = new LinkedHashSet<>();
 
         for (String varyHeader : varyHeaders) {
             StringReader input = new StringReader(varyHeader);
@@ -97,10 +97,12 @@ public class ResponseUtil {
         // Replace existing header(s) to ensure any invalid values are removed
         fieldNames.add(name);
         StringBuilder varyHeader = new StringBuilder();
-        varyHeader.append(name);
-        for (String fieldName : fieldNames) {
+        Iterator<String> iter = fieldNames.iterator();
+        // There must be at least one value as one is added just above
+        varyHeader.append(iter.next());
+        while (iter.hasNext()) {
             varyHeader.append(',');
-            varyHeader.append(fieldName);
+            varyHeader.append(iter.next());
         }
         adapter.setHeader(VARY_HEADER, varyHeader.toString());
     }
diff --git a/test/org/apache/tomcat/util/http/TestResponseUtil.java b/test/org/apache/tomcat/util/http/TestResponseUtil.java
index 826cac8..4c760e0 100644
--- a/test/org/apache/tomcat/util/http/TestResponseUtil.java
+++ b/test/org/apache/tomcat/util/http/TestResponseUtil.java
@@ -16,8 +16,8 @@
  */
 package org.apache.tomcat.util.http;
 
-import java.util.HashSet;
-import java.util.Set;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.junit.Assert;
 import org.junit.Test;
@@ -31,7 +31,7 @@ public class TestResponseUtil {
         TesterResponse response = new TesterResponse();
         response.getCoyoteResponse();
         response.addHeader("vary", "host");
-        Set<String> expected = new HashSet<>();
+        List<String> expected = new ArrayList<>();
         expected.add("*");
         doTestAddVaryFieldName(response, "*", expected);
     }
@@ -42,7 +42,7 @@ public class TestResponseUtil {
         TesterResponse response = new TesterResponse();
         response.getCoyoteResponse();
         response.addHeader("vary", "*");
-        Set<String> expected = new HashSet<>();
+        List<String> expected = new ArrayList<>();
         expected.add("*");
         doTestAddVaryFieldName(response, "*", expected);
     }
@@ -52,7 +52,7 @@ public class TestResponseUtil {
     public void testAddAllWithNone() {
         TesterResponse response = new TesterResponse();
         response.getCoyoteResponse();
-        Set<String> expected = new HashSet<>();
+        List<String> expected = new ArrayList<>();
         expected.add("*");
         doTestAddVaryFieldName(response, "*", expected);
     }
@@ -63,9 +63,9 @@ public class TestResponseUtil {
         TesterResponse response = new TesterResponse();
         response.getCoyoteResponse();
         response.addHeader("vary", "foo, bar");
-        Set<String> expected = new HashSet<>();
-        expected.add("bar");
+        List<String> expected = new ArrayList<>();
         expected.add("foo");
+        expected.add("bar");
         expected.add("too");
         doTestAddVaryFieldName(response, "too", expected);
     }
@@ -76,7 +76,7 @@ public class TestResponseUtil {
         TesterResponse response = new TesterResponse();
         response.getCoyoteResponse();
         response.addHeader("vary", "foo, *");
-        Set<String> expected = new HashSet<>();
+        List<String> expected = new ArrayList<>();
         expected.add("*");
         doTestAddVaryFieldName(response, "too", expected);
     }
@@ -87,9 +87,9 @@ public class TestResponseUtil {
         TesterResponse response = new TesterResponse();
         response.getCoyoteResponse();
         response.addHeader("vary", "foo, bar");
-        Set<String> expected = new HashSet<>();
-        expected.add("bar");
+        List<String> expected = new ArrayList<>();
         expected.add("foo");
+        expected.add("bar");
         doTestAddVaryFieldName(response, "foo", expected);
     }
 
@@ -100,9 +100,9 @@ public class TestResponseUtil {
         response.getCoyoteResponse();
         response.addHeader("vary", "foo");
         response.addHeader("vary", "bar");
-        Set<String> expected = new HashSet<>();
-        expected.add("bar");
+        List<String> expected = new ArrayList<>();
         expected.add("foo");
+        expected.add("bar");
         expected.add("too");
         doTestAddVaryFieldName(response, "too", expected);
     }
@@ -114,7 +114,7 @@ public class TestResponseUtil {
         response.getCoyoteResponse();
         response.addHeader("vary", "foo");
         response.addHeader("vary", "*");
-        Set<String> expected = new HashSet<>();
+        List<String> expected = new ArrayList<>();
         expected.add("*");
         doTestAddVaryFieldName(response, "too", expected);
     }
@@ -126,9 +126,9 @@ public class TestResponseUtil {
         response.getCoyoteResponse();
         response.addHeader("vary", "foo");
         response.addHeader("vary", "bar");
-        Set<String> expected = new HashSet<>();
-        expected.add("bar");
+        List<String> expected = new ArrayList<>();
         expected.add("foo");
+        expected.add("bar");
         doTestAddVaryFieldName(response, "foo", expected);
     }
 
@@ -138,7 +138,7 @@ public class TestResponseUtil {
         TesterResponse response = new TesterResponse();
         response.getCoyoteResponse();
         response.addHeader("vary", "{{{, bar");
-        Set<String> expected = new HashSet<>();
+        List<String> expected = new ArrayList<>();
         expected.add("bar");
         expected.add("too");
         doTestAddVaryFieldName(response, "too", expected);
@@ -150,7 +150,7 @@ public class TestResponseUtil {
         TesterResponse response = new TesterResponse();
         response.getCoyoteResponse();
         response.addHeader("vary", "{{{, *");
-        Set<String> expected = new HashSet<>();
+        List<String> expected = new ArrayList<>();
         expected.add("*");
         doTestAddVaryFieldName(response, "too", expected);
     }
@@ -161,18 +161,18 @@ public class TestResponseUtil {
         TesterResponse response = new TesterResponse();
         response.getCoyoteResponse();
         response.addHeader("vary", "{{{, bar");
-        Set<String> expected = new HashSet<>();
+        List<String> expected = new ArrayList<>();
         expected.add("bar");
         doTestAddVaryFieldName(response, "bar", expected);
     }
 
 
     private void doTestAddVaryFieldName(TesterResponse response, String fieldName,
-            Set<String> expected) {
+            List<String> expected) {
         ResponseUtil.addVaryFieldName(response, fieldName);
         // There will now only be one Vary header
         String resultHeader = response.getHeader("vary");
-        Set<String> result = new HashSet<>();
+        List<String> result = new ArrayList<>();
         // Deliberately do not use Vary.parseVary as it will skip invalid values.
         for (String value : resultHeader.split(",")) {
             result.add(value.trim());
@@ -184,7 +184,7 @@ public class TestResponseUtil {
     @Test
     public void testMimeHeadersAddAllWithNone() {
         MimeHeaders mh = new MimeHeaders();
-        Set<String> expected = new HashSet<>();
+        List<String> expected = new ArrayList<>();
         expected.add("*");
         doTestAddVaryFieldName(mh, "*", expected);
     }
@@ -195,19 +195,19 @@ public class TestResponseUtil {
         MimeHeaders mh = new MimeHeaders();
         mh.addValue("vary").setString("foo");
         mh.addValue("vary").setString("bar");
-        Set<String> expected = new HashSet<>();
-        expected.add("bar");
+        List<String> expected = new ArrayList<>();
         expected.add("foo");
+        expected.add("bar");
         expected.add("too");
         doTestAddVaryFieldName(mh, "too", expected);
     }
 
     private void doTestAddVaryFieldName(MimeHeaders mh, String fieldName,
-            Set<String> expected) {
+            List<String> expected) {
         ResponseUtil.addVaryFieldName(mh, fieldName);
         // There will now only be one Vary header
         String resultHeader = mh.getHeader("vary");
-        Set<String> result = new HashSet<>();
+        List<String> result = new ArrayList<>();
         // Deliberately do not use Vary.parseVary as it will skip invalid values.
         for (String value : resultHeader.split(",")) {
             result.add(value.trim());
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 0f2d87b..38c4f56 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -137,6 +137,10 @@
         been set on the <code>HttpServletResponse</code> before any call is made
         to <code>HttpServletRequest.upgrade()</code>. (markt)
       </fix>
+      <fix>
+        Ensure that values are not duplicated when manipulating the vary header.
+        Based on a pull request by Fredrik Fall. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">


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