Merge pull request #18536 from dreis2211

* pr/18536:
  Polish "Upgrade to Jetty 9.4.21.v20190926"
  Upgrade to Jetty 9.4.21.v20190926

Closes gh-18536

spring-boot-project/spring-boot-dependencies/pom.xml

@@ -107,7 +107,7 @@
 		<jedis.version>2.9.3</jedis.version>
 		<jersey.version>2.27</jersey.version>
 		<jest.version>6.3.1</jest.version>
-		<jetty.version>9.4.19.v20190610</jetty.version>
+		<jetty.version>9.4.21.v20190926</jetty.version>
 		<jetty-jsp.version>2.2.0.v201112011158</jetty-jsp.version>
 		<jetty-el.version>8.5.40</jetty-el.version>
 		<jetty-reactive-httpclient.version>1.0.3</jetty-reactive-httpclient.version>

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

@@ -26,17 +26,28 @@ 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;
 
 /**
  * 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
+ * HttpMethods} rather than just {@code GET}, {@code POST} and {@code HEAD}. By default
+ * 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 {
+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;
 
@@ -47,13 +58,28 @@ class JettyEmbeddedErrorHandler extends ErrorHandler {
 	@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);
+		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+
+		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;

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/AbstractJettyServletWebServerFactoryTests.java

+/*
+ * 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

+/*
+ * 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
-	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<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/servlet/server/AbstractServletWebServerFactoryTests.java

@@ -1073,7 +1073,6 @@ public abstract class AbstractServletWebServerFactoryTests {
 			HttpComponentsClientHttpRequestFactory requestFactory, String... headers)
 			throws IOException, URISyntaxException {
 		ClientHttpRequest request = requestFactory.createRequest(new URI(url), method);
-		request.getHeaders().add("Cookie", "JSESSIONID=" + "123");
 		for (String header : headers) {
 			String[] parts = header.split(":");
 			request.getHeaders().add(parts[0], parts[1]);