Packet Corruption on 64bit about raknet HOT 25 OPEN

Yeah my fix did fix it for me. Here's what I changed in BuildPacketFromSplitPacketList:

    for (j=0; j < splitPacketChannel->splitPacketList.Size(); j++)
    {
        splitPacket=splitPacketChannel->splitPacketList[j];
        memcpy(internalPacket->data + BITS_TO_BYTES(offset), splitPacket->data, (size_t)BITS_TO_BYTES(splitPacketChannel->splitPacketList[j]->dataBitLength));
        offset += splitPacketChannel->splitPacketList[j]->dataBitLength;
    }

to

    BitSize_t offset = 0;
    for (j=0; j < splitPacketChannel->splitPacketList.Size(); j++)
    {
//
// GARRY - RETRIEVE SPLIT PACKETS IN THE PROPER ORDER
//
        splitPacket = NULL;

        for ( int a = 0; a < splitPacketChannel->splitPacketList.Size(); a++ )
        {
            if ( splitPacketChannel->splitPacketList[a]->splitPacketIndex != j )
                continue;

            splitPacket = splitPacketChannel->splitPacketList[a];
            break;
        }

        if ( splitPacket == NULL ) 
        splitPacket = splitPacketChannel->splitPacketList[j];

        memcpy( internalPacket->data + BITS_TO_BYTES( offset ), splitPacket->data, (size_t)BITS_TO_BYTES( splitPacket->dataBitLength ) );
        offset += splitPacket->dataBitLength;
//
// END GARRY
//
    }

from raknet.

I did some logging and found that the issue is only apparent when the packets are split, and when the first fragment isn't received first.

It seems to be that split packets aren't being reconstructed in the correct order, but in the order in which the packets are received. I've changed the BuildPacketFromSplitPacketList function to construct the packets in order splitPacketIndex - will report whether this fixes it.

from raknet.

Hi @garrynewman,

If you could contribute a patch/pull request that would be great - I'm using RakNet in an application that will often have packets > MTU so I'm very interested in any fixes here.

Regards.

from raknet.

Hey @garrynewman

Did you have any success with your fixes?

Note, the latest source already appears to use splitPacketIndex when reconstructing the packets:

memcpy(internalPacket->data+splitPacket->splitPacketIndex*splitPacketPartLength, splitPacket->data, (size_t) BITS_TO_BYTES(splitPacketChannel->splitPacketList[j]->dataBitLength));

Regards.

from raknet.

Having a serious issue like this and no replies from the developers doesn't exactly make RakNet look very good.

from raknet.

it, is the code of previous version, it think all split packet have the same size, it store the size into splitPacketPartLength=BITS_TO_BYTES(splitPacketChannel->firstPacket->dataBitLength);

and fill the memory with
memcpy(internalPacket->data+splitPacket->splitPacketIndex*splitPacketPartLength, splitPacket->data, (size_t) BITS_TO_BYTES(splitPacketChannel->splitPacketList[j]->dataBitLength));

and use splitPacketIndex to put the split pack in this position.

but i am not sure this code it is ok and all split packet have the same size.

----------- CODE -----------------------------
'''
InternalPacket * ReliabilityLayer::BuildPacketFromSplitPacketList( SplitPacketChannel *splitPacketChannel, CCTimeType time )
{
#if PREALLOCATE_LARGE_MESSAGES==1
InternalPacket *returnedPacket=splitPacketChannel->returnedPacket;
RakNet::OP_DELETE(splitPacketChannel, FILE, LINE);
(void) time;
return returnedPacket;
#else
unsigned int j;
InternalPacket * internalPacket, *splitPacket;
int splitPacketPartLength;
internalPacket = CreateInternalPacketCopy( splitPacketChannel->splitPacketList[0], 0, 0, time );
internalPacket->dataBitLength=0;
for (j=0; j < splitPacketChannel->splitPacketList.Size(); j++)
internalPacket->dataBitLength+=splitPacketChannel->splitPacketList[j]->dataBitLength;
splitPacketPartLength=BITS_TO_BYTES(splitPacketChannel->firstPacket->dataBitLength);

internalPacket->data = (unsigned char*) rakMalloc_Ex( (size_t) BITS_TO_BYTES( internalPacket->dataBitLength ), _FILE_AND_LINE_ );
internalPacket->allocationScheme=InternalPacket::NORMAL;

for (j=0; j < splitPacketChannel->splitPacketList.Size(); j++)
{
    splitPacket=splitPacketChannel->splitPacketList[j];
    memcpy(internalPacket->data+splitPacket->splitPacketIndex*splitPacketPartLength, splitPacket->data, (size_t) BITS_TO_BYTES(splitPacketChannel->splitPacketList[j]->dataBitLength));
}

for (j=0; j < splitPacketChannel->splitPacketList.Size(); j++)
{
    FreeInternalPacketData(splitPacketChannel->splitPacketList[j], _FILE_AND_LINE_ );
    ReleaseToInternalPacketPool(splitPacketChannel->splitPacketList[j]);
}
RakNet::OP_DELETE(splitPacketChannel, __FILE__, __LINE__);

return internalPacket;

#endif
}
'''

from raknet.

I might want to wrape that code with ```

from raknet.

sorry i only speak a little english

from raknet.

Put ``` at the start and end of your code.

from raknet.

That's almost right. You have to include new lines before and after it. So like

'''
Code
Code
'''

from raknet.

sorry i try it,y put ''' but it is not ok

from raknet.

It's ```, I just put ''' as an example so it didn't show as code. You still have to put a new line before your last one.

from raknet.

in the code of GARRY i dont understan how this part of the code it is possible

if ( splitPacket == NULL )
splitPacket = splitPacketChannel->splitPacketList[j];

how ist si posible not found splitPacket by splitPacketIndex

from raknet.

Packet editing? Packet corruption? Dunno.

from raknet.

i think if it not found splitPacket , it is a error and this mesage must be discard.
if use splitPacket = splitPacketChannel->splitPacketList[j]; it must be copying data in a Incorrect position

from raknet.

Can this be fixed and officially be committed on the repo? Ever since Oculus bought it there are no big updates...

from raknet.

RakNet has been abandoned, as is shown on the website.

from raknet.

Maybe Oculus, the new owner of RakNet, is too busy with VR to care about this project for now...

from raknet.

Fine, Oculus shall then assign someone repo admin rights so he can process pull requests.

from raknet.

I'm not really sure why they bought something to leave it die. It certainly at least needs someone to process pull requests, so we can include fixes like Garry's.

from raknet.

Maybe someone with the know-how will start a community fork

Thanks for the fix @garrynewman! I was beating my head against this bug for hours :(
As noted by larku, it seems e97c4bb introduced this bug.

from raknet.

I've got a fork ( https://github.com/larku/RakNet ) that includes most of the pending pull requests from the official if that's of use to you. I'm also happy to accept pull requests into this fork.

from raknet.

Seems the problem described in this ticket is still present in your fork.

from raknet.

Hey @Cleroth, that fork includes:

larku@c15dd90

Which should address this.

from raknet.

Oh... so it was fixed long ago in a different way...
Thanks. I might actually give RakNet a try now.

from raknet.