Skip to content

Conversation

@raydelto
Copy link
Contributor

NullPointerException is making the server to crash. This is also a security breach because an intruder could make your server to crash by simply connecting and start typing random characters. This commit would fix this.

@threat
Copy link

threat commented Jun 7, 2015

@raydelto Would it be possible to get steps to reproduce the issue that this pull fixes?

@raydelto
Copy link
Contributor Author

raydelto commented Jun 7, 2015

Hi @threat these are are the steps for reproducing the issue:

1- Run the ChatServer Example provided within the Java-WebSoket build distribution.
2- Connect via telnet to the port where Java-WebSocket is listening (8887 by default).
3- Type random sequences of characters and line breaks (\n by hitting enter).
4- The server will stop answering requests(because it crashed due to a null pointer exception)

This is the stack trace:
java.lang.NullPointerException
at org.java_websocket.SocketChannelIOHelper.batch(SocketChannelIOHelper.
java:64)
at org.java_websocket.server.WebSocketServer.run(WebSocketServer.java:35
2)
at java.lang.Thread.run(Thread.java:745)

threat pushed a commit that referenced this pull request Jun 7, 2015
Preventing the server to crash because of a NPE. Issue #224
@threat threat merged commit 2b8ecd6 into TooTallNate:master Jun 7, 2015
@threat
Copy link

threat commented Jun 7, 2015

Thanks @raydelto.

Just some things I've noticed:

  1. Might be worth catching specific NullPointerException instead of plain Exception
  2. The server no longer crashes after this fix but I'm wondering if the exception should even occur at all?

@raydelto
Copy link
Contributor Author

raydelto commented Jun 7, 2015

@threat

  1. I agree.
    2A. Someone might open a browser and open the URL http://localhost:8887 , which would make the error to occur.
    2B. Someone with bad intentions might intentionally exploit this issue in order to make the server stop answering requests, it will make the WebSocket server unavailable.

@threat
Copy link

threat commented Jun 7, 2015

@raydelto That makes sense. Maybe we could even introduce a new exception specifically for that case...

maakolk pushed a commit to maakolk/Java-WebSocket that referenced this pull request Mar 5, 2017
Preventing the server to crash because of a NPE. Issue TooTallNate#224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants