Skip to content

Commit 3edc6c9

Browse files
lachlan-robertslachlan-roberts
authored
Issue #3734 - throw ISE for WebSocket suspend after close (jetty-9.4) (#4098)
* Issue #3734 - throw ISE for WebSocket suspend after close Signed-off-by: Lachlan Roberts <lachlan@webtide.com> * Issue #3734 - suspend is error if onClose() has been called Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
1 parent 11b60db commit 3edc6c9

4 files changed

Lines changed: 15 additions & 21 deletions

File tree

‎jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/SuspendResumeTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import static org.hamcrest.MatcherAssert.assertThat;
4242
import static org.hamcrest.Matchers.is;
4343
import static org.junit.jupiter.api.Assertions.assertNull;
44+
import static org.junit.jupiter.api.Assertions.assertThrows;
4445
import static org.junit.jupiter.api.Assertions.assertTrue;
4546

4647
public class SuspendResumeTest
@@ -195,8 +196,7 @@ public void testSuspendAfterClose() throws Exception
195196
assertThat(clientSocket.closeCode, is(StatusCode.NORMAL));
196197
assertThat(serverSocket.closeCode, is(StatusCode.NORMAL));
197198

198-
// suspend the client so that no read events occur
199-
SuspendToken suspendToken = clientSocket.session.suspend();
200-
suspendToken.resume();
199+
// suspend after closed throws ISE
200+
assertThrows(IllegalStateException.class, () -> clientSocket.session.suspend());
201201
}
202202
}

‎jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Rem
6868
private final EventDriver websocket;
6969
private final Executor executor;
7070
private final WebSocketPolicy policy;
71-
private final AtomicBoolean closed = new AtomicBoolean();
71+
private final AtomicBoolean onCloseCalled = new AtomicBoolean(false);
7272
private ClassLoader classLoader;
7373
private ExtensionFactory extensionFactory;
7474
private RemoteEndpointFactory remoteEndpointFactory;
@@ -80,7 +80,6 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Rem
8080
private UpgradeRequest upgradeRequest;
8181
private UpgradeResponse upgradeResponse;
8282
private CompletableFuture<Session> openFuture;
83-
private AtomicBoolean onCloseCalled = new AtomicBoolean(false);
8483

8584
public WebSocketSession(WebSocketContainerScope containerScope, URI requestURI, EventDriver websocket, LogicalConnection connection)
8685
{
@@ -338,10 +337,9 @@ public void incomingFrame(Frame frame)
338337
public boolean isOpen()
339338
{
340339
if (this.connection == null)
341-
{
342340
return false;
343-
}
344-
return !closed.get() && this.connection.isOpen();
341+
342+
return !onCloseCalled.get() && this.connection.isOpen();
345343
}
346344

347345
@Override
@@ -546,6 +544,9 @@ public void setUpgradeResponse(UpgradeResponse response)
546544
@Override
547545
public SuspendToken suspend()
548546
{
547+
if (onCloseCalled.get())
548+
throw new IllegalStateException("Not open");
549+
549550
return connection.suspend();
550551
}
551552

‎jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/ReadState.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,8 @@ Action getAction(ByteBuffer buffer)
8787

8888
/**
8989
* Requests that reads from the connection be suspended.
90-
*
91-
* @return whether the suspending was successful
9290
*/
93-
boolean suspending()
91+
void suspending()
9492
{
9593
synchronized (this)
9694
{
@@ -101,9 +99,7 @@ boolean suspending()
10199
{
102100
case READING:
103101
state = State.SUSPENDING;
104-
return true;
105-
case EOF:
106-
return false;
102+
break;
107103
default:
108104
throw new IllegalStateException(toString(state));
109105
}
@@ -131,8 +127,6 @@ ByteBuffer resume()
131127
ByteBuffer bb = buffer;
132128
buffer = null;
133129
return bb;
134-
case EOF:
135-
return null;
136130
default:
137131
throw new IllegalStateException(toString(state));
138132
}

‎jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/io/ReadStateTest.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import static org.hamcrest.Matchers.is;
2828
import static org.junit.jupiter.api.Assertions.assertNull;
2929
import static org.junit.jupiter.api.Assertions.assertThrows;
30-
import static org.junit.jupiter.api.Assertions.assertTrue;
3130

3231
public class ReadStateTest
3332
{
@@ -50,7 +49,7 @@ public void testSuspendingThenResume()
5049
ReadState readState = new ReadState();
5150
assertThat("Initially reading", readState.isReading(), is(true));
5251

53-
assertTrue(readState.suspending());
52+
readState.suspending();
5453
assertThat("Suspending doesn't take effect immediately", readState.isSuspended(), is(false));
5554

5655
assertNull(readState.resume());
@@ -64,7 +63,7 @@ public void testSuspendingThenSuspendThenResume()
6463
ReadState readState = new ReadState();
6564
assertThat("Initially reading", readState.isReading(), is(true));
6665

67-
assertThat(readState.suspending(), is(true));
66+
readState.suspending();
6867
assertThat("Suspending doesn't take effect immediately", readState.isSuspended(), is(false));
6968

7069
ByteBuffer content = BufferUtil.toBuffer("content");
@@ -84,8 +83,8 @@ public void testEof()
8483

8584
assertThat(readState.isReading(), is(false));
8685
assertThat(readState.isSuspended(), is(true));
87-
assertThat(readState.suspending(), is(false));
86+
assertThrows(IllegalStateException.class, readState::suspending);
8887
assertThat(readState.getAction(content), is(ReadState.Action.EOF));
89-
assertNull(readState.resume());
88+
assertThrows(IllegalStateException.class, readState::resume);
9089
}
9190
}

0 commit comments

Comments
 (0)