[ENet-discuss] Bug in fragmented reliable packet reassembly

[Lee Salzman]

This indeed seems to be a valid bug, and I added your fix to my copy of
the source. However, since CVS is *cough* down *cough* I can't update
the CVS tree or the tarball.

I would advise ANYONE using ENet to patch their sources in the manner
described, which should be in peer.c around line 347.

Lee

On Mon, Oct 04, 2004 at 11:43:15PM -0400, Kevin Wasserman wrote:
> 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
> _______________________________________________
> ENet-discuss mailing list
> ENet-discuss at cubik.org
> http://lists.cubik.org/mailman/listinfo/enet-discuss