fix(yamux): handle WINDOW_UPDATE|SYN without reading payload#1322
Open
arpittkhandelwal wants to merge 1 commit into
Open
fix(yamux): handle WINDOW_UPDATE|SYN without reading payload#1322arpittkhandelwal wants to merge 1 commit into
arpittkhandelwal wants to merge 1 commit into
Conversation
Fixes libp2p#1312. Separated SYN handling for DATA and WINDOW_UPDATE frames. Added regression tests for WINDOW_UPDATE|SYN, DATA|SYN|FIN, and DATA|SYN|RST.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1312
Summary
This PR fixes a bug in the Yamux stream muxer where
WINDOW_UPDATE|SYNframes were incorrectly treated as carrying payload bytes.According to the Yamux specification, the
SYNflag can be used with bothDATAandWINDOW_UPDATEframes to open a new stream. However, forWINDOW_UPDATEframes, thelengthfield represents the window increment, not a payload size.Previous behavior:
The muxer unconditionally attempted to read
lengthbytes from the connection whenever theSYNflag was present. When receiving aWINDOW_UPDATE|SYNframe (e.g., from a Lua-based libp2p dialer), the muxer would hang while waiting for a non-existent payload.New behavior:
SYNhandling logic based on the frame type.TYPE_DATA | SYN: Continues to read the optional inline payload iflength > 0.TYPE_WINDOW_UPDATE | SYN: Applies thelengthas a window increment and skips the payload reading step.FINandRSTflags onSYNframes (opening and immediately half-closing or resetting a stream).Fixes #1312
Tests
I have added regression tests to
tests/core/stream_muxer/test_yamux.py:test_yamux_syn_with_window_update: Verifies thatWINDOW_UPDATE|SYNframes are handled correctly without hanging and that the window increment is applied.test_yamux_syn_with_fin: Verifies thatDATA|SYN|FINcorrectly opens and half-closes a stream.test_yamux_syn_with_rst: Verifies thatDATA|SYN|RSTcorrectly opens and resets a stream.All 25 Yamux tests are passing.