Repository navigation
fix(tcp): correct MPTCP unknown-subtype length and drop an undeclared parse-only attribute - #2396
Merged
Merged
Conversation
… parse-only attribute (#2272, #2293) #2293: _MPTCP.post_process set ret.option = Enum_Option.Multipath_TCP on the subtype schema. MPTCP declares no option field, the declared kind field already holds Multipath_TCP, and no _make_mptcp_* maker mirrors it -- so a parsed schema carried a key a built one did not, though both packed identical octets. Drop the assignment; length and subtype stay, since those really are set here and nowhere else on the unpacking path. #2272: an unknown MPTCP subtype with data=b'' declared Length 2 but MPTCPUnknown always packs 3 header octets (kind, length, and the subtype octet whose low nibble takes data[0]) plus data[1:]. The two agree for any non-empty data and disagree only at data=b''. Size the length from what the schema actually writes: 3 + len(data[1:]). Tests: option-attribute parity across MP_CAPABLE, MP_FASTCLOSE and the MPTCPUnknown fallback; declared-vs-wire length across payload widths, the 3-octet minimum, and a real parse-back through the public TCP() constructor. test_tcp_udp_unit.py's post_process assertion is updated to require option's absence rather than its value.
JarryShaw
added
fix
Owner
Author
|
Review: GOOD TO GO at |
JarryShaw
added
review: good-to-go
Contributor
|
Coverage: 91.67% (unit tier, Python 3.15,
Per-file detail: the |
JarryShaw
removed
the
review: good-to-go
4 tasks done
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.
make testpasses, and a test case covers the changeWhat is the purpose of your pull request?
fix — bug fixDescription
Closes #2272. Closes #2293.
_MPTCP.post_processsetret.option = Enum_Option.Multipath_TCPon the subtype schema.MPTCPdeclares nooptionfield, the declaredkindfield already holdsMultipath_TCP, and no_make_mptcp_*maker mirrors the assignment — so a parsed schema carried a key a built one did not, though both packed identical octets. Fix: drop the assignment.lengthandsubtypestay; those really are set here and nowhere else on the unpacking path.data=b'': an unknown MPTCP subtype declaredLength 2, butMPTCPUnknownalways packs 3 header octets (kind,length, and the subtype octet whose low nibble takesdata[0]) plusdata[1:]. The two agree for any non-emptydataand disagree only atdata=b''. Fix: size the length from what the schema actually writes,3 + len(data[1:]).MP_CAPABLE,MP_FASTCLOSEand theMPTCPUnknownfallback; declared-vs-wire length across payload widths, the 3-octet minimum, and a real parse-back through the publicTCP()constructor.