Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GzipHandler and GzipHttpOutputInterceptor do not flush response when body is empty #4835

Closed
geeknotnerd opened this issue May 1, 2020 · 5 comments · Fixed by #4838
Closed
Labels
Bug For general bugs on Jetty side

Comments

@geeknotnerd
Copy link

geeknotnerd commented May 1, 2020

Jetty version
9.4.15.v20190215
Java version
Java 11
OS type/version
Most Os (Mac and Unix)
Description

Recently we switched to Jetty 9.4 from Jetty 9.0. Jetty 9.4 comes with GzipHandler so we used that. At the request processing time we flush out response to send back (link) headers to browser early by doing response.flushBuffer(), this is done on purpose. In jetty 9.0's GzipFilter response used to be flushed right away but in GzipHandler it isn't flushed (because we are flushing only headers and body is empty). This is causing an increase in wait time in the browser as headers are received a little late on the browser side. I was wondering how can we make this response flush immediately in GzipHanlder in Jetty9.4? apparently GZIP output interceptor doesn't flush response headers when the response.flushBuffer() is called and when the body is empty!

@geeknotnerd geeknotnerd changed the title GzipHandler in Jetty 9.4 - increased wait times GzipHandler in Jetty 9.4 - GzipHttpOutputInterceptor doesn't flush response when body is empty May 1, 2020
@joakime
Copy link
Contributor

joakime commented May 1, 2020

9.4.15 is old. (and has a few CVEs filed against it - https://www.eclipse.org/jetty/security-reports.html)
Please, upgrade your version of Jetty.
Version 9.4.28.v20200408 is the latest on the Jetty 9.4.x series.
https://github.com/eclipse/jetty.project/releases/tag/jetty-9.4.28.v20200408
https://search.maven.org/search?q=g:org.eclipse.jetty%20AND%20v:9.4.28.v20200408

What are your HttpServletResponse headers when you call response.flushBuffer()?
Does debug logging of org.eclipse.jetty.server.handler.gzip tell you anything?

@joakime
Copy link
Contributor

joakime commented May 1, 2020

Looking at the code, it seems that on initial flush the GzipHttpOutputInterceptor.gzip() method executes.

https://github.com/eclipse/jetty.project/blob/ab228fde9e55e9164c738d7fa121f8ac5acd51c9/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java#L135-L141

Which enters with the parameters ...

  • content - java.nio.HeapByteBuffer[pos=0 lim=0 cap=0]
  • complete - false
  • callback - SharedBlockingCallback$Blocker@23b8c642{null}

Which means that that content.hasRemaining() is false, and the call just succeeds the callback without actually writing.
A behavior that is perfectly reasonable for all uses except a demand to flush.

Using HttpServletResponse.flushBuffer() or ServletOutputStream.flush() result in the same behavior.

Also, it seems our GzipHttpOutputInterceptor is violating the HttpOutput.Interceptor API contract, which allows for empty content buffers to trigger a commit.

https://github.com/eclipse/jetty.project/blob/ab228fde9e55e9164c738d7fa121f8ac5acd51c9/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java#L145-L158

@joakime joakime added the Bug For general bugs on Jetty side label May 1, 2020
@joakime joakime added this to To do in Jetty 9.4.29 via automation May 1, 2020
@joakime
Copy link
Contributor

joakime commented May 1, 2020

Small demo project that reproduces the report.

Use curl in verbose mode to follow along in the behavior.

$ curl -H "Accept-Encoding: gzip" -o /dev/null -vvvv http://localhost:8888/demo/
package jetty.gzip;

import java.io.IOException;
import java.util.Arrays;
import java.util.concurrent.ThreadLocalRandom;
import java.util.stream.IntStream;
import javax.servlet.ServletException;
import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.servlet.ServletContextHandler;

public class GzipFlushBufferDemo
{
    public static void main(String[] args) throws Exception
    {
        Server server = new Server();
        ServerConnector connector = new ServerConnector(server);
        connector.setPort(8888);
        server.addConnector(connector);

        ServletContextHandler contextHandler = new ServletContextHandler();
        contextHandler.setContextPath("/");
        contextHandler.addServlet(FlushBufferServlet.class, "/demo/");

        GzipHandler gzipHandler = new GzipHandler();
        gzipHandler.setHandler(contextHandler);

        server.setHandler(gzipHandler);
        server.start();
        server.join();
    }

    public static class FlushBufferServlet extends HttpServlet
    {
        @Override
        protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
        {
            response.setContentType("text/plain");
            ServletOutputStream out = response.getOutputStream();
            response.flushBuffer();

            try { Thread.sleep(5000); } catch (InterruptedException ignored) { }

            byte[] buf = new byte[8000];
            ThreadLocalRandom.current().nextBytes(buf);
            out.write(buf);
        }
    }
}

joakime added a commit that referenced this issue May 1, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@geeknotnerd
Copy link
Author

geeknotnerd commented May 5, 2020

Thanks for the quick response to this. May I know when (ETA) 9.4.29 will be released?

joakime added a commit that referenced this issue May 5, 2020
Issue #4835 - Addressing flush/commit with GzipHttpOutputInterceptor
@joakime
Copy link
Contributor

joakime commented May 5, 2020

Merged into jetty-9.4.x ...

@joakime joakime closed this as completed May 5, 2020
Jetty 9.4.29 automation moved this from To do to Done May 5, 2020
@joakime joakime changed the title GzipHandler in Jetty 9.4 - GzipHttpOutputInterceptor doesn't flush response when body is empty GzipHandler and GzipHttpOutputInterceptor do not flush response when body is empty May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Jetty 9.4.29
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants