chandramanic / unitt

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

Fragmented packets aren't correctly assembled #40

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Currently fragmented packets may see doubling by having their data appended 
twice due to in WebSocket.m's handleMessageData method:

<pre>
        fragment = [WebSocketFragment fragmentWithData:aData];
...
        [fragment.fragment appendData:aData];
...
</pre>

They also don't correctly reassemble, at least for me using Tornado 2.1.1 HEAD. 
The issue being the logic surrounding creating a fragment or pulling it from 
the pendingFragments queue. This rewritten method performs correctly for me:

<pre>
- (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 {
        //append the data
        [fragment.fragment appendData:aData];
        //if we have a fragment, let's see if we can parse it and continue
        NSString *dataAsStr = [[NSString alloc] initWithData:fragment.fragment encoding:NSUTF8StringEncoding];
        inspect(@"fragment as string: %@", dataAsStr);
    }
    NSAssert(fragment != nil, @"Websocket fragment should never be nil");

    //parse the data, if possible
    if (fragment.canBeParsed) 
    {
        [fragment parseContent];

        //if we have a complete fragment, handle it
        if (fragment.isValid)
        {
            [self handleCompleteFragment:fragment];
        }
    }
    //if we have extra data, handle it
    if (fragment.messageLength > 0 && ([aData length] > fragment.messageLength))
    {
        [self handleMessageData:[aData subdataWithRange:NSMakeRange(fragment.messageLength, [aData length] - fragment.messageLength)]];
    }
}
</pre>

Aaron

Original issue reported on code.google.com by aa...@azinman.com on 18 Jan 2012 at 7:14

GoogleCodeExporter commented 9 years ago
Hmm.. guess issues don't parse the HTML PRE block like other parts of Google 
Code. Do you know how I'm supposed to do this? Upload a file instead?

Original comment by aa...@azinman.com on 18 Jan 2012 at 7:14

GoogleCodeExporter commented 9 years ago
Uploading a file will work. :) 

Did you try this using the code in the trunk? Or the released library?

Original comment by joshuadmorris@gmail.com on 18 Jan 2012 at 9:26

GoogleCodeExporter commented 9 years ago
Noted for next time :)  I do wish that was built into the interface; I find it 
useful to have everything in one place.

I think it's HEAD, although I could be remembering that wrong. Either way the 
code in the trunk looks wrong, where you'll get the doubling effect. I noticed 
downstream there's some band-aid where it looks for data segments longer than 
the header indicated and just chops them off to match. Seems like a bad 
band-aid, which just ended up covering up the earlier error. I've removed it in 
my version.

Aaron

Original comment by aa...@azinman.com on 18 Jan 2012 at 9:34

GoogleCodeExporter commented 9 years ago
I will make some changes shortly. Can you test using your environment once it's 
fixed?

Original comment by joshuadmorris@gmail.com on 30 Jan 2012 at 4:26

GoogleCodeExporter commented 9 years ago
This is now fixed in trunk. Can you take a quick look and verify that it is now 
working for you? I will generate a new build if this is working for your 
particular server. Do you have a sample/test project I can verify this with? I 
can include it in my tests I run before I generate new builds.

Original comment by joshuadmorris@gmail.com on 30 Jan 2012 at 6:45

GoogleCodeExporter commented 9 years ago

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