Skip to content

Fall back to reliable for channelId equal to channel count - #1087

Open
jimdroberts wants to merge 1 commit into
FirstGearGames:mainfrom
jimdroberts:fix/fallback-channel-bounds
Open

jimdroberts wants to merge 1 commit into
FirstGearGames:mainfrom
jimdroberts:fix/fallback-channel-bounds

Conversation

@jimdroberts

Copy link
Copy Markdown
Contributor

Problem

Sending on an unsupported channel is meant to fall back to Channel.Reliable. That works for channel ids 3 and
above, but channel id 2 (one past the last valid channel) is passed through unchanged and throws
IndexOutOfRangeException in TransportManager.SendToServer / SendToClient. It is reachable from any public
send that takes a Channel, for example ServerManager.Broadcast(..., (Channel)2).

Cause

Runtime/Managing/Transporting/TransportManager.cs:583:

private byte GetFallbackChannelIdAsNeeded(byte channelId) => channelId > _toServerBundles.Count ? (byte)Channel.Reliable : channelId;

_toServerBundles.Count is CHANNEL_COUNT (2), so valid ids are 0 and 1, but 2 > 2 is false. The returned id
then indexes arrays sized CHANNEL_COUNT. The first is _lowestMtus[channel] in GetLowestMTU(channelId)
(line 425), reached from SendSplitMessage. Next is _toServerBundles[channelId] (line 577).
NetworkConnection.SendToClient has its own >= clamp, but the throw happens before it is reached.

This is a regression from the 4.7 send-path rewrite. 4.6.x checked channelId >= _toServerBundles.Count.

Fix

Compare with >= against CHANNEL_COUNT, the size every array indexed afterwards uses (_lowestMtus,
_toServerBundles, the connection's _toClientBundles). This also keeps the server-side SendToClient path from
depending on the to-server bundle list.

Evidence

Verified by mapping of the old and new expressions (channel count 2):

channelId=0: main -> 0 (ok), fixed -> 0 (ok)
channelId=1: main -> 1 (ok), fixed -> 1 (ok)
channelId=2: main -> 2 (IndexOutOfRange), fixed -> 0 (ok)
channelId=3: main -> 0 (ok), fixed -> 0 (ok)

Behavior change

None for valid channels. Channel id 2 now falls back to reliable like every other unsupported id, instead of
throwing.

GetFallbackChannelIdAsNeeded used > instead of >=, so a channelId equal
to the channel count was passed through and indexed out of range.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant