Issue #7858 - GzipHandler request.isHandled support

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

@@ -664,14 +664,20 @@ public void setInflateBufferSize(int size)
     @Override
     public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
     {
+        if (baseRequest.isHandled())
+        {
+            super.handle(target, baseRequest, request, response);
+            return;
+        }
+
         final ServletContext context = baseRequest.getServletContext();
         final String path = baseRequest.getPathInContext();
         LOG.debug("{} handle {} in {}", this, baseRequest, context);
 
         if (!_dispatchers.contains(baseRequest.getDispatcherType()))
         {
             LOG.debug("{} excluded by dispatcherType {}", this, baseRequest.getDispatcherType());
-            _handler.handle(target, baseRequest, request, response);
+            super.handle(target, baseRequest, request, response);
             return;
         }
 
@@ -688,6 +694,15 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
             baseRequest.getHttpInput().addInterceptor(gzipHttpInputInterceptor);
         }
 
+        // From here on out, the response output gzip determination is made
+
+        // Don't attempt to modify the response output if it's already committed.
+        if (response.isCommitted())
+        {
+            super.handle(target, baseRequest, request, response);
+            return;
+        }
+
         // Are we already being gzipped?
         HttpOutput out = baseRequest.getResponse().getHttpOutput();
         HttpOutput.Interceptor interceptor = out.getInterceptor();
@@ -764,15 +779,15 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
         if (alreadyGzipped)
         {
             LOG.debug("{} already intercepting {}", this, request);
-            _handler.handle(target, baseRequest, request, response);
+            super.handle(target, baseRequest, request, response);
             return;
         }
 
         // If not a supported method - no Vary because no matter what client, this URI is always excluded
         if (!_methods.test(baseRequest.getMethod()))
         {
             LOG.debug("{} excluded by method {}", this, request);
-            _handler.handle(target, baseRequest, request, response);
+            super.handle(target, baseRequest, request, response);
             return;
         }
 
@@ -781,7 +796,7 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
         if (!isPathGzipable(path))
         {
             LOG.debug("{} excluded by path {}", this, request);
-            _handler.handle(target, baseRequest, request, response);
+            super.handle(target, baseRequest, request, response);
             return;
         }
 
@@ -794,7 +809,7 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
             {
                 LOG.debug("{} excluded by path suffix mime type {}", this, request);
                 // handle normally without setting vary header
-                _handler.handle(target, baseRequest, request, response);
+                super.handle(target, baseRequest, request, response);
                 return;
             }
         }
@@ -804,9 +819,7 @@ else if (COMMA_GZIP.matcher(field.getValue()).matches())
         {
             // install interceptor and handle
             out.setInterceptor(new GzipHttpOutputInterceptor(this, getVaryField(), baseRequest.getHttpChannel(), origInterceptor, isSyncFlush()));
-
-            if (_handler != null)
-                _handler.handle(target, baseRequest, request, response);
+            super.handle(target, baseRequest, request, response);
         }
         finally
         {

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

@@ -0,0 +1,124 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2022 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.servlet;
+
+import java.io.IOException;
+import java.util.concurrent.LinkedBlockingQueue;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.eclipse.jetty.server.handler.DefaultHandler;
+import org.eclipse.jetty.server.handler.HandlerCollection;
+import org.eclipse.jetty.server.handler.ResourceHandler;
+import org.eclipse.jetty.server.handler.gzip.GzipHandler;
+import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
+import org.eclipse.jetty.util.component.LifeCycle;
+import org.eclipse.jetty.util.resource.PathResource;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+
+/**
+ * Tests of behavior of GzipHandler when Request.isHandled() or Response.isCommitted() is true
+ */
+public class GzipHandlerIsHandledTest
+{
+    public WorkDir workDir;
+
+    private Server server;
+    private HttpClient client;
+    public LinkedBlockingQueue<String> events = new LinkedBlockingQueue<>();
+
+    public void startServer(Handler rootHandler) throws Exception
+    {
+        server = new Server();
+        ServerConnector connector = new ServerConnector(server);
+        connector.setPort(0);
+        server.addConnector(connector);
+
+        server.setHandler(rootHandler);
+        server.start();
+
+        client = new HttpClient();
+        client.start();
+    }
+
+    @AfterEach
+    public void tearDown()
+    {
+        LifeCycle.stop(client);
+        LifeCycle.stop(server);
+    }
+
+    @Test
+    public void testRequest() throws Exception
+    {
+        HandlerCollection handlers = new HandlerCollection();
+
+        ResourceHandler resourceHandler = new ResourceHandler();
+        resourceHandler.setBaseResource(new PathResource(workDir.getPath()));
+        resourceHandler.setDirectoriesListed(true);
+        resourceHandler.setHandler(new EventHandler(events, "ResourceHandler"));
+
+        GzipHandler gzipHandler = new GzipHandler();
+        gzipHandler.setMinGzipSize(32);
+        gzipHandler.setHandler(new EventHandler(events, "GzipHandler-wrapped-handler"));
+
+        handlers.setHandlers(new Handler[]{resourceHandler, gzipHandler, new DefaultHandler()});
+
+        startServer(handlers);
+
+        ContentResponse response = client.GET(server.getURI().resolve("/"));
+        assertThat("response.status", response.getStatus(), is(200));
+        // we should have received a directory listing from the ResourceHandler
+        assertThat("response.content", response.getContentAsString(), containsString("Directory: /"));
+        // resource handler should have handled the request
+        // the gzip handler and default handlers should have been executed, seeing as this is a HandlerCollection
+        // but the gzip handler should not have acted on the request, as the response is committed
+        assertThat("One event should have been recorded", events.size(), is(1));
+        // the event handler should see the request.isHandled = true
+        // and response.isCommitted = true as the gzip handler didn't really do anything due to these
+        // states and let the wrapped handler (the EventHandler in this case) make the call on what it should do.
+        assertThat("Event indicating that GzipHandler-wrapped-handler ran", events.remove(), is("GzipHandler-wrapped-handler [request.isHandled=true, response.isCommitted=true]"));
+    }
+
+    private static class EventHandler extends AbstractHandler
+    {
+        private final LinkedBlockingQueue<String> events;
+        private final String action;
+
+        public EventHandler(LinkedBlockingQueue<String> events, String action)
+        {
+            this.events = events;
+            this.action = action;
+        }
+
+        @Override
+        public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
+        {
+            events.offer(String.format("%s [request.isHandled=%b, response.isCommitted=%b]", action, baseRequest.isHandled(), response.isCommitted()));
+        }
+    }
+}