Align Jetty's compression of non-GET requests with Tomcat and Undertow

[CharlesLenk]

The following was tested with SpringBoot 1.4.4.RELEASE and Jetty 9.3.16.

When enabling server compression through yaml configuration as described in the documentation, I found that response bodies for my POST requests weren't being compressed even when specifying "gzip" in the Accept-Encoding header.

Through debugging, I found that Jetty's GzipHandler.class supports including/excluding HTTP methods for gzip. By default, the GET verb is included when constructing a GzipHandler, but other methods are not.

I confirmed the issue by enabling an endpoint for both the POST and GET verbs, and sending an identical request using each verb. When using the GET verb, the response is compressed. When using POST, the response is not compressed and GzipHandler logs a debug message indicating that the request was excluded based on the HTTP method.

It appears there's not a way to configure the HTTP methods for GzipHandler through the SpringBoot yaml configuration. Is this an oversight?

[wilkinsona]

Is this an oversight?

No, not really. Generally speaking we don't expose every possible configuration setting unless we see demand for them. You're the first person who wants to configure the HTTP methods for which compression will be performed via application.properties. We also only generally only expose functionality that's supported across all three containers (Jetty, Tomcat, and Undertow) and Jetty's the only one with built-in support for only performing compression for particular HTTP methods.

Note that you don't have to use Boot's built-in configuration of the handler. Rather than setting the various compression-related properties, you could use a JettyServerCustomizer to configure the handler yourself. Something like this:

@Bean
public JettyEmbeddedServletContainerFactory jettyFactory() {
	JettyEmbeddedServletContainerFactory jettyFactory = new JettyEmbeddedServletContainerFactory();
	jettyFactory.addServerCustomizers((server) -> {
		GzipHandler gzipHandler = new GzipHandler();
		gzipHandler.setHandler(server.getHandler());
		gzipHandler.setIncludedMethods("GET", "POST");
		server.setHandler(gzipHandler);
	});
	return jettyFactory;
}

Does this meet your needs?

[CharlesLenk]

Yes, that configuration should work great for my needs, thank you.

It may be worth updating the documentation to indicate that additional configuration may be needed to enable compression based on the embedded servlet container being used. After stepping through with the debugger and finding the issue, my mind first went to "hey, they forgot a property!", but using a ServerCustomizer is also a perfectly reasonable solution. I think the root of the issue is that it's possible for a reasonable person to follow the documentation and end up with compression that doesn't work consistently (at least with Jetty), and there's no indication in the documentation that it shouldn't work "out of the box."

[CharlesLenk]

I've had a chance to review the recommended solution. I believe the line "server.setHandler(server.getHandler());" should have been "server.setHandler(gzipHandler);" After updating that line, the solution worked.

Thinking about it a bit more, I do have some added concerns. In customizing the GzipHandler through a ServerCustomizer, the yml properties aren't being used at all. In effect, this is saying that SpringBoot's compression configuration properties aren't compatible with Jetty if you need to compress POST responses (a very likely scenario). The entire point of these properties is that they should be an easy way to configure compression across servlet containers, so while configuring the compression via a customizer has resolved my issue, I still feel that there's an inconsistency between the intent of the compression properties and the implementation.

[wilkinsona]

In effect, this is saying that SpringBoot's compression configuration properties aren't compatible with Jetty if you need to compress POST responses

Agreed.

(a very likely scenario)

I don't agree with this. I can't speak for Jetty's developers but I strongly suspect that GZipHandler defaults to only compressing GET requests as they believe that's the best setting for most people.

I still feel that there's an inconsistency between the intent of the compression properties and the implementation

The intent is to expose common configuration settings that are work across all three containers that we support. The implementation is consistent with this intent.

Jetty is unique in having built-in support for only compressing the responses to requests with a particular HTTP method. We could build something similar on top of Undertow using a custom predicate, but Tomcat has no such support so we couldn't expose it as a general purpose configuration setting.

One change that we could consider making here is to configure Jetty's Gzip handler to perform compression for responses for all HTTP methods rather than just GET requests. That would deviate from Jetty's defaults but would align with what Tomcat and Undertow to. It could be a breaking change for some so if we did this it may have to wait for 2.0.

Another change we could consider making is a new server.jetty.compression.included-methods property that can be used alongside the existing server.compression properties.

Let's see what the rest of the team thinks.

[wilkinsona]

We're going to change our auto-configuration of Jetty's GZipHandler so that it will compress requests with any method (not just GETs).

[ksperling]

Another difference I've noticed between the the Undertow and Jetty implementations is that the Undertow implementation seems to handle a flush() of the output stream correctly in terms of flushing the deflater as well, whereas Jetty needs a GzipHandler.setSyncFlush(true) to achieve the same behaviour.

This is obviously more of a corner case than which HTTP methods are cached, but it seems worthwhile aligning this as well, especially given that the Undertow behaviour feels like the more reasonable default -- the application should be able to rely on flush being an actual flush.

[shorn]

@wilkinsona
Argh! Andy - your jettyFactory() method has a slight bug in it.

It's not actually assigning the gzipHandler reference, you're setting the original server.getHandler() reference - resulting in the whole gzipHandler just being ignored.

Can you edit your code snippet so nobody else goes off down the rabbit-hole trying to figure out why they can't get the handler working?

The one I'm using:

  @Bean
  public JettyEmbeddedServletContainerFactory jettyFactory() {
  	JettyEmbeddedServletContainerFactory jettyFactory = 
          new JettyEmbeddedServletContainerFactory();
  	jettyFactory.addServerCustomizers((server) -> {
  		GzipHandler gzipHandler = new GzipHandler();
  		gzipHandler.setHandler(server.getHandler());
  		gzipHandler.setIncludedMethods("GET", "POST");
  		gzipHandler.addIncludedMimeTypes("application/json");
  		server.setHandler(gzipHandler);
  	});
  	return jettyFactory;
  }

[wilkinsona]

@shorn Sorry, and thank you for pointing out my mistake. I’ve updated the snippet to set the GZIP handler on the server