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

Fix websocket dropping messages for WSLPeer #98167

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Spartan322
Copy link
Contributor

@Spartan322 Spartan322 commented Oct 14, 2024

Supercedes #98128
Fixes #70738

Address WSLPeer dropping messages

Currently the behavior of wsl_peer allows messages that would be able to fit into the in_buffer to be dropped completely while being received.

To address this issue the method WSLPeer::_wsl_on_frame_start_callback has been implemented as a callback from wslay. It is used to grab the size of the incoming message and use that size within WSLPeer::_wsl_recv_callback to refuse incoming packets for that message until space is available to hold the entire message.

[Note] Within the current implementation the words 'Payload' and 'Message' are used interchangeably. Godot refers to them as messages and wslay refers to them as payloads.

on_frame_recv_start_callback

This callback function is called with an argument that includes the incoming payload length. Using this length we can compare it to the free space currently in the in_buffer. This gives us the ability to wait on receiving the message untill enough space is available in the in_buffer to receive it.

void WSLPeer::_wsl_on_frame_start_callback(wslay_event_context_ptr ctx, const wslay_event_on_frame_recv_start_arg *arg, void *user_data) {
	WSLPeer *peer = (WSLPeer *)user_data;
	Ref<StreamPeer> conn = peer->connection;
	if (arg->opcode == WSLAY_TEXT_FRAME || arg->opcode == WSLAY_BINARY_FRAME) {
		uint64_t space_left = peer->in_buffer.space_left();
		if (space_left < arg->payload_length) {
			peer->length_needed = arg->payload_length;
		}
	}
}

uint64_t length_needed = 0;

This public member variable was introduced to store the size of the incoming message and is used when telling wslay to start accepting packets for the message.

WSLPeer::_wsl_recv_callback

Using the length_needed variable defined by the on_frame_recv_start_callback we are able to check if the current space available is sufficient to accept the incoming message. If it's not we continue waiting until enough space has opened up.

uint64_t space_left = peer->in_buffer.space_left();
if (space_left < peer->length_needed) {
	return WSLAY_ERR_NOMEM;
}

space_left function

This function was added to the PacketBuffer class to allow callback functions in wsl_peer.cpp to get the current freespace in the in_buffer.

int space_left() const {
	return _payload.space_left();
}

@RadenTheFolf


This PR is generously donated by Redot.

Allows WSLPeer to grab the length of incoming messages
	Only accept them if they fit in in_buffer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Websocket dropping data violates TCP reliability and congestion control
3 participants