Repository navigation
fix(const)!: give tcp.flags.Flags their wire masks (#2310) - #2340
Merged
Merged
Conversation
JarryShaw
added
breaking
Owner
Author
|
Review: NEEDS CHANGES at
|
JarryShaw
added
review: needs-changes
Problem: the crawler generated each member as 1 << offset in IANA's most-significant-first numbering, so FIN was 0x8000 and int() of a parsed SYN/ACK's connection was 0x4800 where the header carries 0x012. Fix: generate 1 << (15 - offset), each flag's mask in the 12-bit flags field, and narrow the range guard to 0 <= value <= 0xFFF. The const module is regenerated from the crawler. Tests and the dumpkit docstring that pinned the old integers, or used Flags for undeclared bits it no longer has, move to the new values or to BindingUpdateFlag. Test: test_const_tcp_flags_wire_masks_2310_unit checks every member's mask, composites, the guard, a parsed SYN/ACK, every flags octet reading back as itself with a byte-exact rebuild, and the crawler's output. Changelog entry under pcapkit.const, CHANGELOG.md regenerated.
JarryShaw
force-pushed
the
fix/2310-tcp-flags-wire-masks
branch
from
October 26, 2026 23:10
b0d4897 to
633ab11
Compare
Owner
Author
|
Review: GOOD TO GO at |
JarryShaw
added
review: good-to-go
Contributor
|
Coverage: 91.56% (unit tier, Python 3.15,
Per-file detail: the |
JarryShaw
removed
the
review: good-to-go
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 change1.5.0.rst(breaking change)What is the purpose of your pull request?
fix — bug fixDescription
Closes #2310, per the ruling (take the recommendation). Breaking: every
tcp.flags.Flagsvalue changes.1 << offset, but IANA counts offsets from the high end of the 16-bit word. So FIN was0x8000, and a parsed SYN/ACK gaveint(connection) == 0x4800where the header carries0x12.1 << (15 - offset), so FIN is0x001and AE is0x100. The range checks narrow to the 12-bit field (0xFFF), andpcapkit/const/tcp/flags.pyis regenerated from the crawler. No library code depended on the old integers. The tests that pinned them, and therender_enumdocstring examples, are updated.test_const_tcp_flags_wire_masks_2310_unit.pychecks every wire mask, rejects0x1000, and covers a SYN/ACK whoseint(connection)equals the header bits. It also checks that all 256 flag-octet values rebuild byte-exact and thatmake()agrees with what it writes.origin/main, 302 failed, e.g.18432 != 18. On the head, 139 passed.connectionis built from the 8-bit flags byte, so AE and the reserved bits never appear in it. That predates this change.