Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue #1418: WebSocketServer sometimes misses GET request after SSL handshake #1419

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Fix WebSocketServer sometimes missing GET request
On Ubuntu 22.04 with Linux 6.5, it was observed that when the server
gets the SSL records containing the client handshake finished message
and the first HTTP GET request in ONE read operation, the latter SSL
record is never processed.

Commit 89eaf41 should have fixed this,
but it turned out that when SSLSocketChannel2#processHandshake() is
called from SSLSocketChannel2#write(), the second SSL record containing
the HTTP GET request is stashed away, but never retrieved, since the
calling code in WebSocketServer#doWrite() has no provisions for this,
only WebSocketServer#doRead() does.

Change SSLSocketChannel2#processHandshake() to only read from the socket
when called from SSLSocketChannel#read(), to ensure that when two SSL
records are read, the second one is processed as well.

This fixes issue #1418.
  • Loading branch information
robert-s-ubi committed Jun 8, 2024
commit ad3d043f6a09f6f3bd36e64e2eb41ec5f7606730
12 changes: 6 additions & 6 deletions src/main/java/org/java_websocket/SSLSocketChannel2.java
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public SSLSocketChannel2(SocketChannel channel, SSLEngine sslEngine, ExecutorSer
createBuffers(sslEngine.getSession());
// kick off handshake
socketChannel.write(wrap(emptybuffer));// initializes res
processHandshake();
processHandshake(false);
}

private void consumeFutureUninterruptible(Future<?> f) {
Expand All @@ -148,7 +148,7 @@ private void consumeFutureUninterruptible(Future<?> f) {
* This method will do whatever necessary to process the sslEngine handshake. Thats why it's
* called both from the {@link #read(ByteBuffer)} and {@link #write(ByteBuffer)}
**/
private synchronized void processHandshake() throws IOException {
private synchronized void processHandshake(boolean isReading) throws IOException {
if (sslEngine.getHandshakeStatus() == HandshakeStatus.NOT_HANDSHAKING) {
return; // since this may be called either from a reading or a writing thread and because this method is synchronized it is necessary to double check if we are still handshaking.
}
Expand All @@ -167,7 +167,7 @@ private synchronized void processHandshake() throws IOException {
}
}

if (sslEngine.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP) {
if (isReading && sslEngine.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP) {
if (!isBlocking() || readEngineResult.getStatus() == Status.BUFFER_UNDERFLOW) {
inCrypt.compact();
int read = socketChannel.read(inCrypt);
Expand Down Expand Up @@ -273,7 +273,7 @@ protected void createBuffers(SSLSession session) {

public int write(ByteBuffer src) throws IOException {
if (!isHandShakeComplete()) {
processHandshake();
processHandshake(false);
return 0;
}
// assert(bufferallocations > 1); // see #190
Expand Down Expand Up @@ -303,10 +303,10 @@ public int read(ByteBuffer dst) throws IOException {
if (!isHandShakeComplete()) {
if (isBlocking()) {
while (!isHandShakeComplete()) {
processHandshake();
processHandshake(true);
}
} else {
processHandshake();
processHandshake(true);
if (!isHandShakeComplete()) {
return 0;
}
Expand Down
Loading