Polish "Upgrade to Jetty 9.4.21.v20190926"

See gh-18536

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
+ * HttpMethods} rather than just {@code GET}, {@code POST} and {@code HEAD}. By default
- * <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=446039">intentionally only
+ * Jetty <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=446039">intentionally only
  * supports a limited set of HTTP methods</a> 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();
+		}
+
+	}
+
 }

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

@@ -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
- * <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=446039">intentionally only
- * supports a limited set of HTTP methods</a> 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();
-		}
-
-	}
-
-}

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);
-			}
-
 		};
 	}
 

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<String, String> 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);
+	}
+
+}

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();
+	}
+
+}

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
+	@Test
-	protected JettyServletWebServerFactory getFactory() {
+	public void correctVersionOfJettyUsed() {
-		return new JettyServletWebServerFactory(0);
+		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<String, String> 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);
-	}
-
 }