Since 2.2.2 Jetty error handler cannot be overridden

[oleborup]

We use Spring Boot with Jetty and Jersey for microservices. We want to expose as little server information as possible so the Jetty server is customized with silent error handler to return no content on errors.

/**
 * Custom Jetty container with no server tag and content empty error response
 */
@Bean
public ServletWebServerFactory servletContainer() {
    JettyServletWebServerFactory factory = new JettyServletWebServerFactory();
    factory.setDocumentRoot(new File(System.getProperty("java.io.tmpdir")));
    factory.addServerCustomizers(server -> {
        Handler handler = server.getHandler();
        StatisticsHandler stats = new StatisticsHandler();
        stats.setHandler(handler);
        server.setHandler(stats);
        if (handler instanceof GzipHandler) {
            handler = ((GzipHandler) handler).getHandler();
        }
        WebAppContext context = (WebAppContext) handler;
        context.setErrorHandler(new SilentErrorHandler());
        for (Connector connector : server.getConnectors()) {
            for (ConnectionFactory connFac : connector.getConnectionFactories()) {
                if (connFac instanceof HttpConnectionFactory) {
                    ((HttpConnectionFactory) connFac).getHttpConfiguration().setSendServerVersion(false);
                }
            }
        }
    });
    return factory;
}

After upgrade from 2.2.1 to 2.2.2 the custom error handler is no longer invoked.

A minimal sample can be found here: https://github.com/oleborup/spring-boot-jetty-jersey-error-handling

Tests will fail when changing version from 2.2.1 to 2.2.2.

In some cases we use MVC for static resources and property(ServletProperties.FILTER_STATIC_CONTENT_REGEX, "(/.*\\.html)"); in JerseyConfig. For errors in the static matched cases the default MVC error handler started to be invoked after upgrade to 2.2.2. Solved with @EnableAutoConfiguration(exclude = {ErrorMvcAutoConfiguration.class}) but again the default Jetty error handler was invoked by JettyEmbeddedErrorHandler and not the custom error handler.

Best regards, Ole

[oleborup]

A diff shows the reason for the changed functionality

git diff v2.2.1.RELEASE v2.2.2.RELEASE  spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java                                                                                                                                                             tags/v2.2.1.RELEASE^0
diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java
index 8876432d36..63445ec0d2 100644
--- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java
+++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java
@@ -342,8 +342,8 @@ public class JettyServletWebServerFactory extends AbstractServletWebServerFactor

                        @Override
                        public void configure(WebAppContext context) throws Exception {
-                               ErrorHandler errorHandler = context.getErrorHandler();
-                               context.setErrorHandler(new JettyEmbeddedErrorHandler(errorHandler));
+                               JettyEmbeddedErrorHandler errorHandler = new JettyEmbeddedErrorHandler();
+                               context.setErrorHandler(errorHandler);
                                addJettyErrorPages(errorHandler, getErrorPages());
                        }

[bclozel]

This is related to #18842.

[oleborup]

This is related to #18842.

The change will make it impossible to customize the error handler, or am I missing something?

[wilkinsona]

We may have to chat with the Jetty team again. A part of their recommendations was the following:

Your custom ErrorHandler shouldn't delegate, it should just extend ErrorPageErrorHandler (if using the Context based error handling)

That's the reason for JettyEmbeddedErrorHandler no longer considering any existing error handler.

A couple of possibilities that we could explore:

• Skip the configuration of JettyEmbeddedErrorHandler if a custom error handler has already been configured
• Find an alternative to the approach shown above for configuring a custom error handler so that it can replace JettyEmbeddedErrorHandler

[wilkinsona]

@oleborup Here's an alternative approach that allows you to override Boot's JettyEmbeddedErrorHandler:

	@Bean
	public JettyServerCustomizer jettyCustomizer() {
		return (server) -> {
			WebAppContext context = (WebAppContext)server.getHandler();
			Configuration[] configurations = context.getConfigurations();
			Configuration[] modifiedConfigurations = new Configuration[configurations.length + 1];
			System.arraycopy(configurations, 0, modifiedConfigurations, 0, configurations.length);
			modifiedConfigurations[configurations.length] = new CustomErrorHandlerConfiguration();
			context.setConfigurations(modifiedConfigurations);
		};
	}
	
	private static class CustomErrorHandlerConfiguration extends AbstractConfiguration {

		@Override
		public void configure(WebAppContext context) throws Exception {
			System.out.println("Existing error handler: " + context.getErrorHandler());
			context.setErrorHandler(new SilentErrorHandler());
		}
		
	}

The getting and setting of the configurations on the WebAppContext is rather cumbersome. Unfortunately, it's necessary at the moment as the configuration that sets up the JettyEmbeddedErrorHandler is added after any that have been configured on JettyServletWebServerFactory:

spring-boot/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java

Lines 330 to 331 in 2916304

	configurations.addAll(getConfigurations());
	configurations.add(getErrorPageConfiguration());

If this ordering was reversed, you could just add CustomErrorHandlerConfiguration to JettyServletWebServerFactory. I'm not sure that the current ordering is intentional, but it's been that way pretty much since the beginning. We might be able to change it as I can't think of a reason why we'd want to prevent overriding of the error page handler (or the MIME type configuration which also currently goes after user configurations). Flagging for team attention to see if I'm overlooking something.

[wilkinsona]

We're going to switch the ordering so that the user's configurations are added after all of Boot's own configurations.