Issue #5171 Simplify GzipHandler user-agent handling

jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java

@@ -738,6 +738,159 @@ public Mutable clear()
             return this;
         }
 
+        /** Ensure that specific HttpField exists when the field may not exist or may
+         * exist and be multi valued.  Multiple existing fields are merged into a
+         * single field.
+         * @param field The header to ensure is contained.  The field is used
+         *              directly if possible so {@link PreEncodedHttpField}s can be
+         *              passed.  If the value needs to be merged with existing values,
+         *              then a new field is created.
+         */
+        public void ensureField(HttpField field)
+        {
+            // Is the field value multi valued?
+            if (field.getValue().indexOf(',') < 0)
+            {
+                // Call Single valued computeEnsure with either String header name or enum HttpHeader
+                if (field.getHeader() != null)
+                    computeField(field.getHeader(), (h, l) -> computeEnsure(field, l));
+                else
+                    computeField(field.getName(), (h, l) -> computeEnsure(field, l));
+            }
+            else
+            {
+                // call multi valued computeEnsure with either String header name or enum HttpHeader
+                if (field.getHeader() != null)
+                    computeField(field.getHeader(), (h, l) -> computeEnsure(field, field.getValues(), l));
+                else
+                    computeField(field.getName(), (h, l) -> computeEnsure(field, field.getValues(), l));
+            }
+        }
+
+        /**
+         * Compute ensure field with a single value
+         * @param ensure The field to ensure exists
+         * @param fields The list of existing fields with the same header
+         */
+        private static HttpField computeEnsure(HttpField ensure, List<HttpField> fields)
+        {
+            // If no existing fields return the ensure field
+            if (fields == null || fields.isEmpty())
+                return ensure;
+
+            String ensureValue = ensure.getValue();
+
+            // Handle a single existing field
+            if (fields.size() == 1)
+            {
+                // If the existing field contains the ensure value, return it, else append values.
+                HttpField f = fields.get(0);
+                return f.contains(ensureValue)
+                    ? f
+                    : new HttpField(ensure.getHeader(), ensure.getName(), f.getValue() + ", " + ensureValue);
+            }
+
+            // Handle multiple existing fields
+            StringBuilder v = new StringBuilder();
+            for (HttpField f : fields)
+            {
+                // Always append multiple fields into a single field value
+                if (v.length() > 0)
+                    v.append(", ");
+                v.append(f.getValue());
+
+                // check if the ensure value is already contained
+                if (ensureValue != null && f.contains(ensureValue))
+                    ensureValue = null;
+            }
+
+            // If the ensure value was not contained append it
+            if (ensureValue != null)
+                v.append(", ").append(ensureValue);
+
+            return new HttpField(ensure.getHeader(), ensure.getName(), v.toString());
+        }
+
+        /**
+         * Compute ensure field with a multiple values
+         * @param ensure The field to ensure exists
+         * @param values The QuotedCSV parsed field values.
+         * @param fields The list of existing fields with the same header
+         */
+        private static HttpField computeEnsure(HttpField ensure, String[] values, List<HttpField> fields)
+        {
+            // If no existing fields return the ensure field
+            if (fields == null || fields.isEmpty())
+                return ensure;
+
+            // Handle a single existing field
+            if (fields.size() == 1)
+            {
+                HttpField f = fields.get(0);
+                // check which ensured values are already contained
+                int ensured = values.length;
+                for (int i = 0; i < values.length; i++)
+                {
+                    if (f.contains(values[i]))
+                    {
+                        ensured--;
+                        values[i] = null;
+                    }
+                }
+
+                // if all ensured values contained return the existing field
+                if (ensured == 0)
+                    return f;
+                // else if no ensured values contained append the entire ensured valued
+                if (ensured == values.length)
+                    return new HttpField(ensure.getHeader(), ensure.getName(),
+                        f.getValue() + ", " + ensure.getValue());
+                // else append just the ensured values that are not contained
+                StringBuilder v = new StringBuilder(f.getValue());
+                for (String value : values)
+                {
+                    if (value != null)
+                        v.append(", ").append(value);
+                }
+                return new HttpField(ensure.getHeader(), ensure.getName(), v.toString());
+            }
+
+            // Handle a multiple existing field
+            StringBuilder v = new StringBuilder();
+            int ensured = values.length;
+            for (HttpField f : fields)
+            {
+                // Always append multiple fields into a single field value
+                if (v.length() > 0)
+                    v.append(", ");
+                v.append(f.getValue());
+
+                // null out ensured values that are included
+                for (int i = 0; i < values.length; i++)
+                {
+                    if (values[i] != null && f.contains(values[i]))
+                    {
+                        ensured--;
+                        values[i] = null;
+                    }
+                }
+            }
+
+            // if no ensured values exist append them all
+            if (ensured == values.length)
+                v.append(", ").append(ensure.getValue());
+            // else if some ensured values are missing, append them
+            else if (ensured > 0)
+            {
+                for (String value : values)
+                    if (value != null)
+                        v.append(", ").append(value);
+            }
+
+            // return a merged header with missing ensured values added
+            return new HttpField(ensure.getHeader(), ensure.getName(), v.toString());
+        }
+
         @Override
         public boolean equals(Object o)
         {

jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java

@@ -41,6 +41,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -899,7 +900,6 @@ public void testComputeField()
     {
         HttpFields.Mutable fields = HttpFields.build();
         assertThat(fields.size(), is(0));
-
         fields.computeField("Test", (n, f) -> null);
         assertThat(fields.size(), is(0));
 
@@ -922,4 +922,140 @@ public void testComputeField()
         fields.computeField("TEST", (n, f) -> null);
         assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Before: value", "After: value"));
     }
-}
+
+    @Test
+    public void testEnsureSingleValue()
+    {
+        HttpFields.Mutable fields = HttpFields.build();
+
+        // 0 existing case
+        assertThat(fields.size(), is(0));
+        fields.ensureField(new PreEncodedHttpField(HttpHeader.VARY, "one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one"));
+        assertThat(fields.getField(0), instanceOf(PreEncodedHttpField.class));
+
+        // 1 existing cases
+        fields.ensureField(new HttpField(HttpHeader.VARY, "one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one"));
+;
+        fields.ensureField(new HttpField(HttpHeader.VARY, "two"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two"));
+
+        // many existing cases
+        fields.put(new HttpField(HttpHeader.VARY, "one"));
+        fields.add(new HttpField(HttpHeader.VARY, "two"));
+        fields.ensureField(new HttpField(HttpHeader.VARY, "one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two"));
+
+        fields.put(new HttpField(HttpHeader.VARY, "one"));
+        fields.add(new HttpField(HttpHeader.VARY, "two"));
+        fields.ensureField(new HttpField(HttpHeader.VARY, "three"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two, three"));
+    }
+
+    @Test
+    public void testEnsureMultiValue()
+    {
+        HttpFields.Mutable fields = HttpFields.build();
+
+        // zero existing case
+        assertThat(fields.size(), is(0));
+        fields.ensureField(new PreEncodedHttpField(HttpHeader.VARY, "one, two"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two"));
+        assertThat(fields.getField(0), instanceOf(PreEncodedHttpField.class));
+
+        // one existing cases
+        fields.ensureField(new HttpField(HttpHeader.VARY, "two, one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two"));
+
+        fields.ensureField(new HttpField(HttpHeader.VARY, "three, one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two, three"));
+
+        fields.ensureField(new HttpField(HttpHeader.VARY, "four, five"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two, three, four, five"));
+
+        // many existing cases
+        fields.put(new HttpField(HttpHeader.VARY, "one"));
+        fields.add(new HttpField(HttpHeader.VARY, "two"));
+        fields.ensureField(new HttpField(HttpHeader.VARY, "two, one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two"));
+
+        fields.put(new HttpField(HttpHeader.VARY, "one"));
+        fields.add(new HttpField(HttpHeader.VARY, "two"));
+        fields.ensureField(new HttpField(HttpHeader.VARY, "three, two"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two, three"));
+
+        fields.put(new HttpField(HttpHeader.VARY, "one"));
+        fields.add(new HttpField(HttpHeader.VARY, "two"));
+        fields.ensureField(new HttpField(HttpHeader.VARY, "three, four"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Vary: one, two, three, four"));
+    }
+
+    @Test
+    public void testEnsureStringSingleValue()
+    {
+        HttpFields.Mutable fields = HttpFields.build();
+
+        // 0 existing case
+        assertThat(fields.size(), is(0));
+        fields.ensureField(new PreEncodedHttpField("Test", "one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one"));
+        assertThat(fields.getField(0), instanceOf(PreEncodedHttpField.class));
+
+        // 1 existing cases
+        fields.ensureField(new HttpField("Test", "one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one"));
+        ;
+        fields.ensureField(new HttpField("Test", "two"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two"));
+
+        // many existing cases
+        fields.put(new HttpField("Test", "one"));
+        fields.add(new HttpField("Test", "two"));
+        fields.ensureField(new HttpField("Test", "one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two"));
+
+        fields.put(new HttpField("Test", "one"));
+        fields.add(new HttpField("Test", "two"));
+        fields.ensureField(new HttpField("Test", "three"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two, three"));
+    }
+
+    @Test
+    public void testEnsureStringMultiValue()
+    {
+        HttpFields.Mutable fields = HttpFields.build();
+
+        // zero existing case
+        assertThat(fields.size(), is(0));
+        fields.ensureField(new PreEncodedHttpField("Test", "one, two"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two"));
+        assertThat(fields.getField(0), instanceOf(PreEncodedHttpField.class));
+
+        // one existing cases
+        fields.ensureField(new HttpField("Test", "two, one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two"));
+
+        fields.ensureField(new HttpField("Test", "three, one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two, three"));
+
+        fields.ensureField(new HttpField("Test", "four, five"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two, three, four, five"));
+
+        // many existing cases
+        fields.put(new HttpField("Test", "one"));
+        fields.add(new HttpField("Test", "two"));
+        fields.ensureField(new HttpField("Test", "two, one"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two"));
+
+        fields.put(new HttpField("Test", "one"));
+        fields.add(new HttpField("Test", "two"));
+        fields.ensureField(new HttpField("Test", "three, two"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two, three"));
+
+        fields.put(new HttpField("Test", "one"));
+        fields.add(new HttpField("Test", "two"));
+        fields.ensureField(new HttpField("Test", "three, four"));
+        assertThat(fields.stream().map(HttpField::toString).collect(Collectors.toList()), contains("Test: one, two, three, four"));
+    }
+}

jetty-rewrite/src/main/config/modules/msie.mod

@@ -0,0 +1,13 @@
+# DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html
+
+[description]
+Enables the MSIE rewrite rule for MSIE 5 and 6 known bugs.
+
+[depend]
+rewrite
+
+[files]
+basehome:modules/rewrite/rewrite-msie.xml|etc/rewrite-msie.xml
+
+[xml]
+etc/rewrite-msie.xml

jetty-rewrite/src/main/config/modules/rewrite/rewrite-msie.xml

@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
+<Configure id="Rewrite" class="org.eclipse.jetty.rewrite.handler.RuleContainer">
+  <Call name="addRule">
+    <Arg>
+      <New class="org.eclipse.jetty.rewrite.handler.MsieRule"/>
+    </Arg>
+  </Call>
+</Configure>
+

jetty-rewrite/src/main/config/modules/rewrite/rewrite-rules.xml

@@ -2,14 +2,6 @@
 <!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://www.eclipse.org/jetty/configure_10_0.dtd">
 <Configure id="Rewrite" class="org.eclipse.jetty.rewrite.handler.RuleContainer">
 
-  <!-- Add rule to protect against IE ssl bug 
-  <Call name="addRule">
-    <Arg>
-      <New class="org.eclipse.jetty.rewrite.handler.MsieSslRule"/>
-    </Arg>
-  </Call>
-  -->
-
   <!-- protect favicon handling
   <Call name="addRule">
     <Arg>
@@ -102,5 +94,13 @@
   </Call>
   -->
 
+  <!-- Add rule to protect against IE ssl bug (see msie module and rewrite-msie.xml)
+  <Call name="addRule">
+    <Arg>
+      <New class="org.eclipse.jetty.rewrite.handler.MsieRule"/>
+    </Arg>
+  </Call>
+  -->
+
 </Configure>