Description
Describe the bug
The WebSocketWorker
does not catch thrown (non-Exception
) Throwable
s; instead they cause the worker thread to die. Even though in general catching Throwable
should be avoided it seems appropriate here, otherwise an adversary could exploit a vulnerability in the application, killing one worker thread after the other until no more workers are remaining, causing a denial of service.
Even though java.lang.Error
is described as:
An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch.
Some of the subclasses are thrown for non-serious problems, e.g. StackOverflowError
could occur when malicious deeply nested data is deserialized.
To Reproduce
Steps to reproduce the behavior:
- Modify the
onMessage(WebSocket, String)
method of the exampleChatServer
to throw ajava.lang.Error
(simulating an error in the application code) - Start the server
- Run the
ExampleClient
multiple times
❌ At some point all worker threads have died
Expected behavior
WebSocketWorker
should catch Throwable
and log it as fatal error, but it should not terminate due to it.
Alternatively shut down the complete server so whoever started it will notice that an error occurred (and can restart the server).
Environment
- Version used: 1.5.3-SNAPSHOT (5f8d597)
Additional context
It appears in previous versions the following catch
block for RuntimeException
s would have caused a complete server shutdown, see #207:
Java-WebSocket/src/main/java/org/java_websocket/server/WebSocketServer.java
Lines 1082 to 1084 in 5f8d597
However, in current versions that
catch
block cannot be reached (except for bugs caused by this library) since the called method doDecode
catches and logs any thrown Exception
.