Nethravathitcs / unitt

Automatically exported from code.google.com/p/unitt
0 stars 0 forks source link

16 bit payload sizes over 32768 become negative due to cast to short #32

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. send a fragment size over 32768 bytes
2. first 7 payload bits -> size of 126, triggering 16 bit size.  spec calls for 
this to be a 16 bit uint.
3. 16 bit size is passed through WebSocketUtil.convertBytesToShort(buffer, 
index); creating negative size
4. ???
5. Profit

What is the expected output? What do you see instead?
Size should always be a unsigned integer.  instead a negative size is 
calculated (for example, -12668 instead of the expected 52856), and the read 
thread (silently) dies since there is no trapping/reporting of 
RuntimeExceptions like 

java.lang.NegativeArraySizeException
    at com.unitt.framework.websocket.WebSocketUtil.copySubArray(WebSocketUtil.java:39)
    at com.unitt.framework.websocket.WebSocketFragment.parseContent(WebSocketFragment.java:356)
    at com.unitt.framework.websocket.WebSocketFragment.<init>(WebSocketFragment.java:80)
    at com.unitt.framework.websocket.WebSocketConnection.handleMessageData(WebSocketConnection.java:540)
    at com.unitt.framework.websocket.WebSocketConnection.onReceivedData(WebSocketConnection.java:186)
    at com.unitt.framework.websocket.WebSocketClientConnection.onReceivedData(WebSocketClientConnection.java:46)
    at com.unitt.framework.websocket.simple.NetworkSocket.run(NetworkSocket.java:296)
    at java.lang.Thread.run(Thread.java:636)

What version of the product are you using? On what operating system?
HEAD on CentOS 6.0 (final)

Please provide any additional information below.
Fixing this locally Monday morning, will attach patch then.  also probably will 
include some other minor fixes I've added.

Original issue reported on code.google.com by nw...@detroitsci.com on 11 Nov 2011 at 8:25

GoogleCodeExporter commented 8 years ago
Excellent catch! I can't believe I missed this! Thank you so much. I will get a 
fix out this weekend.

Original comment by joshuadmorris@gmail.com on 14 Nov 2011 at 3:12

GoogleCodeExporter commented 8 years ago
While looking into a local fix for this, I realized that there is a fundamental 
problem with storing the packet bytes in a byte array.  Since Java limits 
arrays to (int), a message with a 64 bit length cannot be allocated into an 
array of bytes.  In parseHeader() we're casting the dataLength to an int which 
isn't going to work long term.  In the short term I've changed my local copy to:

else if ( dataLength == 127 )
{
  throw new RuntimeException("Library cannot support Long byte arrays");
}

Original comment by nw...@detroitsci.com on 14 Nov 2011 at 3:44

GoogleCodeExporter commented 8 years ago
Just noticed there is a snippet of code already existing that throws an 
exception if the size exceeds MAX_INT.

Original comment by nw...@detroitsci.com on 14 Nov 2011 at 4:27

GoogleCodeExporter commented 8 years ago
Sorry for the delay, I am now working on a fix. The holidays are playing havoc 
with my schedule.

Original comment by joshuadmorris@gmail.com on 2 Dec 2011 at 1:19

GoogleCodeExporter commented 8 years ago
I still need to fix a couple other issues and run tests, but this should be 
working in trunk now.

Original comment by joshuadmorris@gmail.com on 2 Dec 2011 at 4:44

GoogleCodeExporter commented 8 years ago

Original comment by joshuadmorris@gmail.com on 31 Jan 2012 at 6:32