Issue #12324 - support Accept-Encoding: * on CompressionHandler

jetty-core/jetty-compression/jetty-compression-server/pom.xml

@@ -26,5 +26,10 @@
       <artifactId>jetty-compression-api</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <artifactId>jetty-slf4j-impl</artifactId>
+      <groupId>org.eclipse.jetty</groupId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 </project>

jetty-core/jetty-compression/jetty-compression-server/src/main/java/org/eclipse/jetty/compression/server/CompressionConfig.java

@@ -13,6 +13,7 @@
 
 package org.eclipse.jetty.compression.server;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
@@ -35,13 +36,13 @@
  * Configuration for a specific compression behavior per matching path from the {@link CompressionHandler}.
  *
  * <p>
- *    Configuration is split between compression (of responses) and decompression (of requests).
+ * Configuration is split between compression (of responses) and decompression (of requests).
  * </p>
  *
  * <p>
- *    Experimental Configuration, subject to change while the implementation is being settled.
- *    Please provide feedback at the <a href="https://github.com/jetty/jetty.project/issues">Jetty Issue tracker</a>
- *    to influence the direction / development of these experimental features.
+ * Experimental Configuration, subject to change while the implementation is being settled.
+ * Please provide feedback at the <a href="https://github.com/jetty/jetty.project/issues">Jetty Issue tracker</a>
+ * to influence the direction / development of these experimental features.
  * </p>
  */
 @ManagedObject("Compression Configuration")
@@ -79,11 +80,16 @@ public class CompressionConfig extends AbstractLifeCycle
      * Set of paths that support decompressing Request content.
      */
     private final IncludeExcludeSet<String, String> decompressPaths;
+    /**
+     * Optional preferred order of encoders for compressing Response content.
+     */
+    private final List<String> compressPreferredEncoderOrder;
 
     private final HttpField vary;
 
     private CompressionConfig(Builder builder)
     {
+        this.compressPreferredEncoderOrder = builder.compressPreferredEncoderOrder;
         this.compressEncodings = builder.compressEncodings.asImmutable();
         this.decompressEncodings = builder.decompressEncodings.asImmutable();
         this.compressMethods = builder.decompressMethods.asImmutable();
@@ -178,20 +184,40 @@ public Set<String> getCompressPathIncludes()
         return Collections.unmodifiableSet(includes);
     }
 
+    /**
+     * Get the preferred order of encoders for compressing response content.
+     *
+     * <p>
+     *     See {@link Builder#compressPreferredEncoderOrder(List)} for details
+     *     on how the {@code Accept-Encoding} request header interacts with
+     *     this configuration.
+     * </p>
+     *
+     * @return the preferred order of encoders.
+     * @see Builder#compressPreferredEncoderOrder(List)
+     */
+    @ManagedAttribute()
+    public List<String> getCompressPreferredEncoderOrder()
+    {
+        return Collections.unmodifiableList(compressPreferredEncoderOrder);
+    }
+
+    /**
+     * Return the encoder that best matches the provided details.
+     *
+     * @param requestAcceptEncoding the HTTP {@code Accept-Encoding} header list (includes only supported encodings,
+     *      and possibly the {@code *} glob value)
+     * @param request the request itself
+     * @param pathInContext the path in context
+     * @return the selected compression encoding
+     */
     public String getCompressionEncoding(List<String> requestAcceptEncoding, Request request, String pathInContext)
     {
         if (requestAcceptEncoding == null || requestAcceptEncoding.isEmpty())
             return null;
 
-        String matchedEncoding = null;
-
-        for (String encoding : requestAcceptEncoding)
-        {
-            if (compressEncodings.test(encoding))
-            {
-                matchedEncoding = encoding;
-            }
-        }
+        List<String> preferredEncoders = calcPreferredEncoders(requestAcceptEncoding);
+        String matchedEncoding = selectEncoderMatch(preferredEncoders);
 
         if (matchedEncoding == null)
             return null;
@@ -205,6 +231,44 @@ public String getCompressionEncoding(List<String> requestAcceptEncoding, Request
         return matchedEncoding;
     }
 
+    protected List<String> calcPreferredEncoders(List<String> requestAcceptEncoding)
+    {
+        if (compressPreferredEncoderOrder.isEmpty())
+        {
+            List<String> result = new ArrayList<>(requestAcceptEncoding);
+            result.removeIf((str) -> str.equals("*"));
+            return result;
+        }
+
+        if (requestAcceptEncoding.contains("*"))
+        {
+            // anything else in request Accept-Encoding is moot if glob exists.
+            return compressPreferredEncoderOrder;
+        }
+
+        List<String> preferredEncoderOrder = new ArrayList<>();
+        for (String preferredEncoder: compressPreferredEncoderOrder)
+        {
+            if (requestAcceptEncoding.contains(preferredEncoder))
+            {
+                preferredEncoderOrder.add(preferredEncoder);
+            }
+        }
+        return preferredEncoderOrder;
+    }
+
+    protected String selectEncoderMatch(List<String> preferredEncoders)
+    {
+        for (String encoding : preferredEncoders)
+        {
+            if (compressEncodings.test(encoding))
+            {
+                return encoding;
+            }
+        }
+        return null;
+    }
+
     /**
      * Get the set of excluded HTTP methods for Request decompression.
      *
@@ -362,6 +426,11 @@ public static class Builder
          * Mime-Types that support compressing Response content.
          */
         private final IncludeExclude<String> decompressMimeTypes = new IncludeExclude<>(AsciiLowerCaseSet.class);
+        /**
+         * Optional preferred order of encoders for compressing Response content.
+         */
+        private final List<String> compressPreferredEncoderOrder = new ArrayList<>();
+
         private HttpField vary = new PreEncodedHttpField(HttpHeader.VARY, HttpHeader.ACCEPT_ENCODING.asString());
 
         public CompressionConfig build()
@@ -485,6 +554,81 @@ public Builder compressPathInclude(String pathSpecString)
             return this;
         }
 
