Merge pull request #4838 from eclipse/jetty-9.4.x-4835-gziphandler-flush

Issue #4835 - Addressing flush/commit with GzipHttpOutputInterceptor

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

@@ -229,7 +229,15 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
             LOG.debug("{} compressing {}", this, _deflater);
             _state.set(GZState.COMPRESSING);
 
-            gzip(content, complete, callback);
+            if (BufferUtil.isEmpty(content))
+            {
+                // We are committing, but have no content to compress, so flush empty buffer to write headers.
+                _interceptor.write(BufferUtil.EMPTY_BUFFER, complete, callback);
+            }
+            else
+            {
+                gzip(content, complete, callback);
+            }
         }
         else
             callback.failed(new WritePendingException());
@@ -412,7 +420,7 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
         @Override
         public String toString()
         {
-            return String.format("%s[content=%s last=%b copy=%s buffer=%s deflate=%s",
+            return String.format("%s[content=%s last=%b copy=%s buffer=%s deflate=%s %s]",
                 super.toString(),
                 BufferUtil.toDetailString(_content),
                 _last,

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

@@ -0,0 +1,130 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.servlet;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.Arrays;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import javax.servlet.Servlet;
+import javax.servlet.http.HttpServlet;
+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.client.api.Request;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.server.handler.gzip.GzipHandler;
+import org.eclipse.jetty.util.component.LifeCycle;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class GzipHandlerCommitTest
+{
+    private Server server;
+    private HttpClient client;
+
+    public void start(Servlet servlet) throws Exception
+    {
+        server = new Server();
+        ServerConnector connector = new ServerConnector(server);
+        connector.setPort(0);
+        server.addConnector(connector);
+
+        ServletContextHandler contextHandler = new ServletContextHandler();
+        contextHandler.setContextPath("/");
+        ServletHolder servletHolder = new ServletHolder(servlet);
+        contextHandler.addServlet(servletHolder, "/test/*");
+
+        GzipHandler gzipHandler = new GzipHandler();
+        gzipHandler.setHandler(contextHandler);
+
+        server.setHandler(gzipHandler);
+        server.start();
+
+        client = new HttpClient();
+        client.start();
+    }
+
+    @AfterEach
+    public void tearDown()
+    {
+        LifeCycle.stop(client);
+        LifeCycle.stop(server);
+    }
+
+    @Test
+    public void testImmediateFlushNoContent() throws Exception
+    {
+        CountDownLatch latch = new CountDownLatch(1);
+        start(new HttpServlet()
+        {
+            @Override
+            protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
+            {
+                response.flushBuffer();
+                assertDoesNotThrow(() -> assertTrue(latch.await(1, TimeUnit.SECONDS)));
+            }
+        });
+
+        URI uri = server.getURI().resolve("/test/");
+        Request request = client.newRequest(uri);
+        request.header(HttpHeader.CONNECTION, "Close");
+        request.onResponseHeaders((r) -> latch.countDown());
+        ContentResponse response = request.send();
+        assertThat("Response status", response.getStatus(), is(200));
+    }
+
+    @Test
+    public void testImmediateFlushWithContent() throws Exception
+    {
+        int size = 8000;
+        CountDownLatch latch = new CountDownLatch(1);
+        start(new HttpServlet()
+        {
+            @Override
+            protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
+            {
+                response.flushBuffer();
+                assertDoesNotThrow(() -> assertTrue(latch.await(1, TimeUnit.SECONDS)));
+                response.getOutputStream();
+                byte[] buf = new byte[size];
+                Arrays.fill(buf, (byte)'a');
+                response.getOutputStream().write(buf);
+            }
+        });
+
+        URI uri = server.getURI().resolve("/test/");
+        Request request = client.newRequest(uri);
+        request.header(HttpHeader.CONNECTION, "Close");
+        request.onResponseHeaders((r) -> latch.countDown());
+        ContentResponse response = request.send();
+        assertThat("Response status", response.getStatus(), is(200));
+        assertThat("Response content size", response.getContent().length, is(size));
+    }
+}