[ENet-discuss] Bug in fragmented reliable packet reassembly

[Kevin Wasserman]

Hi everyone,

I've been using enet for a few weeks
now and found it to be generally quite
stable and exactly what I hoped it would
be.  However, I believe I have found
quite a serious bug in the receive path
for fragmented reliable packets.

In the distribution I downloaded (in early
September), when fragments are received
out of order, the reconstructed packet is
given the reliable sequence number of the
first fragment received instead of the start
sequence number, with the result that no
further reliable packets are ever processed
for that channel, since the channel believes
it is still waiting for a packet with the start
sequence number.

I fixed the bug in my local codebase by
adding a single line to the block that
initially creates the recomposed packet
in enet_handle_send_fragment():

old code:
    if (currentCommand == enet_list_end (& channel -> 
incomingReliableCommands))
    {
       ENetProtocol hostCommand = * command;
       hostCommand.sendFragment.startSequenceNumber = startSequenceNumber;
       hostCommand.sendFragment.fragmentNumber = fragmentNumber;
       hostCommand.sendFragment.fragmentCount = fragmentCount;
       hostCommand.sendFragment.fragmentOffset = fragmentOffset;
       hostCommand.sendFragment.totalLength = totalLength;

       startCommand = enet_peer_queue_incoming_command (peer,
                                                        & hostCommand,
                                                        enet_packet_create 
(NULL, totalLength, ENET_PACKET_FLAG_RELIABLE),
                                                        fragmentCount);
    }

new code:
    if (currentCommand == enet_list_end (& channel -> 
incomingReliableCommands))
    {
       ENetProtocol hostCommand = * command;
       hostCommand.header.reliableSequenceNumber = startSequenceNumber;
       hostCommand.sendFragment.startSequenceNumber = startSequenceNumber;
       hostCommand.sendFragment.fragmentNumber = fragmentNumber;
       hostCommand.sendFragment.fragmentCount = fragmentCount;
       hostCommand.sendFragment.fragmentOffset = fragmentOffset;
       hostCommand.sendFragment.totalLength = totalLength;

       startCommand = enet_peer_queue_incoming_command (peer,
                                                        & hostCommand,
                                                        enet_packet_create 
(NULL, totalLength, ENET_PACKET_FLAG_RELIABLE),
                                                        fragmentCount);
    }

I would encourage others actively using enet
for reliable delivery of packets large enough
to be fragmented to incorporate a similar
fix.  It's a rare, but quite fatal and difficult
to diagnose bug.

-Kevin Wasserman