Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4702 from eclipse/jetty-10.0.x-4603-WS_HTTP2_NPE 

Issue #4603 - avoid NPE during websocket HTTP/2 upgrade
This commit is contained in:
Lachlan 2020-03-27 09:44:24 +11:00 committed by GitHub
parent 2969a0a7af 85ea14e33d
commit d595c59622
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 31 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java
View File

@ -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();
}
}

6
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
View File

@ -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

1
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java
View File

@ -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;
}

2
jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/server/internal/AbstractHandshaker.java
View File

@ -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);

16
jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java
View File

@ -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