From 035c6d263905833a3e1ee6fde0355cd3ff6c1109 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 2 Mar 2020 16:48:42 +1100 Subject: [PATCH] Issue #4603 - avoid NPE during websocket HTTP/2 upgrade Signed-off-by: Lachlan Roberts --- .../http2/server/HttpTransportOverHTTP2.java | 24 ++++++------------- .../org/eclipse/jetty/server/HttpChannel.java | 2 +- 2 files changed, 8 insertions(+), 18 deletions(-) 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 d9bc67dc04c6..8cb35a62076e 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 @@ -320,12 +320,16 @@ boolean prepareUpgrade() 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 +344,6 @@ public void onCompleted() 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 +355,7 @@ public void onCompleted() // 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 15b917e5d4db..c287e2695b86 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 boolean handle() break; } - // Check if an update is done (if so, do not close) + // If an upgrade is attempted and failed with sendError call, then do not close here. if (checkAndPrepareUpgrade()) break;