Merge pull request #3467 from eclipse/jetty-9.4.x-3422-wss-close-wait

Issue #3422 - WebSocket wss CLOSE_WAIT on aborted client connection
This commit is contained in:
Joakim Erdfelt 2019-03-22 11:36:40 -05:00 committed by GitHub
commit 65528f76c5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 72 additions and 19 deletions

View File

@ -38,7 +38,6 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
public class CloseTrackingEndpoint extends WebSocketAdapter
{
@ -66,11 +65,7 @@ public class CloseTrackingEndpoint extends WebSocketAdapter
assertThat("Client Close Event Occurred", closeLatch.await(clientTimeoutMs, TimeUnit.MILLISECONDS), is(true));
assertThat("Client Close Event Count", closeCount.get(), is(1));
assertThat("Client Close Event Status Code", closeCode, statusCodeMatcher);
if (reasonMatcher == null)
{
assertThat("Client Close Event Reason", closeReason, nullValue());
}
else
if (reasonMatcher != null)
{
assertThat("Client Close Event Reason", closeReason, reasonMatcher);
}

View File

@ -20,9 +20,12 @@ package org.eclipse.jetty.websocket.tests.client;
import java.io.IOException;
import java.net.URI;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.DefaultHandler;
@ -36,6 +39,10 @@ import org.eclipse.jetty.websocket.api.StatusCode;
import org.eclipse.jetty.websocket.api.WebSocketListener;
import org.eclipse.jetty.websocket.api.util.WSURI;
import org.eclipse.jetty.websocket.client.WebSocketClient;
import org.eclipse.jetty.websocket.common.WebSocketSession;
import org.eclipse.jetty.websocket.common.WebSocketSessionListener;
import org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection;
import org.eclipse.jetty.websocket.server.WebSocketServerFactory;
import org.eclipse.jetty.websocket.servlet.WebSocketServlet;
import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
import org.eclipse.jetty.websocket.tests.CloseTrackingEndpoint;
@ -43,16 +50,21 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* Tests for conditions due to bad networking.
*/
public class BadNetworkTest
{
private static final Logger LOG = Log.getLogger(BadNetworkTest.class);
private Server server;
private WebSocketClient client;
private ServletContextHandler context;
private ServerConnector connector;
@BeforeEach
public void startClient() throws Exception
@ -67,20 +79,21 @@ public class BadNetworkTest
{
server = new Server();
ServerConnector connector = new ServerConnector(server);
connector = new ServerConnector(server);
connector.setPort(0);
server.addConnector(connector);
ServletContextHandler context = new ServletContextHandler();
context = new ServletContextHandler();
context.setContextPath("/");
ServletHolder holder = new ServletHolder(new WebSocketServlet()
{
@Override
public void configure(WebSocketServletFactory factory)
{
factory.getPolicy().setIdleTimeout(10000);
factory.getPolicy().setIdleTimeout(100000);
factory.getPolicy().setMaxTextMessageSize(1024 * 1024 * 2);
factory.register(ServerEndpoint.class);
factory.setCreator((req, resp) -> new ServerEndpoint());
}
});
context.addServlet(holder, "/ws");
@ -108,6 +121,37 @@ public class BadNetworkTest
@Test
public void testAbruptClientClose() throws Exception
{
AtomicReference<WebSocketSession> serverSessionRef = new AtomicReference<>();
CountDownLatch serverCloseLatch = new CountDownLatch(1);
connector.addBean(new Connection.Listener() {
@Override
public void onOpened(Connection connection)
{
}
@Override
public void onClosed(Connection connection)
{
serverCloseLatch.countDown();
}
});
CountDownLatch sessionCloseLatch = new CountDownLatch(1);
WebSocketServerFactory wssf = (WebSocketServerFactory) context.getServletContext().getAttribute(WebSocketServletFactory.class.getName());
wssf.addSessionListener(new WebSocketSessionListener() {
@Override
public void onSessionOpened(WebSocketSession session)
{
serverSessionRef.set(session);
}
@Override
public void onSessionClosed(WebSocketSession session)
{
sessionCloseLatch.countDown();
}
});
CloseTrackingEndpoint wsocket = new CloseTrackingEndpoint();
URI wsUri = WSURI.toWebsocket(server.getURI().resolve("/ws"));
@ -116,14 +160,23 @@ public class BadNetworkTest
// Validate that we are connected
future.get(30,TimeUnit.SECONDS);
WebSocketSession serverSession = serverSessionRef.get();
// Have client disconnect abruptly
Session session = wsocket.getSession();
LOG.info("client.disconnect");
session.disconnect();
// Client Socket should see a close event, with status NO_CLOSE
// This event is automatically supplied by the underlying WebSocketClientConnection
// in the situation of a bad network connection.
wsocket.assertReceivedCloseEvent(5000, is(StatusCode.NO_CLOSE), containsString(""));
assertTrue(serverCloseLatch.await(1, TimeUnit.SECONDS), "Server Connection Close should have happened");
assertTrue(sessionCloseLatch.await(1, TimeUnit.SECONDS), "Server Session Close should have happened");
AbstractWebSocketConnection conn = (AbstractWebSocketConnection) serverSession.getConnection();
assertThat("Connection.isOpen", conn.isOpen(), is(false));
}
@Test

View File

@ -60,6 +60,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import static java.time.Duration.ofSeconds;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
@ -248,9 +249,8 @@ public class ClientCloseTest
// client reads -1 (EOF)
// client triggers close event on client ws-endpoint
clientSocket.assertReceivedCloseEvent(clientTimeout * 2,
is(StatusCode.SHUTDOWN),
containsString("timeout"));
// assert - close code==1006 (abnormal) or code==1001 (shutdown)
clientSocket.assertReceivedCloseEvent(clientTimeout * 2, anyOf(is(StatusCode.SHUTDOWN), is(StatusCode.ABNORMAL)));
}
clientSessionTracker.assertClosedProperly(client);
@ -357,9 +357,6 @@ public class ClientCloseTest
EndPoint endp = clientSocket.getEndPoint();
endp.shutdownOutput();
// TODO: race condition. Client CLOSE actions racing SERVER close actions.
// SECONDS.sleep(1); // let server detect EOF and respond
// client enqueue close frame
// should result in a client write failure
final String origCloseReason = "Normal Close from Client";
@ -369,8 +366,8 @@ public class ClientCloseTest
assertThat("OnError", clientSocket.error.get(), instanceOf(EofException.class));
// client triggers close event on client ws-endpoint
// assert - close code==1006 (abnormal)
clientSocket.assertReceivedCloseEvent(timeout, is(StatusCode.ABNORMAL), containsString("Eof"));
// assert - close code==1006 (abnormal) or code==1001 (shutdown)
clientSocket.assertReceivedCloseEvent(timeout, anyOf(is(StatusCode.SHUTDOWN), is(StatusCode.ABNORMAL)));
clientSessionTracker.assertClosedProperly(client);
}

View File

@ -446,6 +446,10 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp
if (readMode == ReadMode.EOF)
{
readState.eof();
// Handle case where the remote connection was abruptly terminated without a close frame
CloseInfo close = new CloseInfo(StatusCode.SHUTDOWN);
close(close, new DisconnectCallback(this));
}
else if (!readState.suspend())
{

View File

@ -38,6 +38,7 @@ import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.MappedByteBufferPool;
import org.eclipse.jetty.server.ConnectionFactory;
@ -628,7 +629,10 @@ public class WebSocketServerFactory extends ContainerLifeCycle implements WebSoc
// Setup websocket connection
AbstractWebSocketConnection wsConnection = new WebSocketServerConnection(endp, executor, scheduler, driver.getPolicy(), bufferPool);
for (Connection.Listener listener : connector.getBeans(Connection.Listener.class))
wsConnection.addListener(listener);
extensionStack.setPolicy(driver.getPolicy());
extensionStack.configure(wsConnection.getParser());
extensionStack.configure(wsConnection.getGenerator());