diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java index d9bc67dc04c..e8ff9759227 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java @@ -314,18 +314,25 @@ public class HttpTransportOverHTTP2 implements HttpTransport return transportCallback.onIdleTimeout(failure); } + /** + * @return true if error sent, false if upgraded or aborted. + */ boolean prepareUpgrade() { HttpChannelOverHTTP2 channel = (HttpChannelOverHTTP2)stream.getAttachment(); Request request = channel.getRequest(); if (request.getHttpInput().hasContent()) return channel.sendErrorOrAbort("Unexpected content in CONNECT request"); + Connection connection = (Connection)request.getAttribute(UPGRADE_CONNECTION_ATTRIBUTE); + if (connection == null) + return channel.sendErrorOrAbort("No UPGRADE_CONNECTION_ATTRIBUTE available"); + EndPoint endPoint = connection.getEndPoint(); endPoint.upgrade(connection); stream.setAttachment(endPoint); - // Only now that we have switched the attachment, - // we can demand DATA frames to process them. + + // Only now that we have switched the attachment, we can demand DATA frames to process them. stream.demand(1); if (LOG.isDebugEnabled()) @@ -340,21 +347,6 @@ public class HttpTransportOverHTTP2 implements HttpTransport Object attachment = stream.getAttachment(); if (attachment instanceof HttpChannelOverHTTP2) { - // TODO: we used to "fake" a 101 response to upgrade the endpoint - // but we don't anymore, so this code should be deleted. - HttpChannelOverHTTP2 channel = (HttpChannelOverHTTP2)attachment; - if (channel.getResponse().getStatus() == HttpStatus.SWITCHING_PROTOCOLS_101) - { - Connection connection = (Connection)channel.getRequest().getAttribute(UPGRADE_CONNECTION_ATTRIBUTE); - EndPoint endPoint = connection.getEndPoint(); - // TODO: check that endPoint implements HTTP2Channel. - if (LOG.isDebugEnabled()) - LOG.debug("Tunnelling DATA frames through {}", endPoint); - endPoint.upgrade(connection); - stream.setAttachment(endPoint); - return; - } - // If the stream is not closed, it is still reading the request content. // Send a reset to the other end so that it stops sending data. if (!stream.isClosed()) @@ -366,6 +358,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport // Consume the existing queued data frames to // avoid stalling the session flow control. + HttpChannelOverHTTP2 channel = (HttpChannelOverHTTP2)attachment; channel.consumeInput(); } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index 15b917e5d4d..7da6a20c4ca 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -490,7 +490,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor break; } - // Check if an update is done (if so, do not close) + // If send error is called we need to break. if (checkAndPrepareUpgrade()) break; @@ -527,6 +527,10 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor return !suspended; } + /** + * @param message the error message. + * @return true if we have sent an error, false if we have aborted. + */ public boolean sendErrorOrAbort(String message) { try diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java index 277ea84df2e..1994f27288b 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java @@ -479,7 +479,6 @@ public class HttpChannelOverHttp extends HttpChannel implements HttpParser.Reque if (LOG.isDebugEnabled()) LOG.debug("Upgrade from {} to {}", getEndPoint().getConnection(), upgradeConnection); getRequest().setAttribute(HttpTransport.UPGRADE_CONNECTION_ATTRIBUTE, upgradeConnection); - getResponse().setStatus(HttpStatus.SWITCHING_PROTOCOLS_101); getHttpTransport().onCompleted(); return true; } diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java index 77062cfc100..afd98b662d8 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java @@ -160,12 +160,12 @@ public abstract class AbstractHandshaker implements Handshaker coreSession.setWebSocketConnection(connection); + baseRequest.setHandled(true); Response baseResponse = baseRequest.getResponse(); prepareResponse(baseResponse, negotiation); if (httpConfig.getSendServerVersion()) baseResponse.getHttpFields().put(SERVER_VERSION); baseResponse.flushBuffer(); - baseRequest.setHandled(true); baseRequest.setAttribute(HttpTransport.UPGRADE_CONNECTION_ATTRIBUTE, connection); diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java index 46353096544..1cd8a05caa4 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java @@ -23,6 +23,7 @@ import java.io.InterruptedIOException; import java.net.ConnectException; import java.net.URI; import java.nio.channels.ClosedChannelException; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.function.Function; @@ -294,11 +295,21 @@ public class WebSocketOverHTTP2Test startServer(); startClient(clientConnector -> new ClientConnectionFactoryOverHTTP2.H2(new HTTP2Client(clientConnector))); + CountDownLatch latch = new CountDownLatch(1); + connector.addBean(new HttpChannel.Listener() + { + @Override + public void onComplete(Request request) + { + latch.countDown(); + } + }); + EventSocket wsEndPoint = new EventSocket(); URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/ws/throw"); ExecutionException failure; - try (StacklessLogging stacklessLogging = new StacklessLogging(HttpChannel.class)) + try (StacklessLogging ignored = new StacklessLogging(HttpChannel.class)) { failure = Assertions.assertThrows(ExecutionException.class, () -> wsClient.connect(wsEndPoint, uri).get(5, TimeUnit.SECONDS)); @@ -307,6 +318,9 @@ public class WebSocketOverHTTP2Test Throwable cause = failure.getCause(); assertThat(cause, instanceOf(UpgradeException.class)); assertThat(cause.getMessage(), containsStringIgnoringCase("Unexpected HTTP Response Status Code: 500")); + + // Wait for the request to complete on server before stopping. + assertTrue(latch.await(5, TimeUnit.SECONDS)); } @Test