search

chiArtem / unitt

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

Fragment end sent with additional messages causes loss of messages #34

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. Receive a message fragment
2. Receive a buffer containing the end of the fragment, and one or more 
additional messages
3. Entire incoming buffer is appended to the fragment's byte array
4. Bytes beyond what are needed to complete the fragment are ignored

What is the expected output? What do you see instead?
Expect to not lose messages.  Instead messages are lost.

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

Please provide any additional information below.
WebSocketConnection.handleMessageData only checks to see if aData.length is 
longer than the last message.  In the scenario above the extra bytes have 
already been added to the previous fragment.

Original issue reported on code.google.com by nw...@detroitsci.com on 15 Nov 2011 at 5:05

GoogleCodeExporter commented 9 years ago 
something like this should do it:
                int usedLength = fragment.getMessageLength() - (lastLength == -1 ? 0 : lastLength);
                // if we have extra data, handle it
                if (aData.length > usedLength) {
                    int remaining = aData.length - usedLength;
                    ioLogger.debug("have additional data to parse: " + remaining);
                    handleMessageData(WebSocketUtil.copySubArray(aData, usedLength, remaining));

Original comment by nw...@detroitsci.com on 15 Nov 2011 at 5:13

GoogleCodeExporter commented 9 years ago 
(where lastLength is

int lastLength = -1;
...

else if ( fragment != null )
        {
            lastLength = fragment.getFragment().length;

Original comment by nw...@detroitsci.com on 15 Nov 2011 at 5:14

GoogleCodeExporter commented 9 years ago 
I will take a look at this this week. Expect a build with a fix over the 
weekend. Thank you for reporting the issue.

Original comment by joshuadmorris@gmail.com on 16 Nov 2011 at 5:59

GoogleCodeExporter commented 9 years ago 
If it wasn't clear, the real issue is the test we're using to determine if 
we've used all of the latest byte array to complete the fragment.  the test 
after "// if we have extra data, handle it" is comparing the full size of the 
completed message fragment - which includes bytes that we'd already received - 
to the amount of bytes that were in the latest array.  The real question it 
needs to be asking is, how much of the latest byte array did we use (and did we 
use it all)?  That's the fix in the code above.

Original comment by nw...@detroitsci.com on 16 Nov 2011 at 8:18

GoogleCodeExporter commented 9 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 9 years ago 
After updating to the latest version on the trunk, I'm still seeing the same 
problem. Please consider applying the following patch to 
iOS/UnittWebSocketClient. Thanks.

Original comment by wh101...@gmail.com on 10 Dec 2011 at 2:15

Attachments:

GoogleCodeExporter commented 9 years ago 
I have used an alternate patch from another contributor. Can you see if the fix 
in trunk addresses your issue?

Original comment by joshuadmorris@gmail.com on 30 Jan 2012 at 9:42

GoogleCodeExporter commented 9 years ago 
Our fork of the code has drifted away from yours quite a lot by now, there was 
a mistake in the cut I sent you that I had to fix locally.

Original comment by nw...@detroitsci.com on 30 Jan 2012 at 10:02

GoogleCodeExporter commented 9 years ago

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

GoogleCodeExporter commented 9 years ago 
Unfortunately, the alternate patch does not solve the problem. In my patch, I 
keep track of bytesConsumed and compare that to [aData length] instead of 
comparing it to fragment.messageLength.
To illustrate the problem, assume you have 2 fragments of 4097 bytes each, and 
a server that sends the data
in 3 chunks as follows: 4096, 4096, 2. In the patch you applied, you will lose 
the first 4095 bytes of the second fragment. Here's a sequence from the output 
of a program (using my patch) that shows the difference:

fragment.messageLength = 434, fragment.bytesConsumed = 434, [aData length] = 434
fragment.messageLength = 2431, fragment.bytesConsumed = 927, [aData length] = 
927
fragment.messageLength = 136, fragment.bytesConsumed = 136, [aData length] = 136
fragment.messageLength = 8196, fragment.bytesConsumed = 192, [aData length] = 
192
fragment.messageLength = 1954, fragment.bytesConsumed = 450, [aData length] = 
450
fragment.messageLength = 144, fragment.bytesConsumed = 144, [aData length] = 144
fragment.messageLength = 5796, fragment.bytesConsumed = 292, [aData length] = 
292
fragment.messageLength = 8196, fragment.bytesConsumed = 192, [aData length] = 
192

This program fails using the applied patch. As for the recursive call to 
handleMessageData, it will probably be optimized out by the compiler, but I 
tend to avoid recursion. That's up to you.

Thanks.

Original comment by wh101...@gmail.com on 1 Feb 2012 at 10:49