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

Server/Network: Possible out-of-order packet sending #28625

Open
Lordron opened this issue Dec 27, 2022 · 0 comments
Open

Server/Network: Possible out-of-order packet sending #28625

Lordron opened this issue Dec 27, 2022 · 0 comments

Comments

@Lordron
Copy link
Contributor

Lordron commented Dec 27, 2022

Description

Looking at WorldSocket::Update i notice 2 suspicious things happening inside while loop;

  1. At the beginning of while loop we have this piece of code. What will happend if the very first packet in the queue (event after compression) is still to big for a buffer? Do i understand correctly that we will enqueue an empty buffer?
        // Flush current buffer if too small for next packet
        if (buffer.GetRemainingSpace() < packetSize + sizeof(PacketHeader))
        {
            QueuePacket(std::move(buffer));
            buffer.Resize(_sendBufferSize);
        }
  1. The second thing where possible out-of-order sending can happend is below in the else statement

        else    // single packet larger than _sendBufferSize
        {
            MessageBuffer packetBuffer(packetSize + sizeof(PacketHeader));
            WritePacketToBuffer(*queued, packetBuffer);
            QueuePacket(std::move(packetBuffer));
        }

Let's imagine we have the sequence of packets of different sizes queued. Size of the imagine packet on the right.
A - 5, B - 35, C - 17, D - 50k, E - 150

The first 3 packets (A, B, C) will be pushed to buffer because their sizes is smaller than _sendBufferSize. Then we dequeue packet 'D' whose size is larger, we will create a separate MessageBuffer, write it to it and queue it. After we take packet 'E' and also write it to buffer. After the loop we queue the buffer. And after that our queue will look like this
D - A - B - C - E.
D is the first because we queued it before the buffer.

Idk if this is a real-case scenario considering compression. possible solution would be adding

if (buffer.GetActiveSize() > 0)
{
            QueuePacket(std::move(buffer));
            buffer.Resize(_sendBufferSize);
}

Expected behaviour

None

Steps to reproduce the problem

Check the code

Branch

3.3.5

TC rev. hash/commit

commit dc22160

Operating system

Windows 10 x64

Custom changes

None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant