Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Large unreliable message cannot be sent properly #148

Open
aienabled opened this issue Oct 27, 2020 · 2 comments
Open

Large unreliable message cannot be sent properly #148

aienabled opened this issue Oct 27, 2020 · 2 comments

Comments

@aienabled
Copy link
Contributor

aienabled commented Oct 27, 2020

Hello!

While the project is no longer actively maintained it's important to mention this issue.

If you attempt to send a large unreliable message, by default Lidgren library attempts to send it as a single unfragmented message even if it's above MTU.
However, the data format for the network message header allows a payload only of up to 65535 bits length (16 bits limit; that's less than 8192 bytes) as you can see here and in the code that write the size into the buffer

// 16 bits - Payload length in bits

There are no checks to ensure that the max packet size is not exceeded. The int m_bitLength number is simply encoded into 16 bits.

This way any unreliable messages of larger size will be received with an incorrect size—even though the whole message is written and might be received on the client side, it's not properly parsed as the payload size cannot be properly read. And Lidgren doesn't actually check whether the received payload matches the size, the only check I see is this one:

if (bytesReceived - ptr < payloadByteLength)

E. g. if you're sending a payload of 29914 bytes the library sees only 5332 bytes due to the (unchecked) 16-bit encoding of the payload length.

There is no proper support for fragmenting unreliable messages. While you can enable fragmentation for that case there is a critical drawback:

/// Old behaviour; use normal fragmentation for unreliable messages - if a fragment is dropped, memory for received fragments are never reclaimed!

Bottom line: don't send large unreliable packages. Make sure you split your data—or use reliable mechanisms (as you should whenever you need to actually send something large).
Also, I think at least a max packet size check is required in the library otherwise it's really easy to miss this issue and spend hours investigating it (that's what I did).

Regards!

@aienabled
Copy link
Contributor Author

aienabled commented Oct 28, 2020

A simple check that seems to work very well and also notifies the library user about the problem:

if (message.LengthBits >= ushort.MaxValue
    && m_connection.m_peerConfiguration.UnreliableSizeBehaviour == NetUnreliableSizeBehaviour.IgnoreMTU)
{
    // drop message
    this.m_connection.m_peer.LogError(
        string.Format("Unreliable message max size exceeded: {0} bits (max {1})",
                      message.LengthBits, 
                      ushort.MaxValue));
    return NetSendResult.Dropped;
}

it should be inserted in NetUnreliableSenderChannel right before this line:


This way only the unreliable messages exceeding the max allowed message size are prohibited (either way they cannot be received properly so it's best to drop them and notify the library user).
Please let me know if a pull request would be useful.

@PJB3005
Copy link
Contributor

PJB3005 commented Oct 28, 2020

Oh so that explains why my game state system got overloaded even over localhost.

As for pull requests: the library is unmaintained now, see #147 for maintained forks.

PJB3005 added a commit to space-wizards/SpaceWizards.Lidgren.Network that referenced this issue Oct 30, 2020
Not a proper solution to the unreliable fragmentation issue, but better than nothing.

See lidgren#148
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

No branches or pull requests

2 participants