ChainSafe / js-libp2p-yamux

Typescript implementation of Yamux
Other
12 stars 9 forks source link

Splitted Data When Transmission Data Length Exceeding Send Window Limitation #60

Closed frystal closed 7 months ago

frystal commented 11 months ago

Version:

js-libp2p-yamux v5.0.0

Platform:

Ubuntu20.04

Subsystem:

yamux

Severity:

Low

Description:

In the sendData function of js-libp2p-yamux, there's an issue where if the data to be sent exceeds the send window limit, only the send window amount is transmitted, leaving the rest unsent.

However, in the js-libp2p, it does not verify if the entire data has been received. If the full length of the data is not sent, it will report an error.

For instance, if we set the send window to 100 and disable processing receive window updates, the remote peer will receive fragmented identity data and handle it incorrectly. image

In my opinion, it should send the entire data as a single package rather than splitting it into two and waiting for the remaining data to send.

Steps to reproduce the error: None

frystal commented 11 months ago

I think I may have identified the reason for the issue.

I suspect it is caused by the behavior of the stream-muxer in the js-libp2p. It continues to send data to the function 'sendData' without waiting for the previous 'sendData' operation to finish. This leads to the situation depicted in the diagram below.

When the first protocol negotiation is completed, if the protocol attempts to send a message (specifically, the Identify Message) larger than the current send window limitation, it will first send the initial 'n' bytes allowed by the send window. Subsequently, the rest message (EOF) to send via 'sendData' is continued and blocked. This behavior is due to the non-blocking nature of the stream-muxer.

When the Responder responds with the windows update message for the protocol negotiation, all the previously blocked 'sendData' operations start to execute. However, the 'EOF' is sent before the remaining part of the 'Identify Message,' resulting in message disorder for the protocol. image image

achingbrain commented 7 months ago

The conversation around this is happening at https://github.com/libp2p/js-libp2p/issues/2359