Skip to content

Commit eeed87f

Browse files
committed
fix(tcp): NS flag is written by make() but never parsed
make() packed NS into bit 103 and the OffsetFlag schema declared it, but the reader and _make_data had ns commented out (2e39aeb, a typing-only pass with no stated reason), so a parsed segment had no flags.ns and a from_data rebuild cleared the bit silently. - data/transport/tcp.py: restore ns on the Flags data model - transport/tcp.py: read ns from schema.offset, emit it in _make_data, and list tcp.flags.ns in the module's header table - tcp.rst: drop the footnote saying NS is left unexposed - test_tcp_udp_unit.py: add ns to the hand-built Flags stand-in - new test_tcp_ns_flag_roundtrip_unit.py Closes #1098
1 parent 9d561a6 commit eeed87f

5 files changed

Lines changed: 72 additions & 11 deletions

File tree

‎docs/source/pcapkit/protocols/transport/tcp.rst‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ Octets Bits Name Description
1818
8 64 ``tcp.ack`` Acknowledgement Number (if ACK set)
1919
12 96 ``tcp.hdr_len`` Data Offset
2020
12 100 Reserved (must be ``\x00``)
21-
12 103 ``tcp.flags.ns`` ECN Concealment Protection (NS) [*]_
21+
12 103 ``tcp.flags.ns`` ECN Concealment Protection (NS)
2222
13 104 ``tcp.flags.cwr`` Congestion Window Reduced (CWR)
2323
13 105 ``tcp.flags.ece`` ECN-Echo (ECE)
2424
13 106 ``tcp.flags.urg`` Urgent (URG)
@@ -535,7 +535,3 @@ Data Models
535535
.. rubric:: Footnotes
536536

537537
.. [*] https://en.wikipedia.org/wiki/Transmission_Control_Protocol
538-
.. [*] The NS bit is read off the wire but is **not** surfaced in
539-
:class:`~pcapkit.protocols.data.transport.tcp.Flags`. :rfc:`3540` was
540-
reclassified as Historic, so the field is left unexposed; the row is kept
541-
because the bit is part of the header as the RFC defines it.

‎pcapkit/protocols/data/transport/tcp.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class Flags(Data):
4949
"""Data model for TCP flags."""
5050

5151
#: ECN-nonce concealment protection.
52-
#ns: 'bool'
52+
ns: 'bool'
5353
#: Congestion window reduced.
5454
cwr: 'bool'
5555
#: ECN-Echo.
@@ -68,7 +68,7 @@ class Flags(Data):
6868
fin: 'bool'
6969

