diff --git a/CHANGELOG.md b/CHANGELOG.md index d76e441b02..f622898d49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` to `1.5.0b3` (all 2026-09-19) and - **a breaking change to** `ExtensionHeader`: it is generated from IANA's IPv6 Extension Header Types registry rather than the Protocol Numbers registry's flag, and the two disagree on 147 (`BIT-EMU`) ([#925](https://github.com/JarryShaw/PyPCAPKit/issues/925), [#926](https://github.com/JarryShaw/PyPCAPKit/pull/926)). - the TCP and UDP `AppType` registries load on first use rather than at import ([#1538](https://github.com/JarryShaw/PyPCAPKit/issues/1538), [#1560](https://github.com/JarryShaw/PyPCAPKit/pull/1560)). - **a breaking change to** `LLACode`: `Wilcard` is spelled `Wildcard` and `MH` is `MN`, the names [RFC 5568](https://datatracker.ietf.org/doc/html/rfc5568) gives codes 0 and 2, with no alias for either old name ([#1848](https://github.com/JarryShaw/PyPCAPKit/issues/1848), [#1872](https://github.com/JarryShaw/PyPCAPKit/pull/1872)). +- **a breaking change to** `tcp.flags.Flags`: each member is its wire mask in the flags field, `FIN` `0x001` up to `AE` `0x100`, rather than `1 << offset` in IANA's most-significant-first bit numbering, which put `FIN` at `0x8000`. `int(tcp.connection)` of a SYN/ACK is `0x012`, as the header carries it, not `0x4800`, and `Flags(0x12)` is `SYN | ACK`. The range guard narrows to the 12-bit flags field, `0 <= value <= 0xFFF`. Code that stored or compared the old integers has to change; membership tests and names are unaffected ([#2310](https://github.com/JarryShaw/PyPCAPKit/issues/2310)). #### Fixed diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index 569314c54c..7d8ca48725 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -90,6 +90,14 @@ Changed * **a breaking change to** :class:`~pcapkit.const.mh.lla_code.LLACode`: ``Wilcard`` is spelled ``Wildcard`` and ``MH`` is ``MN``, the names :rfc:`5568` gives codes 0 and 2, with no alias for either old name (:issue:`1848`, :pr:`1872`). +* **a breaking change to** :class:`tcp.flags.Flags `: + each member is its wire mask in the flags field, ``FIN`` ``0x001`` up to ``AE`` + ``0x100``, rather than ``1 << offset`` in IANA's most-significant-first bit numbering, + which put ``FIN`` at ``0x8000``. ``int(tcp.connection)`` of a SYN/ACK is ``0x012``, + as the header carries it, not ``0x4800``, and ``Flags(0x12)`` is ``SYN | ACK``. The + range guard narrows to the 12-bit flags field, ``0 <= value <= 0xFFF``. Code that + stored or compared the old integers has to change; membership tests and names are + unaffected (:issue:`2310`). Fixed ~~~~~ diff --git a/pcapkit/const/tcp/flags.py b/pcapkit/const/tcp/flags.py index e0a7f583d6..6ae6b9593a 100644 --- a/pcapkit/const/tcp/flags.py +++ b/pcapkit/const/tcp/flags.py @@ -26,40 +26,40 @@ class Flags(EnumRegistry, IntFlag): """[Flags] TCP Header Flags""" #: Reserved for future use [:rfc:`9293`] - Reserved_4 = 1 << 4 + Reserved_4 = 1 << 11 #: Reserved for future use [:rfc:`9293`] - Reserved_5 = 1 << 5 + Reserved_5 = 1 << 10 #: Reserved for future use [:rfc:`9293`] - Reserved_6 = 1 << 6 + Reserved_6 = 1 << 9 #: AE (Accurate ECN) [:rfc:`9768`] - AE = 1 << 7 + AE = 1 << 8 #: CWR (Congestion Window Reduced) [:rfc:`3168`] - CWR = 1 << 8 + CWR = 1 << 7 #: ECE (ECN-Echo) [:rfc:`3168`] - ECE = 1 << 9 + ECE = 1 << 6 #: Urgent Pointer field is significant (URG) [:rfc:`9293`] - URG = 1 << 10 + URG = 1 << 5 #: Acknowledgment field is significant (ACK) [:rfc:`9293`] - ACK = 1 << 11 + ACK = 1 << 4 #: Push Function (PSH) [:rfc:`9293`] - PSH = 1 << 12 + PSH = 1 << 3 #: Reset the connection (RST) [:rfc:`9293`] - RST = 1 << 13 + RST = 1 << 2 #: Synchronize sequence numbers (SYN) [:rfc:`9293`] - SYN = 1 << 14 + SYN = 1 << 1 #: No more data from sender (FIN) [:rfc:`9293`] - FIN = 1 << 15 + FIN = 1 << 0 @classmethod def _validate_value(cls, value: 'Any') -> 'None': @@ -80,7 +80,7 @@ def _validate_value(cls, value: 'Any') -> 'None': unaffected. """ - if not (isinstance(value, int) and 0 <= value <= 0xFFFF): + if not (isinstance(value, int) and 0 <= value <= 0xFFF): raise EnumValueError(f'{value!r} is not a valid {cls.__name__}') @classmethod @@ -91,6 +91,6 @@ def _missing_(cls, value: 'int') -> 'Flags': value: Value to get enum item. """ - if not (isinstance(value, int) and 0 <= value <= 0xFFFF): + if not (isinstance(value, int) and 0 <= value <= 0xFFF): raise ValueError(f'{value!r} is not a valid {cls.__name__}') return super()._missing_(value) diff --git a/pcapkit/dumpkit/common.py b/pcapkit/dumpkit/common.py index 93e44bb9f9..b27e4ccdcb 100644 --- a/pcapkit/dumpkit/common.py +++ b/pcapkit/dumpkit/common.py @@ -220,16 +220,18 @@ def render_enum(o: 'enum.Enum | aenum.Enum') -> 'str': plausible member name, so a consumer splitting the rendering on ``::`` cannot tell it from a member genuinely so named -- and ``NONE`` *is* a declared name elsewhere in the library. And the defect is not confined to - zero: ``Flags(1)``, ``Flags(8)`` and ``Flags(15)`` are every bit as - nameless, so a guard written against ``value == 0`` would fix one case - and leave the rest. + zero: a registry whose field has undeclared bits, such as + :class:`~pcapkit.const.mh.binding_update_flag.BindingUpdateFlag`, is + every bit as nameless at ``1`` or ``8``, so a guard written against + ``value == 0`` would fix one case and leave the rest. The fallback is the value's own decimal spelling, which is what the enumeration libraries themselves already use for an undeclared residue: - ``Flags(2057).name`` is ``'ACK|9'``, naming the declared bit and giving - the leftovers as one number. A wholly-undeclared value is that same - rendering with no declared bit to precede it, so ``Flags(9)`` becomes - ``'Flags::9 [9]'`` and ``Flags(0)`` becomes ``'Flags::0 [0]'``. It also + ``BindingUpdateFlag(32777).name`` is ``'A|9'``, naming the declared bit + and giving the leftovers as one number. A wholly-undeclared value is that + same rendering with no declared bit to precede it, so + ``BindingUpdateFlag(9)`` becomes ``'BindingUpdateFlag::9 [9]'`` and + ``Flags(0)`` becomes ``'Flags::0 [0]'``. It also cannot be mistaken for a member name, since a Python identifier may not begin with a digit. diff --git a/pcapkit/protocols/transport/tcp.py b/pcapkit/protocols/transport/tcp.py index bee20b146f..99b71098b2 100644 --- a/pcapkit/protocols/transport/tcp.py +++ b/pcapkit/protocols/transport/tcp.py @@ -528,9 +528,9 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_TCP': # never runs, and ``Enum_Flags.SYN in self._flags`` would raise ``TypeError: # argument of type 'int' is not a container or iterable``. A segment with any flag # set would mask that. :class:`Enum_Flags` is an ``aenum.IntFlag`` whose - # ``_missing_`` accepts any value from ``0`` to ``0xFFFF``, so ``Enum_Flags(0)`` is - # a valid flagless member that compares equal to ``0`` and ORs as usual. This is the type :attr:`connection` - # advertises. + # ``_missing_`` accepts any value in the 12-bit flags field, ``0`` to ``0xFFF``, + # so ``Enum_Flags(0)`` is a valid flagless member that compares equal to ``0`` and + # ORs as usual. This is the type :attr:`connection` advertises. _flag = Enum_Flags(0) for key, val in schema.flags.items(): if val == 1: diff --git a/pcapkit/vendor/tcp/flags.py b/pcapkit/vendor/tcp/flags.py index 2901081db6..1a4c9e92fc 100644 --- a/pcapkit/vendor/tcp/flags.py +++ b/pcapkit/vendor/tcp/flags.py @@ -104,11 +104,13 @@ class Flags(Vendor): """TCP Header Flags""" #: Value limit checker. The registry indexes *bit offsets* 4 through 15, - #: but the members it generates are ``1 << offset``, so the value domain a - #: lookup has to accept is the 16-bit field those bits live in -- a + #: counted from the most significant end of the 16-bit Data Offset/flags + #: word, and the members it generates are their wire masks, + #: ``1 << (15 - offset)`` (:issue:`2310`). So the value domain a lookup has + #: to accept is the low 12 bits of that word, the flags field -- a #: composite such as ``SYN | ACK`` is a legitimate value and only the #: composite path reaches :meth:`~pcapkit.const.tcp.flags.Flags._missing_`. - FLAG = 'isinstance(value, int) and 0 <= value <= 0xFFFF' + FLAG = 'isinstance(value, int) and 0 <= value <= 0xFFF' #: Link to registry. LINK = 'https://www.iana.org/assignments/tcp-parameters/tcp-header-flags.csv' @@ -146,7 +148,10 @@ def process(self, data: 'list[str]') -> 'list[str]': # type: ignore[override] name = DATA.get(int(code), dscp.split(' (')[0]).replace('Reserved for future use', 'Reserved') renm = self.rename(name or 'Unassigned', code, original=dscp) - pres = f"{renm} = 1 << {code}" + # NOTE: IANA numbers the bits from the most significant end of the + # 16-bit Data Offset/flags word, so the wire mask of offset ``n`` is + # ``1 << (15 - n)`` -- FIN, at offset 15, is the low bit (#2310). + pres = f"{renm} = 1 << {15 - int(code)}" sufs = f'#: {desc}' # if len(pres) > 74: diff --git a/tests/const/test_const_enum_builtin_parity.py b/tests/const/test_const_enum_builtin_parity.py index 406859182c..733cfa8965 100644 --- a/tests/const/test_const_enum_builtin_parity.py +++ b/tests/const/test_const_enum_builtin_parity.py @@ -543,7 +543,7 @@ class ConstFlagCompositeTests(unittest.TestCase): ``FLAG = 'isinstance(value, int) and 4 <= value <= 15'`` checker in :mod:`pcapkit.vendor.tcp.flags` would have done had the template ever interpolated it: those are *bit offsets*, and the values they generate are - ``1 << offset``. + the wire masks ``1 << (15 - offset)`` (#2310). """ def setUp(self) -> None: @@ -565,19 +565,16 @@ def test_tcp_flag_composites_resolve(self) -> None: every = 0 for member in Flags: every |= int(member) - self.assertEqual(every, 0xFFF0) + self.assertEqual(every, 0xFFF) self.assertEqual(int(Flags(every)), every) - # One past the 16-bit field the registry indexes bits of. - for beyond in (0x10000, -1, -0x10000, UNRESOLVABLE): + # One past the 12-bit flags field, where the data offset begins, and + # the values outside any width. + for beyond in (0x1000, 0x10000, -1, -0x1000, UNRESOLVABLE): with self.subTest(value=beyond): with self.assertRaises(ValueError): Flags(beyond) - # An unassigned low bit is in range and still composes, rather than - # being rejected: bits 0-3 of that field are the data offset. - self.assertEqual(int(Flags(1)), 1) - def test_the_other_flag_registry_composes(self) -> None: """``CommandType`` composes; ``TransportProtocol`` is not in this group. @@ -717,11 +714,11 @@ def test_the_guard_leaves_the_other_lookup_paths_alone(self) -> None: broken either would fail rather than merely go unmeasured. The fallback is probed with a two-part check: a *registered* default - (``1 << 14``, ``SYN``) is genuinely consulted and resolves, while + (``1 << 1``, ``SYN``) is genuinely consulted and resolves, while ``Flags.get(UNRESOLVABLE, 0)`` raises the same original-key error as no default at all. ``0`` is in-bounds for ``Flags``'s own - ``_missing_`` (``0 <= value <= 0xFFFF``), but it is not a *registered* - ``Flags`` member (``Flags``'s smallest declared value is ``1 << 4``), + ``_missing_`` (``0 <= value <= 0xFFF``), but it is not a *registered* + ``Flags`` member (``Flags``'s smallest declared value is ``1 << 0``), and GitHub issue #864 restricts ``default`` to a plain ``_value2member_map_`` lookup -- the same accepted cost as an unassigned integer range, here on an :class:`~aenum.IntFlag` registry @@ -748,12 +745,12 @@ def test_the_guard_leaves_the_other_lookup_paths_alone(self) -> None: # Integer paths, which do. self.assertIs(TransportProtocol.get(1), TransportProtocol.tcp) - self.assertIs(Flags.get(1 << 14), Flags.SYN) + self.assertIs(Flags.get(1 << 1), Flags.SYN) # And the fallback the guard's ValueError is what triggers: GitHub issue # #584's ``get(key, default)``, which an EnumError would have walked past. # A registered default is genuinely consulted and resolves. - self.assertIs(Flags.get(UNRESOLVABLE, 1 << 14), Flags.SYN) + self.assertIs(Flags.get(UNRESOLVABLE, 1 << 1), Flags.SYN) # ``0`` is not a registered ``Flags`` member (GitHub issue #864), so # it does not resolve either -- the original key's error propagates, # the same as omitting the default outright. @@ -1227,26 +1224,30 @@ def test_the_tcp_flags_checker_covers_the_value_domain_not_the_bit_offsets(self) """The template's checker, and why a bit-offset one could not be kept. A ``Flags.FLAG`` of ``4 <= value <= 15`` covers the *offsets* the IANA - registry indexes, while the members it generates are ``1 << offset``. - The template never interpolated it, so it was never wrong in practice; - interpolating it unchanged would have rejected every composite, every - single member above bit 3, and ``Flags(0)``. + registry indexes, while the members it generates are their wire masks, + ``1 << (15 - offset)`` (#2310). The template never interpolated it, so + it was never wrong in practice; interpolating it unchanged would have + rejected ``FIN``, ``SYN``, every member from ``ACK`` up, most + composites, and ``Flags(0)``. """ from pcapkit.vendor.tcp.flags import Flags as VendorFlags checker = VendorFlags.FLAG - self.assertIn('0xFFFF', checker) + self.assertRegex(checker, r'<= 0xFFF$') from pcapkit.const.tcp.flags import Flags + old = 'isinstance(value, int) and 4 <= value <= 15' for member in Flags: value = int(member) with self.subTest(member=member.name): self.assertTrue(eval(checker, {'value': value}), # pylint: disable=eval-used f'the checker rejects its own member {member.name}={value}') - self.assertFalse(eval('isinstance(value, int) and 4 <= value <= 15', - {'value': value}), # pylint: disable=eval-used - f'the old checker accepted {member.name}={value}') + for value in (0, int(Flags.FIN), int(Flags.SYN | Flags.ACK), int(Flags.CWR)): + with self.subTest(value=hex(value)): + self.assertTrue(eval(checker, {'value': value})) # pylint: disable=eval-used + self.assertFalse(eval(old, {'value': value}), # pylint: disable=eval-used + f'the old checker accepted {value:#x}') def test_self_check_of_the_repr_percent_format_walk(self) -> None: """:func:`percent_format_lines_in_repr` against its own fixtures. diff --git a/tests/const/test_const_registry_protocol.py b/tests/const/test_const_registry_protocol.py index b1a0376d10..44eaccac05 100644 --- a/tests/const/test_const_registry_protocol.py +++ b/tests/const/test_const_registry_protocol.py @@ -201,25 +201,27 @@ class _PreConversionFlags(IntFlag): hand-copied ``get`` (omitted here -- irrelevant to ``_missing_``) and this ``_missing_``, copied verbatim. Kept inline rather than fetched from git history so :class:`TCPFlagsConversionTests` runs offline and the - comparison is exact rather than approximate. + comparison is exact rather than approximate. The member values and the + range check track the generated module's, wire masks in the 12-bit flags + field since #2310. """ - Reserved_4 = 1 << 4 - Reserved_5 = 1 << 5 - Reserved_6 = 1 << 6 - AE = 1 << 7 - CWR = 1 << 8 - ECE = 1 << 9 - URG = 1 << 10 - ACK = 1 << 11 - PSH = 1 << 12 - RST = 1 << 13 - SYN = 1 << 14 - FIN = 1 << 15 + Reserved_4 = 1 << 11 + Reserved_5 = 1 << 10 + Reserved_6 = 1 << 9 + AE = 1 << 8 + CWR = 1 << 7 + ECE = 1 << 6 + URG = 1 << 5 + ACK = 1 << 4 + PSH = 1 << 3 + RST = 1 << 2 + SYN = 1 << 1 + FIN = 1 << 0 @classmethod def _missing_(cls, value: 'Any') -> 'Any': - if not (isinstance(value, int) and 0 <= value <= 0xFFFF): + if not (isinstance(value, int) and 0 <= value <= 0xFFF): raise ValueError(f'{value!r} is not a valid {cls.__name__}') return super()._missing_(value) @@ -276,14 +278,14 @@ def test_missing_resolves_identically_to_the_pre_conversion_shape(self) -> None: :class:`~aenum.Flag` machinery exactly as it did when ``Flags`` inherited from ``IntFlag`` directly. Swept rather than sampled at a few boundary values, across every declared member, every boundary of - the 16-bit field the range check bounds, and several composites, + the 12-bit field the range check bounds, and several composites, so a widened or narrowed range check would be caught here rather than only in the composite-specific tests in ``tests/const/test_const_enum_builtin_parity.py``. """ from pcapkit.const.tcp.flags import Flags - probes = [0, 1, 0xF, 0x10, 0xFFFF, 0x10000, -1, -0x10000, + probes = [0, 1, 0xF, 0x10, 0xFFF, 0x1000, 0xFFFF, 0x10000, -1, -0x1000, 1 << 70, 0xFFF0] for member in _PreConversionFlags: probes.append(int(member)) diff --git a/tests/const/test_const_tcp_flags_wire_masks_2310_unit.py b/tests/const/test_const_tcp_flags_wire_masks_2310_unit.py new file mode 100644 index 0000000000..ee3b1ab67f --- /dev/null +++ b/tests/const/test_const_tcp_flags_wire_masks_2310_unit.py @@ -0,0 +1,137 @@ +# -*- coding: utf-8 -*- +"""#2310 -- :class:`~pcapkit.const.tcp.flags.Flags` members are their wire masks. + +IANA's TCP Header Flags registry numbers the bits from the most significant end +of the 16-bit Data Offset/flags word, so FIN is offset 15 and CWR offset 8. The +crawler generated ``1 << offset``, bit-reversed against the wire: ``int()`` of a +parsed SYN/ACK's ``connection`` was ``0x4800`` where the header carries +``0x012``. Each member is now ``1 << (15 - offset)``, its mask in the low 12 bits +of that word, and the registry's range guard is that 12-bit field. + +""" +from __future__ import annotations + +import importlib.util +import struct +import unittest + +from tests._support import reimport_once_per_class + +#: Test tier (#1958): enum mechanics and a hand-picked round trip. +__tier__ = 'ALL' + +RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper') +HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS) + +#: Each member and its mask in the flags field, as :rfc:`9293` section 3.1 +#: and :rfc:`9768` lay it out. +WIRE_MASKS = ( + ('FIN', 0x001), ('SYN', 0x002), ('RST', 0x004), ('PSH', 0x008), + ('ACK', 0x010), ('URG', 0x020), ('ECE', 0x040), ('CWR', 0x080), + ('AE', 0x100), ('Reserved_6', 0x200), ('Reserved_5', 0x400), ('Reserved_4', 0x800), +) + +#: A SYN/ACK as a handshake carries it: data offset 7, flags ``0x012``, and the +#: MSS, NOP, NOP and SACK-permitted options. +SYN_ACK = (struct.pack('!HHIIHHHH', 80, 3372, 0x290E5CE5, 0x38AFFE14, (7 << 12) | 0x012, + 5840, 0xC20E, 0) + + bytes.fromhex('020405b4 01010402')) + + +def _segment(word: 'int') -> 'bytes': + """A bare 20-octet header whose Data Offset/flags word is ``word``.""" + return struct.pack('!HHIIHHHH', 1, 2, 0, 0, word, 0xFFFF, 0, 0) + + +@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') +class TCPFlagsWireMasksTests(unittest.TestCase): + """The registry's integers mean what the header's bits mean.""" + + def setUp(self) -> None: + reimport_once_per_class(self) + + def test_each_member_is_its_wire_mask(self) -> None: + from pcapkit.const.tcp.flags import Flags + + for name, mask in WIRE_MASKS: + with self.subTest(member=name): + self.assertEqual(int(Flags[name]), mask) + self.assertIs(Flags(mask), Flags[name]) + + def test_a_composite_is_the_header_bits(self) -> None: + from pcapkit.const.tcp.flags import Flags + + self.assertEqual(int(Flags.SYN | Flags.ACK), 0x12) + self.assertEqual(Flags(0x12), Flags.SYN | Flags.ACK) + self.assertEqual(int(Flags.FIN | Flags.ACK), 0x11) + self.assertEqual(int(Flags.CWR | Flags.ECE | Flags.SYN), 0xC2) + + def test_the_guard_is_the_twelve_bit_flags_field(self) -> None: + """Bits above the flags field are the data offset, so they are refused.""" + from pcapkit.const.tcp.flags import Flags + + self.assertEqual(int(Flags(0xFFF)), 0xFFF) + for value in (0x1000, 0x4800, 0xFFFF): + with self.subTest(value=hex(value)): + with self.assertRaises(ValueError): + Flags(value) + + def test_a_parsed_syn_ack_reports_its_header_bits(self) -> None: + from pcapkit.const.tcp.flags import Flags + from pcapkit.protocols.transport.tcp import TCP + + tcp = TCP(SYN_ACK, len(SYN_ACK)) + word = struct.unpack('!H', SYN_ACK[12:14])[0] + + self.assertEqual(int(tcp.connection), word & 0x0FFF) + self.assertEqual(int(tcp.connection), 0x012) + self.assertEqual(tcp.connection, Flags.SYN | Flags.ACK) + self.assertEqual(int(tcp.info.connection), 0x012) + + # The round trip stays byte-exact. + self.assertEqual(TCP.from_data(tcp.info).data, SYN_ACK) + + def test_every_flags_octet_reads_back_as_its_value(self) -> None: + """The eight flags the parser reports, each combination, as the octet they came from.""" + from pcapkit.protocols.transport.tcp import TCP + + for octet in range(0x100): + with self.subTest(octet=hex(octet)): + raw = _segment((5 << 12) | octet) + tcp = TCP(raw, len(raw)) + self.assertEqual(int(tcp.connection), octet) + self.assertEqual(TCP.from_data(tcp.info).data, raw) + + def test_make_builds_the_header_bits_its_connection_names(self) -> None: + from pcapkit.protocols.transport.tcp import TCP + + made = TCP(srcport=1, dstport=2, syn=True, ack=True) + self.assertEqual(int(made.connection), 0x012) + self.assertEqual(struct.unpack('!H', made.data[12:14])[0] & 0x0FFF, int(made.connection)) + + +@unittest.skipUnless(importlib.util.find_spec('requests') is not None, + 'pcapkit.vendor needs requests') +class TCPFlagsCrawlerWireMasksTests(unittest.TestCase): + """The crawler, not only the committed module, generates the wire masks.""" + + def test_the_crawler_generates_wire_masks(self) -> None: + from pcapkit.vendor.tcp.flags import Flags as VendorFlags + + crawler = object.__new__(VendorFlags) + crawler.NAME = 'Flags' + crawler.DOCS = VendorFlags.__doc__ + rows = [ + 'Bit,Name,Reference', + '8,CWR (Congestion Window Reduced),[RFC3168]', + '11,Acknowledgment field is significant (ACK),[RFC9293]', + '14,Synchronize sequence numbers (SYN),[RFC9293]', + '15,No more data from sender (FIN),[RFC9293]', + ] + crawler.record = crawler.count(rows) + lines = [line.splitlines()[-1].strip() for line in crawler.process(rows)] + self.assertEqual(lines, ['CWR = 1 << 7', 'ACK = 1 << 4', 'SYN = 1 << 1', 'FIN = 1 << 0']) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/dumpkit/test_nameless_enum_rendering_unit.py b/tests/dumpkit/test_nameless_enum_rendering_unit.py index 1b1ca8148a..f2199422b7 100644 --- a/tests/dumpkit/test_nameless_enum_rendering_unit.py +++ b/tests/dumpkit/test_nameless_enum_rendering_unit.py @@ -31,8 +31,8 @@ class StdFlags(enum.IntFlag): """ - ACK = 2048 - SYN = 16384 + ACK = 16 + SYN = 2 class AnnotatedFlags(enum.IntFlag): @@ -85,10 +85,10 @@ class NamelessEnumRenderingTests(unittest.TestCase): the literal four characters ``None`` into the name half. The fallback is the value's own decimal spelling -- ``'Flags::0 [0]'``, - ``'Flags::8 [8]'`` -- chosen for reasons these tests pin: + ``'BindingUpdateFlag::8 [8]'`` -- chosen for reasons these tests pin: * It is what the enumeration libraries themselves already use for an - undeclared residue. ``Flags(2057).name`` is ``'ACK|9'``, naming the + undeclared residue. ``BindingUpdateFlag(32777).name`` is ``'A|9'``, naming the declared bit and giving the leftovers as one decimal number; a wholly-undeclared value is that rendering with no declared bit in front. * It cannot be confused with a member name, because a Python identifier may @@ -101,7 +101,7 @@ class NamelessEnumRenderingTests(unittest.TestCase): The values probed are derived from each enumeration's own declared bits rather than written down, which is #702. A literal such as ``65536`` is ``0x10000`` -- one bit past the sixteen-bit field - :class:`~pcapkit.const.tcp.flags.Flags` bounds its + :class:`~pcapkit.const.mh.binding_update_flag.BindingUpdateFlag` bounds its :meth:`~enum.Enum._missing_` to. That guard is correct, so the probe has to respect it: a value the registry is right to refuse cannot also be a value the dumper renders. Every one of the library's flag registries carries such @@ -121,20 +121,25 @@ def test_scalar_return_renders_a_nameless_member_as_its_value(self) -> None: Covers the nameless zero *and* every non-zero nameless value the registry admits, because a fix that special-cased zero would pass on - ``Flags(0)`` alone and still emit ``'Flags::None [8]'`` in place of - ``'Flags::8 [8]'``. + ``registry(0)`` alone and still emit ``'BindingUpdateFlag::None [8]'`` + in place of ``'BindingUpdateFlag::8 [8]'``. + + :class:`~pcapkit.const.mh.binding_update_flag.BindingUpdateFlag` is the + :mod:`aenum` registry swept: :class:`~pcapkit.const.tcp.flags.Flags` + declares every bit of its field, so zero is its only nameless value + (#2310). Both libraries are swept from the same derivation, so this is still the demonstration that the rendering is not an :mod:`aenum` quirk -- stdlib :class:`enum.IntFlag` spells a wholly-undeclared value the same way. """ - from pcapkit.const.tcp.flags import Flags + from pcapkit.const.mh.binding_update_flag import BindingUpdateFlag from pcapkit.dumpkit.common import make_dumper dumper = make_dumper(BaseDumper)() - for registry, library in ((Flags, 'aenum'), (StdFlags, 'enum')): + for registry, library in ((BindingUpdateFlag, 'aenum'), (StdFlags, 'enum')): values = _nameless_values(registry) # Without this the loop below could sweep nothing at all and still # report a pass, which is how a derived probe set rots silently @@ -156,22 +161,23 @@ def test_scalar_return_renders_a_nameless_member_as_its_value(self) -> None: def test_named_members_are_untouched(self) -> None: """The control. Only the name half of a *nameless* member gets the fallback. - ``Flags(2049)`` matters most here: its name is ``'ACK|1'``, so it needs - no fallback, and it is the precedent the fallback follows. If the guard + ``BindingUpdateFlag(32769)`` matters most here: its name is ``'A|1'``, + so it needs no fallback, and it is the precedent the fallback follows. If the guard were written as "compose the name from the value" rather than "use the value when there is no name", this is the assertion that would catch it. """ + from pcapkit.const.mh.binding_update_flag import BindingUpdateFlag from pcapkit.const.tcp.flags import Flags from pcapkit.dumpkit.common import make_dumper dumper = make_dumper(BaseDumper)() - self.assertEqual(dumper.object_hook(Flags.ACK), 'Flags::ACK [2048]') - self.assertEqual(dumper.object_hook(Flags.SYN | Flags.ACK), 'Flags::ACK|SYN [18432]') - self.assertEqual(dumper.object_hook(Flags(2049)), 'Flags::ACK|1 [2049]') - self.assertEqual(dumper.object_hook(Flags(2057)), 'Flags::ACK|9 [2057]') + self.assertEqual(dumper.object_hook(Flags.ACK), 'Flags::ACK [16]') + self.assertEqual(dumper.object_hook(Flags.SYN | Flags.ACK), 'Flags::ACK|SYN [18]') + self.assertEqual(dumper.object_hook(BindingUpdateFlag(32769)), 'BindingUpdateFlag::A|1 [32769]') + self.assertEqual(dumper.object_hook(BindingUpdateFlag(32777)), 'BindingUpdateFlag::A|9 [32777]') # An explicitly declared zero keeps its declared name, which is the # clearest demonstration that the guard keys on the *name* and not on the @@ -192,6 +198,7 @@ def test_multidict_key_path_renders_a_nameless_key(self) -> None: bracketed half. """ + from pcapkit.const.mh.binding_update_flag import BindingUpdateFlag from pcapkit.const.tcp.flags import Flags from pcapkit.corekit.multidict import MultiDict, OrderedMultiDict from pcapkit.dumpkit.common import make_dumper @@ -200,15 +207,15 @@ def test_multidict_key_path_renders_a_nameless_key(self) -> None: multidict = MultiDict() multidict.add(Flags(0), 'flagless') - multidict.add(Flags(8), 'undeclared-bit') + multidict.add(BindingUpdateFlag(8), 'undeclared-bit') multidict.add(Flags.ACK, 'acknowledged') converted = dumper.object_hook(multidict) self.assertEqual(converted['Flags::0 [0]'], ['flagless']) - self.assertEqual(converted['Flags::8 [8]'], ['undeclared-bit']) - self.assertEqual(converted['Flags::ACK [2048]'], ['acknowledged']) + self.assertEqual(converted['BindingUpdateFlag::8 [8]'], ['undeclared-bit']) + self.assertEqual(converted['Flags::ACK [16]'], ['acknowledged']) self.assertNotIn('Flags::None [0]', converted) - self.assertNotIn('Flags::None [8]', converted) + self.assertNotIn('BindingUpdateFlag::None [8]', converted) ordered = OrderedMultiDict() ordered.add(Flags(0), 'first') @@ -306,8 +313,8 @@ def test_a_value_past_the_field_is_refused_rather_than_rendered(self) -> None: Every flag registry bounds its :meth:`~enum.Enum._missing_` to the width of its own field, and the widths differ: three bits for :class:`~pcapkit.const.ftp.command.CommandType`, eight for the other - Mobility Header flags, sixteen for ``BindingUpdateFlag`` and - :class:`~pcapkit.const.tcp.flags.Flags`. One literal therefore cannot + Mobility Header flags, twelve for :class:`~pcapkit.const.tcp.flags.Flags` + and sixteen for ``BindingUpdateFlag``. One literal therefore cannot mean "past the field" for all of them, which is the whole reason :func:`_field_mask` derives it per registry. @@ -400,8 +407,8 @@ def _declared_bits(registry: 'FlagRegistry') -> int: Iteration over a flag enumeration yields the canonical single-bit members, skipping a zero member and any alias, which is what makes this the registry's - *declared bits* rather than a member count: ``Flags`` declares twelve members - from ``1 << 4`` upwards, so this is ``0xFFF0``. + *declared bits* rather than a member count: ``Flags`` declares every bit + of its 12-bit field, so this is ``0xFFF``. Args: registry: The flag enumeration to inspect. @@ -422,7 +429,7 @@ def _field_mask(registry: 'FlagRegistry') -> int: Every flag registry in the library bounds its :meth:`~enum.Enum._missing_` to the field its declared bits live in, and for every one of them that bound is exactly the smallest all-ones mask covering every declared bit -- - ``0xFFFF`` for ``Flags``' ``1 << 4 .. 1 << 15``, ``0xFF`` for the eight-bit + ``0xFFF`` for ``Flags``' ``1 << 0 .. 1 << 11``, ``0xFF`` for the eight-bit Mobility Header flags, ``0x07`` for the three-bit :class:`~pcapkit.const.ftp.command.CommandType`. @@ -458,8 +465,7 @@ def _nameless_values(registry: 'FlagRegistry') -> 'tuple[int, ...]': multi-bit case, and it is the largest value the field admits with no declared bit in it, which is what a hand-written probe such as ``9``, or the out-of-range ``65536``, reaches for. Returning every subset would be exhaustive - and is not worth 8192 subtests for :class:`StdFlags`' thirteen undeclared - bits. + and is not worth a subtest per subset for a wide field with few declared bits. Nothing out of range is returned, which is #702: the values are all masked into the registry's own field, so a registry that legitimately refuses diff --git a/tests/protocols/transport/test_tcp_udp_unit.py b/tests/protocols/transport/test_tcp_udp_unit.py index 404ea962f4..c0956960c5 100644 --- a/tests/protocols/transport/test_tcp_udp_unit.py +++ b/tests/protocols/transport/test_tcp_udp_unit.py @@ -1607,8 +1607,7 @@ def test_a_flagless_segment_seeds_its_connection_flags_as_an_enum(self) -> None: segment rather than a number for a flagless one alone. The name half of that string is the value's own decimal spelling -- the same - spelling the enumeration library uses for an undeclared residue, as in - ``Flags(2049).name == 'ACK|1'`` -- because + spelling the enumeration library uses for an undeclared residue -- because :func:`~pcapkit.dumpkit.common.make_dumper`'s hook guards a nameless composite member rather than interpolating its ``o.name`` of ``None`` (#648). @@ -1685,7 +1684,7 @@ def segment(flags_octet: 'int') -> 'bytes': rendered = hook(None, proto.info.connection) self.assertEqual(rendered, 'Flags::0 [0]') self.assertNotIn('None', rendered) - self.assertEqual(hook(None, Enum_Flags.ACK), 'Flags::ACK [2048]') + self.assertEqual(hook(None, Enum_Flags.ACK), 'Flags::ACK [16]') if __name__ == '__main__':