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 2bdde164a63..d46a4a40e52 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,7 +18,9 @@ package org.springframework.boot.web.embedded.jetty; 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; @@ -26,17 +28,27 @@ 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; + /** * Variation of Jetty's {@link ErrorHandler} that supports all {@link HttpMethod - * HttpMethods} rather than just {@code GET}, {@code POST} and {@code HEAD}. Jetty - * intentionally only + * 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 * prefers Tomcat, Jetty and Undertow to all behave in the same way. * * @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) { @@ -46,11 +58,17 @@ class JettyEmbeddedErrorHandler extends ErrorHandler implements ErrorPageMapper @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) { + public boolean errorPageForMethod(String method) { // Available in Jetty 9.4.21+ return true; } @@ -62,4 +80,25 @@ class JettyEmbeddedErrorHandler extends ErrorHandler implements ErrorPageMapper 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(); + } + + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyEmbeddedLegacyErrorHandler.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyEmbeddedLegacyErrorHandler.java deleted file mode 100644 index b005c286c97..00000000000 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyEmbeddedLegacyErrorHandler.java +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright 2012-2019 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.web.embedded.jetty; - -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; - -/** - * Variation of Jetty's {@link ErrorHandler} that supports all {@link HttpMethod - * HttpMethods} rather than just {@code GET}, {@code POST} and {@code HEAD}. Jetty - * intentionally only - * supports a limited set of HTTP methods for error pages, however, Spring Boot - * prefers Tomcat, Jetty and Undertow to all behave in the same way. - * - * @author Phillip Webb - * @deprecated As of 2.2.0 in favor of {@link JettyEmbeddedErrorHandler} due to error - * handling changes in Jetty 9.4.21.v20190926 - */ -@Deprecated -class JettyEmbeddedLegacyErrorHandler extends ErrorHandler { - - private final ErrorHandler delegate; - - JettyEmbeddedLegacyErrorHandler(ErrorHandler delegate) { - this.delegate = delegate; - } - - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) - throws IOException { - 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); - } - - 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(); - } - - } - -} 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 a4cec9f0f1c..8f7b2ca1246 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 @@ -19,7 +19,6 @@ package org.springframework.boot.web.embedded.jetty; import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.lang.reflect.Method; import java.net.InetSocketAddress; import java.net.MalformedURLException; import java.net.URL; @@ -63,7 +62,6 @@ import org.springframework.boot.web.servlet.server.ServletWebServerFactory; import org.springframework.context.ResourceLoaderAware; import org.springframework.core.io.ResourceLoader; import org.springframework.util.Assert; -import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** @@ -342,20 +340,10 @@ public class JettyServletWebServerFactory extends AbstractServletWebServerFactor @Override public void configure(WebAppContext context) throws Exception { ErrorHandler errorHandler = context.getErrorHandler(); - context.setErrorHandler(wrapErrorHandler(errorHandler)); + context.setErrorHandler(new JettyEmbeddedErrorHandler(errorHandler)); addJettyErrorPages(errorHandler, getErrorPages()); } - @SuppressWarnings("deprecation") - private ErrorHandler wrapErrorHandler(ErrorHandler errorHandler) { - Method method = ReflectionUtils.findMethod(ErrorHandler.class, "errorPageForMethod", String.class); - // Versions prior to 9.4.21.v20190926 have different error handling - if (method == null) { - return new JettyEmbeddedLegacyErrorHandler(errorHandler); - } - return new JettyEmbeddedErrorHandler(errorHandler); - } - }; } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/AbstractJettyServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/AbstractJettyServletWebServerFactoryTests.java new file mode 100644 index 00000000000..655f9010e6a --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/AbstractJettyServletWebServerFactoryTests.java @@ -0,0 +1,86 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.web.embedded.jetty; + +import java.nio.charset.Charset; +import java.util.Locale; +import java.util.Map; + +import org.apache.jasper.servlet.JspServlet; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.webapp.WebAppContext; + +import org.springframework.boot.web.server.PortInUseException; +import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactory; +import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactoryTests; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Abstract base class for {@link JettyServletWebServerFactory} tests. + * + * @author Phillip Webb + */ +public abstract class AbstractJettyServletWebServerFactoryTests extends AbstractServletWebServerFactoryTests { + + @Override + protected JettyServletWebServerFactory getFactory() { + return new JettyServletWebServerFactory(0); + } + + @Override + protected void addConnector(int port, AbstractServletWebServerFactory factory) { + ((JettyServletWebServerFactory) factory).addServerCustomizers((server) -> { + ServerConnector connector = new ServerConnector(server); + connector.setPort(port); + server.addConnector(connector); + }); + } + + @Override + protected JspServlet getJspServlet() throws Exception { + WebAppContext context = (WebAppContext) ((JettyWebServer) this.webServer).getServer().getHandler(); + ServletHolder holder = context.getServletHandler().getServlet("jsp"); + if (holder == null) { + return null; + } + holder.start(); + holder.initialize(); + return (JspServlet) holder.getServlet(); + } + + @Override + protected Map getActualMimeMappings() { + WebAppContext context = (WebAppContext) ((JettyWebServer) this.webServer).getServer().getHandler(); + return context.getMimeTypes().getMimeMap(); + } + + @Override + protected Charset getCharset(Locale locale) { + WebAppContext context = (WebAppContext) ((JettyWebServer) this.webServer).getServer().getHandler(); + String charsetName = context.getLocaleEncoding(locale); + return (charsetName != null) ? Charset.forName(charsetName) : null; + } + + @Override + protected void handleExceptionCausedByBlockedPort(RuntimeException ex, int blockedPort) { + assertThat(ex).isInstanceOf(PortInUseException.class); + assertThat(((PortInUseException) ex).getPort()).isEqualTo(blockedPort); + } + +} 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 new file mode 100644 index 00000000000..d13c6191982 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/JettyServlet9419WebServerFactoryTests.java @@ -0,0 +1,45 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.web.embedded.jetty; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.boot.testsupport.runner.classpath.ClassPathExclusions; +import org.springframework.boot.testsupport.runner.classpath.ClassPathOverrides; +import org.springframework.boot.testsupport.runner.classpath.ModifiedClassPathRunner; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link JettyServletWebServerFactory} with Jetty 9.4.19. + * + * @author Phillip Webb + */ +@RunWith(ModifiedClassPathRunner.class) +@ClassPathExclusions({ "jetty-*.jar", "tomcat-embed*.jar" }) +@ClassPathOverrides({ "org.eclipse.jetty:jetty-io:9.4.19.v20190610", "org.eclipse.jetty:jetty-server:9.4.19.v20190610", + "org.eclipse.jetty:jetty-servlet:9.4.19.v20190610", "org.eclipse.jetty:jetty-util:9.4.19.v20190610", + "org.eclipse.jetty:jetty-webapp:9.4.19.v20190610", "org.mortbay.jasper:apache-jsp:8.5.40" }) +public class JettyServlet9419WebServerFactoryTests extends AbstractJettyServletWebServerFactoryTests { + + @Test + public void correctVersionOfJettyUsed() { + assertThat(JettyEmbeddedErrorHandler.ERROR_PAGE_FOR_METHOD_AVAILABLE).isFalse(); + } + +} 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 b6daea12891..494c7e9f4dd 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 @@ -17,17 +17,13 @@ package org.springframework.boot.web.embedded.jetty; import java.net.InetAddress; -import java.nio.charset.Charset; import java.time.Duration; import java.util.Arrays; import java.util.Collection; -import java.util.Locale; -import java.util.Map; import javax.servlet.ServletContextEvent; import javax.servlet.ServletContextListener; -import org.apache.jasper.servlet.JspServlet; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Server; @@ -35,7 +31,6 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.server.handler.HandlerWrapper; -import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.ThreadPool; import org.eclipse.jetty.webapp.Configuration; @@ -43,11 +38,8 @@ import org.eclipse.jetty.webapp.WebAppContext; import org.junit.Test; import org.mockito.InOrder; -import org.springframework.boot.web.server.PortInUseException; import org.springframework.boot.web.server.Ssl; import org.springframework.boot.web.server.WebServerException; -import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactory; -import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactoryTests; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -63,11 +55,11 @@ import static org.mockito.Mockito.mock; * @author Andy Wilkinson * @author Henri Kerola */ -public class JettyServletWebServerFactoryTests extends AbstractServletWebServerFactoryTests { +public class JettyServletWebServerFactoryTests extends AbstractJettyServletWebServerFactoryTests { - @Override - protected JettyServletWebServerFactory getFactory() { - return new JettyServletWebServerFactory(0); + @Test + public void correctVersionOfJettyUsed() { + assertThat(JettyEmbeddedErrorHandler.ERROR_PAGE_FOR_METHOD_AVAILABLE).isTrue(); } @Test @@ -143,15 +135,6 @@ public class JettyServletWebServerFactoryTests extends AbstractServletWebServerF assertThat(server.isStopped()).isTrue(); } - @Override - protected void addConnector(int port, AbstractServletWebServerFactory factory) { - ((JettyServletWebServerFactory) factory).addServerCustomizers((server) -> { - ServerConnector connector = new ServerConnector(server); - connector.setPort(port); - server.addConnector(connector); - }); - } - @Test public void sslEnabledMultiProtocolsConfiguration() { JettyServletWebServerFactory factory = getFactory(); @@ -311,35 +294,4 @@ public class JettyServletWebServerFactoryTests extends AbstractServletWebServerF }); } - @Override - protected JspServlet getJspServlet() throws Exception { - WebAppContext context = (WebAppContext) ((JettyWebServer) this.webServer).getServer().getHandler(); - ServletHolder holder = context.getServletHandler().getServlet("jsp"); - if (holder == null) { - return null; - } - holder.start(); - holder.initialize(); - return (JspServlet) holder.getServlet(); - } - - @Override - protected Map getActualMimeMappings() { - WebAppContext context = (WebAppContext) ((JettyWebServer) this.webServer).getServer().getHandler(); - return context.getMimeTypes().getMimeMap(); - } - - @Override - protected Charset getCharset(Locale locale) { - WebAppContext context = (WebAppContext) ((JettyWebServer) this.webServer).getServer().getHandler(); - String charsetName = context.getLocaleEncoding(locale); - return (charsetName != null) ? Charset.forName(charsetName) : null; - } - - @Override - protected void handleExceptionCausedByBlockedPort(RuntimeException ex, int blockedPort) { - assertThat(ex).isInstanceOf(PortInUseException.class); - assertThat(((PortInUseException) ex).getPort()).isEqualTo(blockedPort); - } - }