From 2ed7d7ebdd74ce330b94cf04fdf1dcb6f9815388 Mon Sep 17 00:00:00 2001 From: marci4 Date: Thu, 19 Oct 2017 21:41:13 +0200 Subject: [PATCH 1/2] Refactoring -A Getter/Setter is now used for ReadyState -Invalid UTF-8 messages which cause onClose will not have a null reason any more --- .../org/java_websocket/WebSocketImpl.java | 42 +++++++++++-------- .../java_websocket/framing/CloseFrame.java | 2 +- .../org/java_websocket/framing/TextFrame.java | 2 +- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/java_websocket/WebSocketImpl.java b/src/main/java/org/java_websocket/WebSocketImpl.java index 1444d869a..86eb5f1a2 100644 --- a/src/main/java/org/java_websocket/WebSocketImpl.java +++ b/src/main/java/org/java_websocket/WebSocketImpl.java @@ -93,6 +93,10 @@ public class WebSocketImpl implements WebSocket { * When true no further frames may be submitted to be sent */ private volatile boolean flushandclosestate = false; + + /** + * The current state of the connection + */ private READYSTATE readystate = READYSTATE.NOT_YET_CONNECTED; /** @@ -197,8 +201,8 @@ public void decode( ByteBuffer socketBuffer ) { if( DEBUG ) System.out.println( "process(" + socketBuffer.remaining() + "): {" + ( socketBuffer.remaining() > 1000 ? "too big to display" : new String( socketBuffer.array(), socketBuffer.position(), socketBuffer.remaining() ) ) + "}" ); - if( readystate != READYSTATE.NOT_YET_CONNECTED ) { - if( readystate == READYSTATE.OPEN ) { + if( getReadyState() != READYSTATE.NOT_YET_CONNECTED ) { + if( getReadyState() == READYSTATE.OPEN ) { decodeFrames( socketBuffer ); } } else { @@ -416,11 +420,11 @@ private ByteBuffer generateHttpResponseDueToError( int errorCode ) { } public void close( int code, String message, boolean remote ) { - if( readystate != READYSTATE.CLOSING && readystate != READYSTATE.CLOSED ) { - if( readystate == READYSTATE.OPEN ) { + if( getReadyState() != READYSTATE.CLOSING && readystate != READYSTATE.CLOSED ) { + if( getReadyState() == READYSTATE.OPEN ) { if( code == CloseFrame.ABNORMAL_CLOSE ) { assert ( !remote ); - readystate = READYSTATE.CLOSING; + setReadyState(READYSTATE.CLOSING); flushAndClose( code, message, false ); return; } @@ -452,7 +456,7 @@ public void close( int code, String message, boolean remote ) { } else { flushAndClose( CloseFrame.NEVER_CONNECTED, message, false ); } - readystate = READYSTATE.CLOSING; + setReadyState(READYSTATE.CLOSING); tmpHandshakeBytes = null; return; } @@ -475,7 +479,7 @@ public void close( int code, String message ) { * remote may also be true if this endpoint started the closing handshake since the other endpoint may not simply echo the code but close the connection the same time this endpoint does do but with an other code.
**/ public synchronized void closeConnection( int code, String message, boolean remote ) { - if( readystate == READYSTATE.CLOSED ) { + if( getReadyState() == READYSTATE.CLOSED ) { return; } @@ -505,8 +509,7 @@ public synchronized void closeConnection( int code, String message, boolean remo draft.reset(); handshakerequest = null; - readystate = READYSTATE.CLOSED; - this.outQueue.clear(); + setReadyState(READYSTATE.CLOSED); } protected void closeConnection( int code, boolean remote ) { @@ -663,7 +666,7 @@ private HandshakeState isFlashEdgeCase( ByteBuffer request ) throws IncompleteHa } public void startHandshake( ClientHandshakeBuilder handshakedata ) throws InvalidHandshakeException { - assert ( readystate != READYSTATE.CONNECTING ) : "shall only be called once"; + assert ( getReadyState() != READYSTATE.CONNECTING ) : "shall only be called once"; // Store the Handshake Request we are about to send this.handshakerequest = draft.postProcessHandshakeRequestAsClient( handshakedata ); @@ -717,7 +720,7 @@ private void write( List bufs ) { private void open( Handshakedata d ) { if( DEBUG ) System.out.println( "open using draft: " + draft ); - readystate = READYSTATE.OPEN; + setReadyState(READYSTATE.OPEN); try { wsl.onWebsocketOpen( this, d ); } catch ( RuntimeException e ) { @@ -727,19 +730,19 @@ private void open( Handshakedata d ) { @Override public boolean isConnecting() { - assert ( !flushandclosestate || readystate == READYSTATE.CONNECTING ); - return readystate == READYSTATE.CONNECTING; // ifflushandclosestate + assert ( !flushandclosestate || getReadyState() == READYSTATE.CONNECTING ); + return getReadyState() == READYSTATE.CONNECTING; // ifflushandclosestate } @Override public boolean isOpen() { - assert ( readystate != READYSTATE.OPEN || !flushandclosestate ); - return readystate == READYSTATE.OPEN; + assert ( getReadyState() != READYSTATE.OPEN || !flushandclosestate ); + return getReadyState() == READYSTATE.OPEN; } @Override public boolean isClosing() { - return readystate == READYSTATE.CLOSING; + return getReadyState() == READYSTATE.CLOSING; } @Override @@ -749,7 +752,7 @@ public boolean isFlushAndClose() { @Override public boolean isClosed() { - return readystate == READYSTATE.CLOSED; + return getReadyState() == READYSTATE.CLOSED; } @Override @@ -757,6 +760,10 @@ public READYSTATE getReadyState() { return readystate; } + private void setReadyState( READYSTATE readystate ) { + this.readystate = readystate; + } + @Override public int hashCode() { return super.hashCode(); @@ -816,4 +823,5 @@ public void updateLastPong() { public WebSocketListener getWebSocketListener() { return wsl; } + } diff --git a/src/main/java/org/java_websocket/framing/CloseFrame.java b/src/main/java/org/java_websocket/framing/CloseFrame.java index 6976ff167..8c73db279 100644 --- a/src/main/java/org/java_websocket/framing/CloseFrame.java +++ b/src/main/java/org/java_websocket/framing/CloseFrame.java @@ -210,7 +210,7 @@ public String toString() { public void isValid() throws InvalidDataException { super.isValid(); if (code == CloseFrame.NO_UTF8 && reason == null) { - throw new InvalidDataException( CloseFrame.NO_UTF8 ); + throw new InvalidDataException( CloseFrame.NO_UTF8, "Received text is no valid utf8 string!"); } if (code == CloseFrame.NOCODE && 0 < reason.length()) { throw new InvalidDataException(PROTOCOL_ERROR, "A close frame must have a closecode if it has a reason"); diff --git a/src/main/java/org/java_websocket/framing/TextFrame.java b/src/main/java/org/java_websocket/framing/TextFrame.java index 50d7553f7..5adae762c 100644 --- a/src/main/java/org/java_websocket/framing/TextFrame.java +++ b/src/main/java/org/java_websocket/framing/TextFrame.java @@ -44,7 +44,7 @@ public TextFrame() { public void isValid() throws InvalidDataException { super.isValid(); if (!Charsetfunctions.isValidUTF8( getPayloadData() )) { - throw new InvalidDataException(CloseFrame.NO_UTF8); + throw new InvalidDataException(CloseFrame.NO_UTF8, "Received text is no valid utf8 string!"); } } } From 4057d17ea11054afd20048c9a84dc4828a114cf7 Mon Sep 17 00:00:00 2001 From: marci4 Date: Thu, 19 Oct 2017 21:44:06 +0200 Subject: [PATCH 2/2] Improve onClose behaviour on client side #577 Client will delay closing the socket until there was an interrupt which normally terminates the WebsocketWriteThread and causes all remaining message to be pushed out (testsuite fails on random test cases, which is probably caused by a high load and is not representive for a real usage scenario) --- .../client/WebSocketClient.java | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/java_websocket/client/WebSocketClient.java b/src/main/java/org/java_websocket/client/WebSocketClient.java index b5f727c0c..bb809da18 100644 --- a/src/main/java/org/java_websocket/client/WebSocketClient.java +++ b/src/main/java/org/java_websocket/client/WebSocketClient.java @@ -370,12 +370,6 @@ public final void onWebsocketClose( WebSocket conn, int code, String reason, boo stopConnectionLostTimer(); if( writeThread != null ) writeThread.interrupt(); - try { - if( socket != null ) - socket.close(); - } catch ( IOException e ) { - onWebsocketError( this, e ); - } onClose( code, reason, remote ); connectLatch.countDown(); closeLatch.countDown(); @@ -464,16 +458,34 @@ private class WebsocketWriteThread implements Runnable { public void run() { Thread.currentThread().setName( "WebsocketWriteThread" ); try { - while( !Thread.interrupted() ) { - ByteBuffer buffer = engine.outQueue.take(); - ostream.write( buffer.array(), 0, buffer.limit() ); - ostream.flush(); + try { + while( !Thread.interrupted() ) { + ByteBuffer buffer = engine.outQueue.take(); + ostream.write( buffer.array(), 0, buffer.limit() ); + ostream.flush(); + } + } catch ( InterruptedException e ) { + for (ByteBuffer buffer : engine.outQueue) { + ostream.write( buffer.array(), 0, buffer.limit() ); + ostream.flush(); + } } } catch ( IOException e ) { - handleIOException(e); - } catch ( InterruptedException e ) { - // this thread is regularly terminated via an interrupt + handleIOException( e ); + } finally { + closeOutputAndSocket(); + } + } + } + + private void closeOutputAndSocket() { + try { + if( socket != null ) { + socket.shutdownOutput(); + socket.close(); } + } catch ( IOException ex ) { + onWebsocketError( this, ex ); } }