Problem parsing data packages from multiple socket reads

[GoogleCodeExporter]

What steps will reproduce the problem?
1. receiving data from socket in multiple reads

What is the expected output? What do you see instead?
multiple valid fragments

What version of the product are you using? On what operating system?
0.9.3.1

Please provide any additional information below.

I have difficulties with the unitt library when receiving on valid web-socket 
data package in multiple reads. I will try to give you the best possible 
explanation by going step by step through the functions involved

First read
=========

- read 1400 bytes from socket
- handle message data gets called
- a new WebSocketFragment will be created
- parsing the header -- payload size of 1480 is required
- the new fragment is not valid, as there is 0.7 web-socket packages in the 
current read (not a full package)
- fragment is not valid, so function will go to else if (fragment)
- data will be appended to fragment
- fragment is still not valid, as the payload size is higher than the fragments 
actual size
- dead end 

Second read
=========
- the fragment object from first read is in the pendingFragment queue
- the socket again returns 1200 bytes
- handleMessageData is called with the new data 
- the fragment from Read1 is enqueued
- it is not valid so we go to else if (fragment)
- the new data is appended 
- required payload size for a package is still 1480
- but now: [fragment.fragment length] returns 2600
- fragment.isValid will always return false
- dead end for me

This is the involved function; In some (described) situations 
handleCompleteFragment:fragment and handleMessageData is never called.

- (void) handleMessageData:(NSData*) aData
{
    //grab last fragment, use if not complete
    WebSocketFragment* fragment = [pendingFragments lastObject];
    if (!fragment || fragment.isValid)
    {
        //assign web socket fragment since the last one was complete
        fragment = [WebSocketFragment fragmentWithData:aData];
        [pendingFragments enqueue:fragment];
    }
    else if (fragment)
    {
        [fragment.fragment appendData:aData];
        if (fragment.isValid) 
        {
            [fragment parseContent];
        }
    }

    //if we have a complete fragment, handle it
    if (fragment.isValid) 
    {
        //handle complete fragment
        [self handleCompleteFragment:fragment];

        //if we have extra data, handle it
        if ([aData length] > fragment.messageLength)
        {
            [self handleMessageData:[aData subdataWithRange:NSMakeRange(fragment.messageLength, [aData length] - fragment.messageLength)]];
        }
    }
}

I hope you can reproduce this; I think it's a serious problem.

Cheers,
André

Original issue reported on code.google.com by andre.moencher@gmail.com on 26 Sep 2011 at 3:34

[GoogleCodeExporter]

I see the problem in the code. Thank you for providing such great detail. I am 
working on a fix. However, I cannot reproduce the errors due to the way the 
server I am using sends the fragments. What server are you using? Or would you 
help me test the fix so I know it addresses your issue?

Original comment by joshuadmorris@gmail.com on 27 Sep 2011 at 2:09

• Changed state: Accepted

[GoogleCodeExporter]

I'm using my own server that implements the latest drafts. I'll test the fix as 
soon as it will be available and be happy to provide feedback.

Thanks
André

Original comment by andre.moencher@gmail.com on 27 Sep 2011 at 5:27

[GoogleCodeExporter]

I've attache some logs from my debug output … maybe this helps

============================================================

Socket is open for business.

Read 222 bytes from socket
handleMessageData for data with length: 222
(!fragment || fragment.isValid) --> true
payloadStart: 4
payloadLength: 218
dataLength: 218
(4 + 218 == 222)?  --> true
(fragment.isValid) --> true
dequeue current fragment
handleCompleteFragment -- dispatch text message

--> first (successful) read

Read 1448 bytes from socket
handleMessageData for data with length: 1448
(!fragment || fragment.isValid) --> true
payloadStart: 4
payloadLength: 1489
dataLength: 1489
(4 + 1489 == 1448)? --> false, doing nothing

--> second read ... not yet enough data (see payload length)

Read 1465 bytes from socket
handleMessageData for data with length: 1465
(4 + 1489 == 1448)? --> false
else if (fragment) -- true
appending data to fragment.fragment
(4 + 1489 == 2913)? --> false
(4 + 1489 == 2913)? -- false, doing nothing

--> nothing happens after third read

============================================================

Original comment by andre.moencher@gmail.com on 27 Sep 2011 at 12:09

[GoogleCodeExporter]

Awesome! Great info! I am currently working on a fix, but it might be a few 
days as work is CRAZY busy right now. :)

Original comment by joshuadmorris@gmail.com on 27 Sep 2011 at 9:11

[GoogleCodeExporter]

Almost done with the fix. Really nice catch. This problem exists in both the 
iOS and Java clients.

Original comment by joshuadmorris@gmail.com on 29 Sep 2011 at 2:09

[GoogleCodeExporter]

Great to here … looking forward to the patch; thanks for your work so far.

Original comment by andre.moencher@gmail.com on 30 Sep 2011 at 6:13

[GoogleCodeExporter]

I just updated the source in SVN with the fix. I don't want to do a full build 
without your verification that it is fixed. Are you able to verify using the 
source, or do you need the library itself?

Original comment by joshuadmorris@gmail.com on 30 Sep 2011 at 9:51

[GoogleCodeExporter]

Original comment by joshuadmorris@gmail.com on 1 Oct 2011 at 6:22

