Merge pull request #6550 from eclipse/jetty-10.0.x-6544-gziphandler-e…

…xcludedMimeTypes

Issue #6544 - Fixing broken `jetty.gzip.excludedMimeTypeList` property support

demos/demo-simple-webapp/src/main/webapp/WEB-INF/web.xml

@@ -6,4 +6,10 @@
 
   <display-name>Simple Web Application</display-name>
 
+  <!-- using a mime-type and extension that does NOT exist in jetty-http's mime.properties -->
+  <mime-mapping>
+    <extension>icon</extension>
+    <mime-type>image/vnd.microsoft.icon</mime-type>
+  </mime-mapping>
+
 </web-app>

jetty-server/src/main/config/etc/jetty-gzip.xml

@@ -18,8 +18,8 @@
         <Set name="dispatcherTypes" property="jetty.gzip.dispatcherTypes"/>
         <Set name="includedMethodList" property="jetty.gzip.includedMethodList"/>
         <Set name="excludedMethodList" property="jetty.gzip.excludedMethodList"/>
-        <Set name="includedMimeTypes" property="jetty.gzip.includedMimeTypeList"/>
-        <Set name="excludedMimeTypes" property="jetty.gzip.excludedMimeTypeList"/>
+        <Set name="includedMimeTypesList" property="jetty.gzip.includedMimeTypeList"/>
+        <Set name="excludedMimeTypesList" property="jetty.gzip.excludedMimeTypeList"/>
         <Set name="includedPaths" property="jetty.gzip.includedPathList"/>
         <Set name="excludedPaths" property="jetty.gzip.excludedPathList"/>
         <Set name="inflaterPool">

jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java

@@ -768,6 +768,17 @@ public void setExcludedMimeTypes(String... types)
         _mimeTypes.exclude(types);
     }
 