7070
if TYPE_CHECKING:
71-
def __init__(self, cwr: 'bool', ece: 'bool', urg: 'bool', ack: 'bool',
71+
def __init__(self, ns: 'bool', cwr: 'bool', ece: 'bool', urg: 'bool', ack: 'bool',
7272
psh: 'bool', rst: 'bool', syn: 'bool', fin: 'bool') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin
7373

7474

‎pcapkit/protocols/transport/tcp.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
8 64 ``tcp.ack`` Acknowledgement Number (if ACK set)
2121
12 96 ``tcp.hdr_len`` Data Offset
2222
12 100 Reserved (must be ``\\x00``)
23-
12 103 ECN Concealment Protection (NS), not parsed
23+
12 103 ``tcp.flags.ns`` ECN Concealment Protection (NS)
2424
13 104 ``tcp.flags.cwr`` Congestion Window Reduced (CWR)
2525
13 105 ``tcp.flags.ece`` ECN-Echo (ECE)
2626
13 106 ``tcp.flags.urg`` Urgent (URG)
@@ -471,7 +471,7 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_TCP':
471471
ack=schema.ack,
472472
hdr_len=schema.offset['offset'] * 4,
473473
flags=Data_Flags(
474-
#ns=bool(schema.offset['ns']),
474+
ns=bool(schema.offset['ns']),
475475
cwr=bool(schema.flags['cwr']),
476476
ece=bool(schema.flags['ece']),
477477
urg=bool(schema.flags['urg']),
@@ -692,7 +692,7 @@ def _make_data(cls, data: 'Data_TCP') -> 'dict[str, Any]': # type: ignore[overr
692692
'dstport': data.dstport,
693693
'seq_no': data.seq,
694694
'ack_no': data.ack,
695-
#'ns': data.flags.ns,
695+
'ns': data.flags.ns,
696696
'cwr': data.flags.cwr,
697697
'ece': data.flags.ece,
698698
'urg': data.flags.urg,
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
"""#1098 -- the TCP NS flag round-trips through read, ``make()`` and ``from_data``.
2+
3+
:meth:`TCP.make <pcapkit.protocols.transport.tcp.TCP.make>` accepts ``ns`` and
4+
packs it into bit 103 (the low bit of byte 12), and
5+
:class:`~pcapkit.protocols.schema.transport.tcp.OffsetFlag` declares the bit. The
6+
reader and :meth:`TCP._make_data <pcapkit.protocols.transport.tcp.TCP._make_data>`
7+
must carry it too, so a parsed segment exposes ``flags.ns`` and a
8+
:meth:`~pcapkit.protocols.protocol.ProtocolBase.from_data` rebuild keeps the bit
9+
on the wire rather than clearing it silently.
10+
11+
"""
12+
from __future__ import annotations
13+
14+
import importlib.util
15+
import unittest
16+
17+
from tests._support import reimport_once_per_class
18+
19+
RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper')
20+
HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS)
21+
22+
23+
@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
24+
class TCPNSFlagRoundTripUnitTests(unittest.TestCase):
25+
def setUp(self) -> None:
26+
reimport_once_per_class(self)
27+
28+
def test_make_sets_ns_on_the_wire(self) -> None:
29+
"""``make(ns=True)`` sets the low bit of byte 12, next to a data offset of 5."""
30+
from pcapkit.protocols.transport.tcp import TCP
31+
32+
self.assertEqual(TCP(srcport=1, dstport=2, ns=True).data[12], 0x51)
33+
self.assertEqual(TCP(srcport=1, dstport=2, ns=False).data[12], 0x50)
34+
35+
def test_read_exposes_ns(self) -> None:
36+
"""A parsed segment reports ``flags.ns`` as read from bit 103."""
37+
from pcapkit.protocols.transport.tcp import TCP
38+
39+
for ns in (True, False):
40+
with self.subTest(ns=ns):
41+
raw = TCP(srcport=1, dstport=2, ns=ns).data
42+
self.assertIs(TCP(raw).info.flags.ns, ns)
43+
44+
def test_from_data_keeps_ns(self) -> None:
45+
"""A ``from_data`` rebuild of a parsed segment reproduces its bytes, NS included."""
46+
from pcapkit.protocols.transport.tcp import TCP
47+
48+
for ns in (True, False):
49+
with self.subTest(ns=ns):
50+
raw = TCP(srcport=1, dstport=2, ns=ns, ack=True).data
51+
rebuilt = TCP.from_data(TCP(raw).info)
52+
self.assertEqual(rebuilt.data, raw)
53+
self.assertIs(rebuilt.info.flags.ns, ns)
54+
55+
def test_flags_field_order(self) -> None:
56+
"""``ns`` leads the flags, in wire order, ahead of ``cwr``."""
57+
from pcapkit.protocols.transport.tcp import TCP
58+
59+
flags = TCP(TCP(srcport=1, dstport=2).data).info.flags
60+
self.assertEqual(list(flags.keys()),
61+
['ns', 'cwr', 'ece', 'urg', 'ack', 'psh', 'rst', 'syn', 'fin'])
62+
63+
64+
if __name__ == '__main__':
65+
unittest.main()

‎tests/protocols/transport/test_tcp_udp_unit.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ def test_tcp_index_and_make_data_preserve_flags_and_ports(self) -> None:
119119
from pcapkit.const.reg.transtype import TransType
120120
from pcapkit.protocols.transport.tcp import TCP
121121

122-
flags = DummyData(cwr=False, ece=False, urg=False, ack=True, psh=True, rst=False, syn=False, fin=True)
122+
flags = DummyData(ns=False, cwr=False, ece=False, urg=False, ack=True, psh=True, rst=False, syn=False, fin=True)
123123
data = DummyData(
124124
srcport=443,
125125
dstport=59000,

0 commit comments

Comments
 (0)