From ce4c48a87707c61ff366fa417ce4b63b47777531 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 16 Jan 2017 12:13:05 +0100 Subject: [PATCH 1/4] Fixes #1261 - Intermittent H2C test failure AsyncIOServletTest.testAsyncReadEarlyEOF. Delayed abrupt output shutdown in case of HTTP/2, to allow to reply to SETTINGS frames exchanged during the preface. --- .../org/eclipse/jetty/http/client/AsyncIOServletTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/AsyncIOServletTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/AsyncIOServletTest.java index 61f38956422..ebe9980ef59 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/AsyncIOServletTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/AsyncIOServletTest.java @@ -1024,6 +1024,10 @@ public class AsyncIOServletTest extends AbstractTest break; case H2C: case H2: + // In case of HTTP/2, we not only send the request, but also the preface and + // SETTINGS frames. SETTINGS frame need to be replied, so we want to wait to + // write the reply before shutting output down, so that the test does not fail. + Thread.sleep(1000); Session session = ((HttpConnectionOverHTTP2)connection).getSession(); ((HTTP2Session)session).getEndPoint().shutdownOutput(); break; From ddc63066fcf66e6a2b0541e02e3b5246571ac9ae Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 16 Jan 2017 19:05:09 +0100 Subject: [PATCH 2/4] Fixes #1259 - HostnameVerificationTest.simpleGetWithHostnameVerificationEnabledTest is broken. When a fill() triggered by a flush() throws, now the write flusher is failed rather than completed. This ensures that both the read and the write side see the same exception, rather than the write side seeing a ClosedChannelException. --- .../client/HostnameVerificationTest.java | 13 ++++++---- .../eclipse/jetty/io/ssl/SslConnection.java | 26 ++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java index e4b81b3ec3f..8552b1985e3 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.client; import java.io.IOException; -import java.nio.channels.ClosedChannelException; import java.security.cert.CertificateException; import java.util.concurrent.ExecutionException; @@ -48,13 +47,17 @@ import org.junit.Test; public class HostnameVerificationTest { private SslContextFactory clientSslContextFactory = new SslContextFactory(); - private Server server = new Server(); + private Server server; private HttpClient client; private NetworkConnector connector; @Before public void setUp() throws Exception { + QueuedThreadPool serverThreads = new QueuedThreadPool(); + serverThreads.setName("server"); + server = new Server(serverThreads); + SslContextFactory serverSslContextFactory = new SslContextFactory(); serverSslContextFactory.setKeyStorePath("src/test/resources/keystore.jks"); serverSslContextFactory.setKeyStorePassword("storepwd"); @@ -75,10 +78,10 @@ public class HostnameVerificationTest clientSslContextFactory.setKeyStorePath("src/test/resources/keystore.jks"); clientSslContextFactory.setKeyStorePassword("storepwd"); - QueuedThreadPool executor = new QueuedThreadPool(); - executor.setName(executor.getName() + "-client"); + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); client = new HttpClient(clientSslContextFactory); - client.setExecutor(executor); + client.setExecutor(clientThreads); client.start(); } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 469d89f14f9..fd95a5b5267 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -514,6 +514,7 @@ public class SslConnection extends AbstractConnection { synchronized (this) { + Throwable failure = null; try { // Do we already have some decrypted data? @@ -708,6 +709,7 @@ public class SslConnection extends AbstractConnection catch (SSLHandshakeException x) { notifyHandshakeFailed(_sslEngine, x); + failure = x; throw x; } catch (SSLException x) @@ -717,6 +719,12 @@ public class SslConnection extends AbstractConnection x = (SSLException)new SSLHandshakeException(x.getMessage()).initCause(x); notifyHandshakeFailed(_sslEngine, x); } + failure = x; + throw x; + } + catch (Throwable x) + { + failure = x; throw x; } finally @@ -725,7 +733,7 @@ public class SslConnection extends AbstractConnection if (_flushRequiresFillToProgress) { _flushRequiresFillToProgress = false; - getExecutor().execute(_runCompletWrite); + getExecutor().execute(failure == null ? _runCompletWrite : new FailFlusher(failure)); } if (_encryptedInput != null && !_encryptedInput.hasRemaining()) @@ -1080,5 +1088,21 @@ public class SslConnection extends AbstractConnection { return super.toString()+"->"+getEndPoint().toString(); } + + private class FailFlusher implements Runnable + { + private final Throwable failure; + + private FailFlusher(Throwable failure) + { + this.failure = failure; + } + + @Override + public void run() + { + getWriteFlusher().onFail(failure); + } + } } } From 76ba287f6c9e83176827b23070403e41a639665c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 16 Jan 2017 19:09:26 +0100 Subject: [PATCH 3/4] Fixed JUnit JavaDocs URL. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a8a92edd622..2d63e7bf0b0 100644 --- a/pom.xml +++ b/pom.xml @@ -374,7 +374,7 @@ http://docs.oracle.com/javase/8/docs/api/ http://docs.oracle.com/javaee/7/api/ - http://junit.org/javadoc/latest/ + http://junit.org/junit4/javadoc/latest/ http://download.eclipse.org/jetty/stable-9/apidocs/ From 3b49239407c2e548080a9fbd959fe58bb641b1de Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 16 Jan 2017 19:21:52 +0100 Subject: [PATCH 4/4] Renamed class to clarify behavior. --- .../main/java/org/eclipse/jetty/io/ssl/SslConnection.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index fd95a5b5267..23ea2734b8d 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -733,7 +733,7 @@ public class SslConnection extends AbstractConnection if (_flushRequiresFillToProgress) { _flushRequiresFillToProgress = false; - getExecutor().execute(failure == null ? _runCompletWrite : new FailFlusher(failure)); + getExecutor().execute(failure == null ? _runCompletWrite : new FailWrite(failure)); } if (_encryptedInput != null && !_encryptedInput.hasRemaining()) @@ -1089,11 +1089,11 @@ public class SslConnection extends AbstractConnection return super.toString()+"->"+getEndPoint().toString(); } - private class FailFlusher implements Runnable + private class FailWrite implements Runnable { private final Throwable failure; - private FailFlusher(Throwable failure) + private FailWrite(Throwable failure) { this.failure = failure; }