From 3f0367e2dd4e07bfe8228f47dc20e28bd3b6f8c2 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 5 Nov 2019 13:24:52 +0000 Subject: [PATCH] Apply changes recommended by Jetty team to JettyEmbeddedErrorHandler Closes gh-18842 --- .../jetty/JettyEmbeddedErrorHandler.java | 68 +++---------------- .../jetty/JettyServletWebServerFactory.java | 4 +- ...JettyServlet9419WebServerFactoryTests.java | 3 +- .../JettyServletWebServerFactoryTests.java | 8 ++- 4 files changed, 19 insertions(+), 64 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyEmbeddedErrorHandler.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyEmbeddedErrorHandler.java index d46a4a40e528..170f4680fdae 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyEmbeddedErrorHandler.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyEmbeddedErrorHandler.java @@ -18,20 +18,15 @@ import java.io.IOException; -import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.handler.ErrorHandler; -import org.eclipse.jetty.server.handler.ErrorHandler.ErrorPageMapper; - -import org.springframework.util.ReflectionUtils; +import org.eclipse.jetty.servlet.ErrorPageErrorHandler; /** - * Variation of Jetty's {@link ErrorHandler} that supports all {@link HttpMethod + * Variation of Jetty's {@link ErrorPageErrorHandler} that supports all {@link HttpMethod * HttpMethods} rather than just {@code GET}, {@code POST} and {@code HEAD}. By default * Jetty intentionally only * supports a limited set of HTTP methods for error pages, however, Spring Boot @@ -40,65 +35,18 @@ * @author Phillip Webb * @author Christoph Dreis */ -class JettyEmbeddedErrorHandler extends ErrorHandler implements ErrorPageMapper { - - static final boolean ERROR_PAGE_FOR_METHOD_AVAILABLE; - - static { - ERROR_PAGE_FOR_METHOD_AVAILABLE = ReflectionUtils.findMethod(ErrorHandler.class, "errorPageForMethod", - String.class) != null; - } - - private final ErrorHandler delegate; - - JettyEmbeddedErrorHandler(ErrorHandler delegate) { - this.delegate = delegate; - } +class JettyEmbeddedErrorHandler extends ErrorPageErrorHandler { @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) - throws IOException { - if (!ERROR_PAGE_FOR_METHOD_AVAILABLE) { - String method = request.getMethod(); - if (!HttpMethod.GET.is(method) && !HttpMethod.POST.is(method) && !HttpMethod.HEAD.is(method)) { - request = new ErrorHttpServletRequest(request); - } - } - this.delegate.handle(target, baseRequest, request, response); - } - - @Override - public boolean errorPageForMethod(String method) { // Available in Jetty 9.4.21+ + public boolean errorPageForMethod(String method) { return true; } @Override - public String getErrorPage(HttpServletRequest request) { - if (this.delegate instanceof ErrorPageMapper) { - return ((ErrorPageMapper) this.delegate).getErrorPage(request); - } - return null; - } - - private static class ErrorHttpServletRequest extends HttpServletRequestWrapper { - - private boolean simulateGetMethod = true; - - ErrorHttpServletRequest(HttpServletRequest request) { - super(request); - } - - @Override - public String getMethod() { - return (this.simulateGetMethod ? HttpMethod.GET.toString() : super.getMethod()); - } - - @Override - public ServletContext getServletContext() { - this.simulateGetMethod = false; - return super.getServletContext(); - } - + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + throws IOException { + baseRequest.setMethod("GET"); + super.doError(target, baseRequest, request, response); } } 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 8f7b2ca12462..a00863291eb0 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 @@ -339,8 +339,8 @@ private Configuration getErrorPageConfiguration() { @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()); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServlet9419WebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServlet9419WebServerFactoryTests.java index d13c6191982e..b0f6109d2c45 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServlet9419WebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServlet9419WebServerFactoryTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.web.embedded.jetty; +import org.eclipse.jetty.server.handler.ErrorHandler; import org.junit.Test; import org.junit.runner.RunWith; @@ -39,7 +40,7 @@ public class JettyServlet9419WebServerFactoryTests extends AbstractJettyServletW @Test public void correctVersionOfJettyUsed() { - assertThat(JettyEmbeddedErrorHandler.ERROR_PAGE_FOR_METHOD_AVAILABLE).isFalse(); + assertThat(ErrorHandler.class.getPackage().getImplementationVersion()).isEqualTo("9.4.19.v20190610"); } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactoryTests.java index 494c7e9f4dd3..67e528198070 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactoryTests.java @@ -20,6 +20,8 @@ import java.time.Duration; import java.util.Arrays; import java.util.Collection; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.servlet.ServletContextEvent; import javax.servlet.ServletContextListener; @@ -29,6 +31,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; +import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.server.handler.HandlerWrapper; import org.eclipse.jetty.util.thread.QueuedThreadPool; @@ -59,7 +62,10 @@ public class JettyServletWebServerFactoryTests extends AbstractJettyServletWebSe @Test public void correctVersionOfJettyUsed() { - assertThat(JettyEmbeddedErrorHandler.ERROR_PAGE_FOR_METHOD_AVAILABLE).isTrue(); + String jettyVersion = ErrorHandler.class.getPackage().getImplementationVersion(); + Matcher matcher = Pattern.compile("[0-9]+.[0-9]+.([0-9]+)[\\.-].*").matcher(jettyVersion); + assertThat(matcher.find()).isTrue(); + assertThat(Integer.valueOf(matcher.group(1))).isGreaterThan(19); } @Test