From ac6db57db934dc18d8bbc98028af08fdd9f135ca Mon Sep 17 00:00:00 2001 From: Marcel Prestel Date: Fri, 28 Jun 2019 18:18:03 +0200 Subject: [PATCH 1/5] Wrap IOException and include WebSocket Fixes #900 --- .../exceptions/WrappedIOException.java | 74 +++++++++++++++++++ .../server/WebSocketServer.java | 28 ++++--- 2 files changed, 91 insertions(+), 11 deletions(-) create mode 100644 src/main/java/org/java_websocket/exceptions/WrappedIOException.java diff --git a/src/main/java/org/java_websocket/exceptions/WrappedIOException.java b/src/main/java/org/java_websocket/exceptions/WrappedIOException.java new file mode 100644 index 00000000..7d25900f --- /dev/null +++ b/src/main/java/org/java_websocket/exceptions/WrappedIOException.java @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2010-2019 Nathan Rajlich + * + * Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + */ + +package org.java_websocket.exceptions; + +import org.java_websocket.WebSocket; + +import java.io.IOException; + +/** + * Exception to wrap an IOException and include information about the websocket which had the exception + * @since 1.4.1 + */ +public class WrappedIOException extends Throwable { + + /** + * The websocket where the IOException happened + */ + private final WebSocket connection; + + /** + * The IOException + */ + private final IOException ioException; + + /** + * Wrapp an IOException and include the websocket + * @param connection the websocket where the IOException happened + * @param ioException the IOException + */ + public WrappedIOException(WebSocket connection, IOException ioException) { + this.connection = connection; + this.ioException = ioException; + } + + /** + * The websocket where the IOException happened + * @return the websocket for the wrapped IOException + */ + public WebSocket getConnection() { + return connection; + } + + /** + * The wrapped IOException + * @return IOException which is wrapped + */ + public IOException getIOException() { + return ioException; + } +} diff --git a/src/main/java/org/java_websocket/server/WebSocketServer.java b/src/main/java/org/java_websocket/server/WebSocketServer.java index 19331e96..cc04afaf 100644 --- a/src/main/java/org/java_websocket/server/WebSocketServer.java +++ b/src/main/java/org/java_websocket/server/WebSocketServer.java @@ -48,6 +48,7 @@ import org.java_websocket.drafts.Draft; import org.java_websocket.exceptions.InvalidDataException; import org.java_websocket.exceptions.WebsocketNotConnectedException; +import org.java_websocket.exceptions.WrappedIOException; import org.java_websocket.framing.CloseFrame; import org.java_websocket.framing.Framedata; import org.java_websocket.handshake.ClientHandshake; @@ -320,7 +321,6 @@ public void run() { int selectTimeout = 0; while ( !selectorthread.isInterrupted() && iShutdownCount != 0) { SelectionKey key = null; - WebSocketImpl conn = null; try { if (isclosed.get()) { selectTimeout = 5; @@ -334,7 +334,6 @@ public void run() { while ( i.hasNext() ) { key = i.next(); - conn = null; if( !key.isValid() ) { continue; @@ -358,10 +357,10 @@ public void run() { // an other thread may cancel the key } catch ( ClosedByInterruptException e ) { return; // do the same stuff as when InterruptedException is thrown + } catch ( WrappedIOException ex) { + handleIOException( key, ex.getConnection(), ex.getIOException()); } catch ( IOException ex ) { - if( key != null ) - key.cancel(); - handleIOException( key, conn, ex ); + handleIOException( key, null, ex ); } catch ( InterruptedException e ) { // FIXME controlled shutdown (e.g. take care of buffermanagement) Thread.currentThread().interrupt(); @@ -445,7 +444,7 @@ private void doAccept(SelectionKey key, Iterator i) throws IOExcep * @throws InterruptedException thrown by taking a buffer * @throws IOException if an error happened during read */ - private boolean doRead(SelectionKey key, Iterator i) throws InterruptedException, IOException { + private boolean doRead(SelectionKey key, Iterator i) throws InterruptedException, WrappedIOException { WebSocketImpl conn = (WebSocketImpl) key.attachment(); ByteBuffer buf = takeBuffer(); if(conn.getChannel() == null){ @@ -471,7 +470,7 @@ private boolean doRead(SelectionKey key, Iterator i) throws Interr } } catch ( IOException e ) { pushBuffer( buf ); - throw e; + throw new WrappedIOException(conn, e); } return true; } @@ -481,12 +480,16 @@ private boolean doRead(SelectionKey key, Iterator i) throws Interr * @param key the selectionkey to write on * @throws IOException if an error happened during batch */ - private void doWrite(SelectionKey key) throws IOException { + private void doWrite(SelectionKey key) throws WrappedIOException { WebSocketImpl conn = (WebSocketImpl) key.attachment(); - if( SocketChannelIOHelper.batch( conn, conn.getChannel() ) ) { - if( key.isValid() ) { - key.interestOps(SelectionKey.OP_READ); + try { + if (SocketChannelIOHelper.batch(conn, conn.getChannel())) { + if (key.isValid()) { + key.interestOps(SelectionKey.OP_READ); + } } + } catch (IOException e) { + throw new WrappedIOException(conn, e); } } @@ -598,6 +601,9 @@ private void pushBuffer( ByteBuffer buf ) throws InterruptedException { private void handleIOException( SelectionKey key, WebSocket conn, IOException ex ) { // onWebsocketError( conn, ex );// conn may be null here + if (key != null) { + key.cancel(); + } if( conn != null ) { conn.closeConnection( CloseFrame.ABNORMAL_CLOSE, ex.getMessage() ); } else if( key != null ) { From 0f6cbdd385274f79eb2e1ca9f4f8fbcb5caa8e20 Mon Sep 17 00:00:00 2001 From: Marcel Prestel Date: Sat, 13 Jul 2019 10:38:10 +0200 Subject: [PATCH 2/5] Rework after review --- .../java/org/java_websocket/exceptions/WrappedIOException.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/java_websocket/exceptions/WrappedIOException.java b/src/main/java/org/java_websocket/exceptions/WrappedIOException.java index 7d25900f..075de883 100644 --- a/src/main/java/org/java_websocket/exceptions/WrappedIOException.java +++ b/src/main/java/org/java_websocket/exceptions/WrappedIOException.java @@ -34,7 +34,7 @@ * Exception to wrap an IOException and include information about the websocket which had the exception * @since 1.4.1 */ -public class WrappedIOException extends Throwable { +public class WrappedIOException extends Exception { /** * The websocket where the IOException happened From 7890fffd204aa1d81d16554dac86dc80eac845d0 Mon Sep 17 00:00:00 2001 From: marci4 Date: Sun, 19 Jan 2020 19:03:07 +0100 Subject: [PATCH 3/5] Added test for #900 Overwrite channel to always throw IOException --- .../org/java_websocket/WebSocketImpl.java | 2 +- .../java_websocket/issues/Issue900Test.java | 155 ++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/java_websocket/issues/Issue900Test.java diff --git a/src/main/java/org/java_websocket/WebSocketImpl.java b/src/main/java/org/java_websocket/WebSocketImpl.java index a4066975..5c8e1a47 100644 --- a/src/main/java/org/java_websocket/WebSocketImpl.java +++ b/src/main/java/org/java_websocket/WebSocketImpl.java @@ -512,7 +512,7 @@ public synchronized void closeConnection( int code, String message, boolean remo try { channel.close(); } catch ( IOException e ) { - if( e.getMessage().equals( "Broken pipe" ) ) { + if( e.getMessage() != null && e.getMessage().equals( "Broken pipe" ) ) { log.trace( "Caught IOException: Broken pipe during closeConnection()", e ); } else { log.error("Exception during channel.close()", e); diff --git a/src/test/java/org/java_websocket/issues/Issue900Test.java b/src/test/java/org/java_websocket/issues/Issue900Test.java new file mode 100644 index 00000000..5c8c11d1 --- /dev/null +++ b/src/test/java/org/java_websocket/issues/Issue900Test.java @@ -0,0 +1,155 @@ +/* + * Copyright (c) 2010-2020 Nathan Rajlich + * + * Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + */ + +package org.java_websocket.issues; + +import org.java_websocket.WebSocket; +import org.java_websocket.WebSocketImpl; +import org.java_websocket.WrappedByteChannel; +import org.java_websocket.client.WebSocketClient; +import org.java_websocket.handshake.ClientHandshake; +import org.java_websocket.handshake.ServerHandshake; +import org.java_websocket.server.WebSocketServer; +import org.java_websocket.util.SocketUtil; +import org.junit.Test; + +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Method; +import java.net.InetSocketAddress; +import java.net.URI; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.concurrent.CountDownLatch; + +public class Issue900Test { + + CountDownLatch countServerDownLatch = new CountDownLatch(1); + CountDownLatch countCloseCallsDownLatch = new CountDownLatch(1); + + @Test(timeout = 2000) + public void testIssue() throws Exception { + int port = SocketUtil.getAvailablePort(); + final WebSocketClient client = new WebSocketClient(new URI("ws://localhost:" + port)) { + @Override + public void onOpen(ServerHandshake handshakedata) { + + } + + @Override + public void onMessage(String message) { + } + + @Override + public void onClose(int code, String reason, boolean remote) { + } + + @Override + public void onError(Exception ex) { + + } + }; + WebSocketServer server = new WebSocketServer(new InetSocketAddress(port)) { + @Override + public void onOpen(WebSocket conn, ClientHandshake handshake) { + } + + @Override + public void onClose(WebSocket conn, int code, String reason, boolean remote) { + countCloseCallsDownLatch.countDown(); + } + + @Override + public void onMessage(WebSocket conn, String message) { + + } + + @Override + public void onError(WebSocket conn, Exception ex) { + + } + + @Override + public void onStart() { + countServerDownLatch.countDown(); + } + }; + new Thread(server).start(); + countServerDownLatch.await(); + client.connectBlocking(); + WebSocketImpl websocketImpl = (WebSocketImpl)new ArrayList(server.getConnections()).get(0); + websocketImpl.setChannel(new ExceptionThrowingByteChannel()); + server.broadcast("test"); + countCloseCallsDownLatch.await(); + } + class ExceptionThrowingByteChannel implements WrappedByteChannel { + + @Override + public boolean isNeedWrite() { + return false; + } + + @Override + public void writeMore() throws IOException { + throw new IOException(); + } + + @Override + public boolean isNeedRead() { + return false; + } + + @Override + public int readMore(ByteBuffer dst) throws IOException { + throw new IOException(); + } + + @Override + public boolean isBlocking() { + return false; + } + + @Override + public int read(ByteBuffer dst) throws IOException { + throw new IOException(); + } + + @Override + public int write(ByteBuffer src) throws IOException { + throw new IOException(); + } + + @Override + public boolean isOpen() { + return false; + } + + @Override + public void close() throws IOException { + throw new IOException(); + } + } +} From 992da3d4f9ae40568690616bd39b7fbfc2d9b40a Mon Sep 17 00:00:00 2001 From: marci4 Date: Sun, 19 Jan 2020 19:04:25 +0100 Subject: [PATCH 4/5] Update WrappedIOException.java Adjust year --- .../java/org/java_websocket/exceptions/WrappedIOException.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/java_websocket/exceptions/WrappedIOException.java b/src/main/java/org/java_websocket/exceptions/WrappedIOException.java index 075de883..4603ad85 100644 --- a/src/main/java/org/java_websocket/exceptions/WrappedIOException.java +++ b/src/main/java/org/java_websocket/exceptions/WrappedIOException.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2019 Nathan Rajlich + * Copyright (c) 2010-2020 Nathan Rajlich * * Permission is hereby granted, free of charge, to any person * obtaining a copy of this software and associated documentation From 72aebe1d2ff5ce84d116db4caa9c7a8f4bbed8b1 Mon Sep 17 00:00:00 2001 From: marci4 Date: Mon, 20 Jan 2020 20:21:50 +0100 Subject: [PATCH 5/5] Rework after review --- .../java_websocket/issues/Issue900Test.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/test/java/org/java_websocket/issues/Issue900Test.java b/src/test/java/org/java_websocket/issues/Issue900Test.java index 5c8c11d1..c528534e 100644 --- a/src/test/java/org/java_websocket/issues/Issue900Test.java +++ b/src/test/java/org/java_websocket/issues/Issue900Test.java @@ -37,8 +37,6 @@ import org.junit.Test; import java.io.IOException; -import java.io.InputStream; -import java.lang.reflect.Method; import java.net.InetSocketAddress; import java.net.URI; import java.nio.ByteBuffer; @@ -47,8 +45,8 @@ public class Issue900Test { - CountDownLatch countServerDownLatch = new CountDownLatch(1); - CountDownLatch countCloseCallsDownLatch = new CountDownLatch(1); + CountDownLatch serverStartLatch = new CountDownLatch(1); + CountDownLatch closeCalledLatch = new CountDownLatch(1); @Test(timeout = 2000) public void testIssue() throws Exception { @@ -79,7 +77,7 @@ public void onOpen(WebSocket conn, ClientHandshake handshake) { @Override public void onClose(WebSocket conn, int code, String reason, boolean remote) { - countCloseCallsDownLatch.countDown(); + closeCalledLatch.countDown(); } @Override @@ -94,22 +92,22 @@ public void onError(WebSocket conn, Exception ex) { @Override public void onStart() { - countServerDownLatch.countDown(); + serverStartLatch.countDown(); } }; new Thread(server).start(); - countServerDownLatch.await(); + serverStartLatch.await(); client.connectBlocking(); WebSocketImpl websocketImpl = (WebSocketImpl)new ArrayList(server.getConnections()).get(0); websocketImpl.setChannel(new ExceptionThrowingByteChannel()); server.broadcast("test"); - countCloseCallsDownLatch.await(); + closeCalledLatch.await(); } class ExceptionThrowingByteChannel implements WrappedByteChannel { @Override public boolean isNeedWrite() { - return false; + return true; } @Override @@ -119,7 +117,7 @@ public void writeMore() throws IOException { @Override public boolean isNeedRead() { - return false; + return true; } @Override