From 5873dddc1c9ff41a27eae9ea48f6bf681b859e7e Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 13 Apr 2021 16:15:41 +0100 Subject: [PATCH] Attempt to stabilise web server tests that use h2c Apache HttpClient 5.1 doesn't cope with Jetty 10 sending SETTINGS_ENABLE_CONNECT_PROTOCOL in the settings frame. It also appears to be unstable when using Undertow, resulting in a failure and "UT005032: Listener not making progress on framed channel, closing channel to prevent infinite loop" being logged on the server-side. Local experimentation suggests that Jetty's HTTP/2 client is more robust and that it does not trigger the problem with Undertow. It also fixes the problem with SETTINGS_ENABLE_CONNECT_PROTOCOL when testing against Jetty 10 so this commit updates the tests to use Jetty's client. Closes gh-26040 --- spring-boot-project/spring-boot/build.gradle | 4 +- .../Jetty10ReactiveWebServerFactoryTests.java | 7 --- .../Jetty10ServletWebServerFactoryTests.java | 7 --- .../web/embedded/jetty/TestWithJetty10.java | 11 +++-- ...AbstractReactiveWebServerFactoryTests.java | 46 +++++++------------ .../AbstractServletWebServerFactoryTests.java | 42 ++++++----------- 6 files changed, 38 insertions(+), 79 deletions(-) diff --git a/spring-boot-project/spring-boot/build.gradle b/spring-boot-project/spring-boot/build.gradle index ad71998ea54..0d63abe74a4 100644 --- a/spring-boot-project/spring-boot/build.gradle +++ b/spring-boot-project/spring-boot/build.gradle @@ -98,8 +98,10 @@ dependencies { testImplementation("mysql:mysql-connector-java") testImplementation("net.sourceforge.jtds:jtds") testImplementation("org.apache.derby:derby") - testImplementation("org.apache.httpcomponents:httpasyncclient") testImplementation("org.awaitility:awaitility") + testImplementation("org.eclipse.jetty:jetty-client") + testImplementation("org.eclipse.jetty.http2:http2-client") + testImplementation("org.eclipse.jetty.http2:http2-http-client-transport") testImplementation("org.firebirdsql.jdbc:jaybird-jdk18") testImplementation("org.hsqldb:hsqldb") testImplementation("org.junit.jupiter:junit-jupiter") diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/Jetty10ReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/Jetty10ReactiveWebServerFactoryTests.java index 05b05376eb6..c1820a3fc74 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/Jetty10ReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/Jetty10ReactiveWebServerFactoryTests.java @@ -43,11 +43,4 @@ class Jetty10ReactiveWebServerFactoryTests extends JettyReactiveWebServerFactory } - @Test - @Override - @Disabled("https://github.com/eclipse/jetty.project/issues/6164") - protected void whenHttp2IsEnabledAndSslIsDisabledThenH2cCanBeUsed() { - - } - } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/Jetty10ServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/Jetty10ServletWebServerFactoryTests.java index 9d9f7390087..acf6f6ec150 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/Jetty10ServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/Jetty10ServletWebServerFactoryTests.java @@ -50,11 +50,4 @@ public class Jetty10ServletWebServerFactoryTests extends JettyServletWebServerFa protected void jettyConfigurations() throws Exception { } - @Test - @Override - @Disabled("https://github.com/eclipse/jetty.project/issues/6164") - protected void whenHttp2IsEnabledAndSslIsDisabledThenH2cCanBeUsed() { - - } - } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/TestWithJetty10.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/TestWithJetty10.java index a54e7ec748f..3cfe9631a33 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/TestWithJetty10.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/jetty/TestWithJetty10.java @@ -35,11 +35,12 @@ import org.springframework.boot.testsupport.classpath.ClassPathOverrides; @Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME) @EnabledForJreRange(min = JRE.JAVA_11) -@ClassPathExclusions({ "jetty-*.jar", "tomcat-embed*.jar" }) -@ClassPathOverrides({ "org.slf4j:slf4j-api:1.7.25", "org.eclipse.jetty:jetty-io:10.0.2", - "org.eclipse.jetty:jetty-server:10.0.2", "org.eclipse.jetty:jetty-servlet:10.0.2", - "org.eclipse.jetty:jetty-util:10.0.2", "org.eclipse.jetty:jetty-webapp:10.0.2", - "org.eclipse.jetty.http2:http2-common:10.0.2", "org.eclipse.jetty.http2:http2-hpack:10.0.2", +@ClassPathExclusions({ "jetty-*.jar", "tomcat-embed*.jar", "http2-*.jar" }) +@ClassPathOverrides({ "org.slf4j:slf4j-api:1.7.25", "org.eclipse.jetty:jetty-client:10.0.2", + "org.eclipse.jetty:jetty-io:10.0.2", "org.eclipse.jetty:jetty-server:10.0.2", + "org.eclipse.jetty:jetty-servlet:10.0.2", "org.eclipse.jetty:jetty-util:10.0.2", + "org.eclipse.jetty:jetty-webapp:10.0.2", "org.eclipse.jetty.http2:http2-common:10.0.2", + "org.eclipse.jetty.http2:http2-hpack:10.0.2", "org.eclipse.jetty.http2:http2-http-client-transport:10.0.2", "org.eclipse.jetty.http2:http2-server:10.0.2", "org.mortbay.jasper:apache-jsp:8.5.40" }) @interface TestWithJetty10 { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java index 3f0b2f15594..3fc672d34ec 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java @@ -41,15 +41,12 @@ import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpResponse; import io.netty.handler.ssl.SslProvider; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; -import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; -import org.apache.hc.client5.http.async.methods.SimpleHttpRequests; -import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; -import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; -import org.apache.hc.client5.http.impl.async.HttpAsyncClients; -import org.apache.hc.core5.concurrent.FutureCallback; -import org.apache.hc.core5.http.ContentType; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.awaitility.Awaitility; +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.client.util.StringContentProvider; +import org.eclipse.jetty.http2.client.HTTP2Client; +import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import reactor.core.publisher.Mono; @@ -459,35 +456,24 @@ public abstract class AbstractReactiveWebServerFactoryTests { } @Test - protected void whenHttp2IsEnabledAndSslIsDisabledThenH2cCanBeUsed() - throws InterruptedException, ExecutionException, IOException { + protected void whenHttp2IsEnabledAndSslIsDisabledThenH2cCanBeUsed() throws Exception { AbstractReactiveWebServerFactory factory = getFactory(); Http2 http2 = new Http2(); http2.setEnabled(true); factory.setHttp2(http2); this.webServer = factory.getWebServer(new EchoHandler()); this.webServer.start(); - try (CloseableHttpAsyncClient http2Client = HttpAsyncClients.createHttp2Default()) { - http2Client.start(); - SimpleHttpRequest request = SimpleHttpRequests.post("http://localhost:" + this.webServer.getPort()); - request.setBody("Hello World", ContentType.TEXT_PLAIN); - SimpleHttpResponse response = http2Client.execute(request, new FutureCallback() { - - @Override - public void failed(Exception ex) { - } - - @Override - public void completed(SimpleHttpResponse result) { - } - - @Override - public void cancelled() { - } - - }).get(); - assertThat(response.getCode() == HttpStatus.OK.value()); - assertThat(response.getBodyText()).isEqualTo("Hello World"); + org.eclipse.jetty.client.HttpClient client = new org.eclipse.jetty.client.HttpClient( + new HttpClientTransportOverHTTP2(new HTTP2Client())); + client.start(); + try { + ContentResponse response = client.POST("http://localhost:" + this.webServer.getPort()) + .content(new StringContentProvider("Hello World"), "text/plain").send(); + assertThat(response.getStatus() == HttpStatus.OK.value()); + assertThat(response.getContentAsString()).isEqualTo("Hello World"); + } + finally { + client.stop(); } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java index 2cba1b57075..07ddcc80982 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java @@ -79,12 +79,6 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; import org.apache.catalina.webresources.TomcatURLStreamHandlerFactory; -import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; -import org.apache.hc.client5.http.async.methods.SimpleHttpRequests; -import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; -import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; -import org.apache.hc.client5.http.impl.async.HttpAsyncClients; -import org.apache.hc.core5.concurrent.FutureCallback; import org.apache.http.HttpResponse; import org.apache.http.client.HttpClient; import org.apache.http.client.entity.InputStreamFactory; @@ -103,6 +97,9 @@ import org.apache.jasper.EmbeddedServletOptions; import org.apache.jasper.servlet.JspServlet; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.awaitility.Awaitility; +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.http2.client.HTTP2Client; +import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; @@ -1130,35 +1127,22 @@ public abstract class AbstractServletWebServerFactoryTests { } @Test - protected void whenHttp2IsEnabledAndSslIsDisabledThenH2cCanBeUsed() - throws InterruptedException, ExecutionException, IOException { + protected void whenHttp2IsEnabledAndSslIsDisabledThenH2cCanBeUsed() throws Exception { AbstractServletWebServerFactory factory = getFactory(); Http2 http2 = new Http2(); http2.setEnabled(true); factory.setHttp2(http2); this.webServer = factory.getWebServer(exampleServletRegistration()); this.webServer.start(); - try (CloseableHttpAsyncClient http2Client = HttpAsyncClients.createHttp2Default()) { - http2Client.start(); - SimpleHttpRequest request = SimpleHttpRequests - .get("http://localhost:" + this.webServer.getPort() + "/hello"); - SimpleHttpResponse response = http2Client.execute(request, new FutureCallback() { - - @Override - public void failed(Exception ex) { - } - - @Override - public void completed(SimpleHttpResponse result) { - } - - @Override - public void cancelled() { - } - - }).get(); - assertThat(response.getCode()).isEqualTo(HttpStatus.OK.value()); - assertThat(response.getBodyText()).isEqualTo("Hello World"); + org.eclipse.jetty.client.HttpClient client = new org.eclipse.jetty.client.HttpClient( + new HttpClientTransportOverHTTP2(new HTTP2Client())); + client.start(); + try { + ContentResponse response = client.GET("http://localhost:" + this.webServer.getPort() + "/hello"); + assertThat(response.getStatus() == HttpStatus.OK.value()); + } + finally { + client.stop(); } }