+        /**
+         * Control the preferred order of encoders when compressing response content.
+         *
+         * <p>
+         *     If set to an empty List this preferred order is not considered
+         *     when selecting the encoder from the {@code Accept-Encoding} Request header.
+         * </p>
+         * <p>
+         *     If set, the union of matching encoders is the end result used to determine
+         *     what encoder should be used for compressing response content.
+         * </p>
+         * <p>
+         *     Of special note, the {@code Accept-Encoding: *} (glob) header value will
+         *     return the {@code compressPreferredEncoderOrder} if provided here, otherwise
+         *     the {@code *} (glob) header value will be ignored if this
+         *     {@code compressPreferredEncoderOrder} is not provided.
+         * </p>
+         * <table style="border: 1px solid black; border-collapse: separate; border-spacing: 0px;">
+         * <caption style="font-weight: bold; font-size: 1.2em">Encoder order resolution</caption>
+         * <colgroup>
+         *     <col><col><col>
+         * </colgroup>
+         * <thead style="background-color: lightgray">
+         * <tr>
+         *     <th>{@code compressPreferredEncoderOrder}</th>
+         *     <th>{@code Accept-Encoding} header</th>
+         *     <th>Resulting encoders considered</th>
+         * </tr>
+         * </thead>
+         * <tbody style="text-align: left; vertical-align: top;">
+         * <tr>
+         *     <td>{@code <empty>}</td>
+         *     <td>{@code gzip, br}</td>
+         *     <td>{@code gzip, br}</td>
+         * </tr>
+         * <tr>
+         *     <td>{@code <empty>}</td>
+         *     <td>{@code br, gzip}</td>
+         *     <td>{@code br, gzip}</td>
+         * </tr>
+         * <tr>
+         *     <td>{@code br, gzip}</td>
+         *     <td>{@code gzip, br, zstd}</td>
+         *     <td>{@code br, gzip}</td>
+         * </tr>
+         * <tr>
+         *     <td>{@code zstd, br}</td>
+         *     <td>{@code gzip, br}</td>
+         *     <td>{@code br}</td>
+         * </tr>
+         * <tr>
+         *     <td>{@code zstd, br, gzip}</td>
+         *     <td>{@code *}</td>
+         *     <td>{@code zstd, br, gzip}</td>
+         * </tr>
+         * <tr>
+         *     <td>{@code <empty>}</td>
+         *     <td>{@code *}</td>
+         *     <td>{@code <empty>}</td>
+         * </tr>
+         * </tbody>
+         * </table>
+         *
+         * @param encoders the encoders, in order, to use for compressing response content.
+         *   Will replace any previously set order.
+         * @return this builder.
+         */
+        public Builder compressPreferredEncoderOrder(List<String> encoders)
+        {
+            this.compressPreferredEncoderOrder.clear();
+            if (encoders != null)
+                this.compressPreferredEncoderOrder.addAll(encoders);
+            return this;
+        }
+
         /**
          * A {@code Content-Encoding} encoding to exclude.
          *
@@ -639,7 +783,7 @@ else if (type.startsWith("image/") ||
                 "application/zstd",
                 // It is possible to use SSE with CompressionHandler, but only if you use `gzip` encoding with syncFlush to true which will impact performance.
                 "text/event-stream"
-                ).forEach((type) ->
+            ).forEach((type) ->
             {
                 compressMimeTypeExclude(type);
                 decompressMimeTypeExclude(type);

jetty-core/jetty-compression/jetty-compression-server/src/main/java/org/eclipse/jetty/compression/server/CompressionHandler.java

@@ -14,17 +14,18 @@
 package org.eclipse.jetty.compression.server;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
+import java.util.TreeMap;
 
 import org.eclipse.jetty.compression.Compression;
 import org.eclipse.jetty.http.EtagUtils;
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.MimeTypes;
+import org.eclipse.jetty.http.pathmap.MappedResource;
 import org.eclipse.jetty.http.pathmap.MatchedResource;
 import org.eclipse.jetty.http.pathmap.PathMappings;
 import org.eclipse.jetty.http.pathmap.PathSpec;
@@ -63,8 +64,7 @@ public class CompressionHandler extends Handler.Wrapper
     public static final String HANDLER_ETAGS = CompressionHandler.class.getPackageName() + ".ETag";
 
     private static final Logger LOG = LoggerFactory.getLogger(CompressionHandler.class);
-    // TODO: make into a case-insensitive map
-    private final Map<String, Compression> supportedEncodings = new HashMap<>();
+    private final Map<String, Compression> supportedEncodings = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
     private final PathMappings<CompressionConfig> pathConfigs = new PathMappings<CompressionConfig>();
 
     public CompressionHandler()
@@ -228,7 +228,7 @@ public boolean handle(final Request request, final Response response, final Call
                         {
                             String lvalue = StringUtil.asciiToLowerCase(value);
                             // only track encodings that are supported by this handler
-                            if (supportedEncodings.containsKey(lvalue))
+                            if ("*".equals(value) || supportedEncodings.containsKey(lvalue))
                             {
                                 if (requestAcceptEncoding == null)
                                     requestAcceptEncoding = new ArrayList<>();
@@ -317,6 +317,26 @@ protected void doStart() throws Exception
                     .build());
         }
 
+        // ensure that the preferred encoder order is sane for the configuration.
+        for (MappedResource<CompressionConfig> pathConfig : pathConfigs)
+        {
+            List<String> preferredEncoders = pathConfig.getResource().getCompressPreferredEncoderOrder();
+            if (preferredEncoders.isEmpty())
+                continue;
+            ListIterator<String> preferredIter = preferredEncoders.listIterator();
+            while (preferredIter.hasNext())
+            {
+                String listedEncoder = preferredIter.next();
+                if (!supportedEncodings.containsKey(listedEncoder))
+                {
+                    LOG.warn("Unable to find compression encoder {} from configuration for pathspec {} in registered compression encoders [{}]",
+                        listedEncoder, pathConfig.getPathSpec(),
+                        String.join(", ", supportedEncodings.keySet()));
+                    preferredIter.remove(); // remove bad encoding
+                }
+            }
+        }
+
         super.doStart();
     }
 

jetty-core/jetty-compression/jetty-compression-server/src/test/java/org/eclipse/jetty/compression/server/CompressionConfigTest.java

@@ -0,0 +1,65 @@
+//
+// ========================================================================
+// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.compression.server;
+
+import java.util.List;
+
+import org.eclipse.jetty.http.QuotedQualityCSV;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.hasSize;
+
+public class CompressionConfigTest
+{
+    private static List<String> qcsv(String rawheadervalue)
+    {
+        QuotedQualityCSV csv = new QuotedQualityCSV();
+        csv.addValue(rawheadervalue);
+        return csv.getValues();
+    }
+
+    @ParameterizedTest
+    @CsvSource(useHeadersInDisplayName = true, delimiterString = "|", textBlock = """
+        PreferredEncoders | AcceptEncodings | ExpectedResult
+                          | gzip, br        | gzip, br
+                          | br, gzip        | br, gzip
+        br, gzip          | gzip, br        | br, gzip
+        zstd, br          | gzip, br, zstd  | zstd, br
+        zstd, br, gzip    | *               | zstd, br, gzip
+                          | *               |   
+        """)
+    public void testCalcPreferredEncoders(String preferredEncoderOrderCsv, String acceptEncodingHeaderValuesCsv, String expectedEncodersCsv)
+    {
+        List<String> preferredEncoderOrder = qcsv(preferredEncoderOrderCsv);
+        List<String> acceptEncodingHeaderValues = qcsv(acceptEncodingHeaderValuesCsv);
+        List<String> expectedEncodersResult = qcsv(expectedEncodersCsv);
+
+        CompressionConfig config = CompressionConfig.builder()
+            .compressPreferredEncoderOrder(preferredEncoderOrder)
+            .build();
+        List<String> result = config.calcPreferredEncoders(acceptEncodingHeaderValues);
+        if (expectedEncodersResult.isEmpty())
+        {
+            assertThat(result, hasSize(0));
+        }
+        else
+        {
+            String[] expected = expectedEncodersResult.toArray(new String[0]);
+            assertThat(result, contains(expected));
+        }
+    }
+}