• Changed state: Fixed

[GoogleCodeExporter]

Great! I'll check the fix and get back to you as soon as possible. 

Original comment by andre.moencher@gmail.com on 1 Oct 2011 at 7:13

[GoogleCodeExporter]

Hi, I've tested the fix; unfortunately it does not fix the problem.

Socket is open for business.
handle 222 bytes --> first read
payload length: 0
messageLength: 222
messageLength: 222
messageLength: 222
isVaild? 1
handleCompleteFragment
dispatchTextMessageReceived --> successful read …

messageLength: 222
handle 1448 bytes --> second read
payload length: 0
messageLength: 1493
messageLength: 1493
isVaild? 0
handle 1465 bytes --> third read
messageLength: 1493
isVaild? 0
messageLength: 1493
canBeParsed? 1
messageLength: 1493
messageLength: 1493
messageLength: 1493
isVaild? 1
handleCompleteFragment
dispatchTextMessageReceived -- successfully parsing data from first read … 
than nothing happens 

The problem seems to be, that the rest of the data (received on third read in 
this example) is not parsed. 

Original comment by andre.moencher@gmail.com on 4 Oct 2011 at 7:44

[GoogleCodeExporter]

>> dispatchTextMessageReceived -- successfully parsing data from first read … 
than nothing happens 

this has to be "successfully parsing data from second read" …. 

Original comment by andre.moencher@gmail.com on 4 Oct 2011 at 5:04

[GoogleCodeExporter]

Hi Josh, 
do I have to re-open a new ticket? 

Thanks,
André

Original comment by andre.moencher@gmail.com on 5 Oct 2011 at 4:16

[GoogleCodeExporter]

I reopened it. Can you tell me if these messages are fragmented or not? It 
changes the flow. I am trying to chase this down and that would really help me 
figure out what is going on.

Original comment by joshuadmorris@gmail.com on 5 Oct 2011 at 4:33

• Changed state: Started

[GoogleCodeExporter]

Seems like only the message from the second read gets fragmented; after parsing 
the first package of data nothing happens and the rest of the data remains in 
the buffer …

Original comment by andre.moencher@gmail.com on 6 Oct 2011 at 5:37

[GoogleCodeExporter]

If you need more precise output statements just tell me what should be logged 
… 

Original comment by andre.moencher@gmail.com on 6 Oct 2011 at 5:37

[GoogleCodeExporter]

Can you log the value for the opcode and isFinal property?

Original comment by joshuadmorris@gmail.com on 9 Oct 2011 at 3:17

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Hi Josh.

I think we've fixed the problem. I'll test it a bit more and then send you the 
diff. 

Cheers
André

Original comment by andre.moencher@gmail.com on 11 Oct 2011 at 12:10

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I've attached a patch that should be applied from the root directory of the 
UnittWebsocketClient iOS App;
This patch fixes an protocol level based error with parsing data in multiple 
reads from the socket.

I've also enclose a verbose diff that I have used for debugging.

Original comment by andre.moencher@gmail.com on 12 Oct 2011 at 9:18

Attachments:

• issue19_verbose_patch.diff
• issue19.patch

[GoogleCodeExporter]

AWESOME!!! I will check this out Monday and get everything straightened out. 
Thank you!

Original comment by joshuadmorris@gmail.com on 16 Oct 2011 at 8:36

[GoogleCodeExporter]

Thank you so much for all your hard work in identifying this issue and a 
solution. I will get the fix integrated and a build out this week/weekend.

Original comment by joshuadmorris@gmail.com on 18 Oct 2011 at 5:21

[GoogleCodeExporter]

Can we review this diff. I did a comparison and it is functionally identical. 
There is literally no change. I must be missing something. Do you have time for 
a Skype call or something?

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

[GoogleCodeExporter]

Sorry for the delay. I've been on holidays. 
I think the problem is, that you are kind of  loosing data in ''- (void) 
parseContent''  in some special cases. Those cases are when less than one full 
package ore more than one package comes with each read. 

Since I've added the patch I've never experienced the problem again. 

Today I've checked with your Rev631 which still seems to have this problem. 

Please have a detailed look at the '''- (void) parseContent'' function as well 
as the the modified init function '' - (NSData *) initWithData:(NSData*) aData 
'''. 

Original comment by andre.moencher@gmail.com on 30 Nov 2011 at 6:37

[GoogleCodeExporter]

I guess third time is the charm. I feel like an idiot. Thank you. I found the 
problem, or at least a problem that causes exactly what you are describing. I 
have checked in some changes, can you try it out and see if it fixes your issue?

Original comment by joshuadmorris@gmail.com on 1 Dec 2011 at 10:08

• Changed state: Fixed

[GoogleCodeExporter]

Great news, thank you! I'll have a look at the current version asap.

Original comment by andre.moencher@gmail.com on 2 Dec 2011 at 2:01

[GoogleCodeExporter]

Any luck?

Original comment by joshuadmorris@gmail.com on 14 Dec 2011 at 6:24

[GoogleCodeExporter]

I am marking this complete. Please reopen a new ticket if the problem persists.

Original comment by joshuadmorris@gmail.com on 18 Jan 2012 at 6:59

• Changed state: Verified