+    /**
+     * Set the excluded filter list of MIME types (replacing any previously set)
+     *
+     * @param csvTypes The list of mime types to exclude (without charset or other parameters), CSV format
+     * @see #setIncludedMimeTypesList(String)
+     */
+    public void setExcludedMimeTypesList(String csvTypes)
+    {
+        setExcludedMimeTypes(StringUtil.csvSplit(csvTypes));
+    }
+
     /**
      * Set the excluded filter list of Path specs (replacing any previously set)
      *
@@ -819,6 +830,17 @@ public void setIncludedMimeTypes(String... types)
         _mimeTypes.include(types);
     }
 
+    /**
+     * Set the included filter list of MIME types (replacing any previously set)
+     *
+     * @param csvTypes The list of mime types to include (without charset or other parameters), CSV format
+     * @see #setExcludedMimeTypesList(String)
+     */
+    public void setIncludedMimeTypesList(String csvTypes)
+    {
+        setIncludedMimeTypes(StringUtil.csvSplit(csvTypes));
+    }
+
     /**
      * Set the included filter list of Path specs (replacing any previously set)
      *

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/GzipHandlerTest.java

@@ -61,7 +61,6 @@
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
-@SuppressWarnings("serial")
 public class GzipHandlerTest
 {
     private static final String __content =
@@ -88,6 +87,8 @@ public class GzipHandlerTest
 
     private Server _server;
     private LocalConnector _connector;
+    private GzipHandler gzipHandler;
+    private ServletContextHandler context;
 
     @BeforeEach
     public void init() throws Exception
@@ -96,25 +97,25 @@ public void init() throws Exception
         _connector = new LocalConnector(_server);
         _server.addConnector(_connector);
 
-        GzipHandler gzipHandler = new GzipHandler();
+        gzipHandler = new GzipHandler();
         gzipHandler.setMinGzipSize(16);
         gzipHandler.setInflateBufferSize(4096);
 
-        ServletContextHandler context = new ServletContextHandler(gzipHandler, "/ctx");
-        ServletHandler servlets = context.getServletHandler();
+        context = new ServletContextHandler(gzipHandler, "/ctx");
 
         _server.setHandler(gzipHandler);
         gzipHandler.setHandler(context);
-        servlets.addServletWithMapping(MicroServlet.class, "/micro");
-        servlets.addServletWithMapping(MicroChunkedServlet.class, "/microchunked");
-        servlets.addServletWithMapping(TestServlet.class, "/content");
-        servlets.addServletWithMapping(ForwardServlet.class, "/forward");
-        servlets.addServletWithMapping(IncludeServlet.class, "/include");
-        servlets.addServletWithMapping(EchoServlet.class, "/echo/*");
-        servlets.addServletWithMapping(DumpServlet.class, "/dump/*");
-        servlets.addServletWithMapping(AsyncServlet.class, "/async/*");
-        servlets.addServletWithMapping(BufferServlet.class, "/buffer/*");
-        servlets.addFilterWithMapping(CheckFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
+        context.addServlet(MicroServlet.class, "/micro");
+        context.addServlet(MicroChunkedServlet.class, "/microchunked");
+        context.addServlet(TestServlet.class, "/content");
+        context.addServlet(MimeTypeContentServlet.class, "/mimetypes/*");
+        context.addServlet(ForwardServlet.class, "/forward");
+        context.addServlet(IncludeServlet.class, "/include");
+        context.addServlet(EchoServlet.class, "/echo/*");
+        context.addServlet(DumpServlet.class, "/dump/*");
+        context.addServlet(AsyncServlet.class, "/async/*");
+        context.addServlet(BufferServlet.class, "/buffer/*");
+        context.addFilter(CheckFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST));
 
         _server.start();
     }
@@ -147,6 +148,31 @@ protected void doGet(HttpServletRequest req, HttpServletResponse response) throw
         }
     }
 
+    public static class MimeTypeContentServlet extends HttpServlet
+    {
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+        {
+            String pathInfo = req.getPathInfo();
+            resp.setContentType(getContentTypeFromRequest(pathInfo, req));
+            resp.getWriter().println("This is content for " + pathInfo);
+        }
+
+        private String getContentTypeFromRequest(String filename, HttpServletRequest req)
+        {
+            String defaultContentType = "application/octet-stream";
+            if (req.getParameter("type") != null)
+                defaultContentType = req.getParameter("type");
+            ServletContextHandler servletContextHandler = ServletContextHandler.getServletContextHandler(getServletContext());
+            if (servletContextHandler == null)
+                return defaultContentType;
+            String contentType = servletContextHandler.getMimeTypes().getMimeByExtension(filename);
+            if (contentType != null)
+                return contentType;
+            return defaultContentType;
+        }
+    }
+
     public static class TestServlet extends HttpServlet
     {
         @Override
@@ -797,6 +823,52 @@ public void testGzipBomb() throws Exception
         assertThat(response.getContentBytes().length, is(512 * 1024));
     }
 
+    @Test
+    public void testGzipExcludeNewMimeType() throws Exception
+    {
+        // setting all excluded mime-types to a mimetype new mime-type
+        // Note: this mime-type does not exist in MimeTypes object.
+        gzipHandler.setExcludedMimeTypes("image/webfoo");
+
+        // generated and parsed test
+        HttpTester.Request request = HttpTester.newRequest();
+        HttpTester.Response response;
+
+        // Request something that is not present on MimeTypes and is also
+        // excluded by GzipHandler configuration
+        request.setMethod("GET");
+        request.setURI("/ctx/mimetypes/foo.webfoo?type=image/webfoo");
+        request.setVersion("HTTP/1.1");
+        request.setHeader("Host", "tester");
+        request.setHeader("Accept", "*/*");
+        request.setHeader("Accept-Encoding", "gzip"); // allow compressed responses
+        request.setHeader("Connection", "close");
+
+        response = HttpTester.parseResponse(_connector.getResponse(request.generate()));
+
+        assertThat(response.getStatus(), is(200));
+        assertThat("Should not be compressed with gzip", response.get("Content-Encoding"), nullValue());
+        assertThat(response.get("ETag"), nullValue());
+        assertThat(response.get("Vary"), nullValue());
+
+        // Request something that is present on MimeTypes and is also compressible
+        // by the GzipHandler configuration
+        request.setMethod("GET");
+        request.setURI("/ctx/mimetypes/zed.txt");
+        request.setVersion("HTTP/1.1");
+        request.setHeader("Host", "tester");
+        request.setHeader("Accept", "*/*");
+        request.setHeader("Accept-Encoding", "gzip"); // allow compressed responses
+        request.setHeader("Connection", "close");
+
+        response = HttpTester.parseResponse(_connector.getResponse(request.generate()));
+
+        assertThat(response.getStatus(), is(200));
+        assertThat(response.get("Content-Encoding"), containsString("gzip"));
+        assertThat(response.get("ETag"), nullValue());
+        assertThat(response.get("Vary"), is("Accept-Encoding"));
+    }
+
     public static class CheckFilter implements Filter
     {
         @Override

tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/GzipModuleTests.java

@@ -0,0 +1,176 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2021 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.tests.distribution;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpStatus;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class GzipModuleTests extends AbstractJettyHomeTest
+{
+    @Test
+    public void testGzipDefault() throws Exception
+    {
+        Path jettyBase = newTestJettyBaseDirectory();
+        String jettyVersion = System.getProperty("jettyVersion");
+        JettyHomeTester distribution = JettyHomeTester.Builder.newInstance()
+            .jettyVersion(jettyVersion)
+            .jettyBase(jettyBase)
+            .mavenLocalRepository(System.getProperty("mavenRepoPath"))
+            .build();
+
+        int httpPort = distribution.freePort();
+
+        String[] argsConfig = {
+            "--add-modules=gzip",
+            "--add-modules=deploy,webapp,http"
+        };
+
+        try (JettyHomeTester.Run runConfig = distribution.start(argsConfig))
+        {
+            assertTrue(runConfig.awaitFor(5, TimeUnit.SECONDS));
+            assertEquals(0, runConfig.getExitValue());
+
+            String[] argsStart = {
+                "jetty.http.port=" + httpPort,
+                "jetty.httpConfig.port=" + httpPort
+            };
+
+            File war = distribution.resolveArtifact("org.eclipse.jetty.demos:demo-simple-webapp:war:" + jettyVersion);
+            distribution.installWarFile(war, "demo");
+
+            try (JettyHomeTester.Run runStart = distribution.start(argsStart))
+            {
+                assertTrue(runStart.awaitConsoleLogsFor("Started Server@", 20, TimeUnit.SECONDS));
+
+                startHttpClient();
+                ContentResponse response = client.GET("http://localhost:" + httpPort + "/demo/index.html");
+                String responseDetails = toResponseDetails(response);
+                assertEquals(HttpStatus.OK_200, response.getStatus(), responseDetails);
+                assertThat(responseDetails, response.getHeaders().get(HttpHeader.CONTENT_ENCODING), containsString("gzip"));
+            }
+        }
+    }
+
+    @Test
+    public void testGzipDefaultExcludedMimeType() throws Exception
+    {
+        Path jettyBase = newTestJettyBaseDirectory();
+        String jettyVersion = System.getProperty("jettyVersion");
+        JettyHomeTester distribution = JettyHomeTester.Builder.newInstance()
+            .jettyVersion(jettyVersion)
+            .jettyBase(jettyBase)
+            .mavenLocalRepository(System.getProperty("mavenRepoPath"))
+            .build();
+
+        int httpPort = distribution.freePort();
+
+        String[] argsConfig = {
+            "--add-modules=gzip",
+            "--add-modules=deploy,webapp,http"
+        };
+
+        try (JettyHomeTester.Run runConfig = distribution.start(argsConfig))
+        {
+            assertTrue(runConfig.awaitFor(5, TimeUnit.SECONDS));
+            assertEquals(0, runConfig.getExitValue());
+
+            String[] argsStart = {
+                "jetty.http.port=" + httpPort,
+                "jetty.httpConfig.port=" + httpPort
+            };
+
+            File war = distribution.resolveArtifact("org.eclipse.jetty.demos:demo-simple-webapp:war:" + jettyVersion);
+            distribution.installWarFile(war, "demo");
+
+            try (JettyHomeTester.Run runStart = distribution.start(argsStart))
+            {
+                assertTrue(runStart.awaitConsoleLogsFor("Started Server@", 20, TimeUnit.SECONDS));
+
+                startHttpClient();
+                ContentResponse response = client.GET("http://localhost:" + httpPort + "/demo/jetty.webp");
+                String responseDetails = toResponseDetails(response);
+                assertEquals(HttpStatus.OK_200, response.getStatus(), responseDetails);
+                assertThat(responseDetails, response.getHeaders().get(HttpHeader.CONTENT_TYPE), containsString("image/webp"));
+                assertThat(responseDetails, response.getHeaders().get(HttpHeader.CONTENT_ENCODING), not(containsString("gzip")));
+            }
+        }
+    }
+
+    @Test
+    public void testGzipAddWebappSpecificExcludeMimeType() throws Exception
+    {
+        Path jettyBase = newTestJettyBaseDirectory();
+        String jettyVersion = System.getProperty("jettyVersion");
+        JettyHomeTester distribution = JettyHomeTester.Builder.newInstance()
+            .jettyVersion(jettyVersion)
+            .jettyBase(jettyBase)
+            .mavenLocalRepository(System.getProperty("mavenRepoPath"))
+            .build();
+
+        int httpPort = distribution.freePort();
+
+        String[] argsConfig = {
+            "--add-modules=gzip",
+            "--add-modules=deploy,webapp,http"
+        };
+
+        try (JettyHomeTester.Run runConfig = distribution.start(argsConfig))
+        {
+            assertTrue(runConfig.awaitFor(5, TimeUnit.SECONDS));
+            assertEquals(0, runConfig.getExitValue());
+
+            String[] argsStart = {
+                "jetty.http.port=" + httpPort,
+                "jetty.httpConfig.port=" + httpPort,
+                "jetty.gzip.excludedMimeTypeList=image/vnd.microsoft.icon"
+            };
+
+            File war = distribution.resolveArtifact("org.eclipse.jetty.demos:demo-simple-webapp:war:" + jettyVersion);
+            distribution.installWarFile(war, "demo");
+
+            try (JettyHomeTester.Run runStart = distribution.start(argsStart))
+            {
+                assertTrue(runStart.awaitConsoleLogsFor("Started Server@", 20, TimeUnit.SECONDS));
+
+                startHttpClient();
+                ContentResponse response = client.GET("http://localhost:" + httpPort + "/demo/jetty.icon");
+                String responseDetails = toResponseDetails(response);
+                assertEquals(HttpStatus.OK_200, response.getStatus(), responseDetails);
+                assertThat(responseDetails, response.getHeaders().get(HttpHeader.CONTENT_ENCODING), not(containsString("gzip")));
+                assertThat(responseDetails, response.getHeaders().get(HttpHeader.CONTENT_TYPE), containsString("image/vnd.microsoft.icon"));
+            }
+        }
+    }
+
+    private static String toResponseDetails(ContentResponse response)
+    {
+        StringBuilder ret = new StringBuilder();
+        ret.append(response.toString()).append(System.lineSeparator());
+        ret.append(response.getHeaders().toString()).append(System.lineSeparator());
+        ret.append(response.getContentAsString()).append(System.lineSeparator());
+        return ret.toString();
+    }
+}