Skip to content

Commit fea3246

Browse files
authored
docs(vendor,const): MPTCPOption cites RFC 8684, which obsoletes 6824 (#2330) (#2380)
Problem: the class docstring of the MPTCPOption crawler, mirrored into the generated const module, and two citations in pcapkit/protocols/transport/ tcp.py (the Multipath TCP option-kind map entry, and _read_mode_mp's generic Figure 3) all cited RFC 6824. IANA's "MPTCP Option Subtypes" and "TCP Option Kind Numbers" registries cite RFC 8684, which obsoletes 6824 as of 2020-03, and Figure 3 is identical in both RFCs. The eight other 6824 citations in tcp.py stay -- those document the legacy v0 wire layouts (the ADD_ADDR IPVer nibble, the MP_PRIO 4-octet AddrID form) the parser still accepts, which 8684 itself records as removed. Fix: cite 8684 at all four sites. The crawler/const pair is edited identically, no re-crawl. Test: tests/const/test_const_mp_tcp_option_rfc_2330_unit.py pins the table against the obsoleted RFC rather than a single value (so a legitimate future IANA assignment to an unassigned subtype, which 8684 section 7.2 allows, won't fail it), the class docstring and the two tcp.py sites against the registries' own RFC, and template/module agreement. 5 of 6 tests fail on the parent commit; the agreement check passes there since both sides said 6824 together.
1 parent 884df0f commit fea3246

4 files changed

Lines changed: 225 additions & 4 deletions

File tree

‎pcapkit/const/tcp/mp_tcp_option.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525

2626
class MPTCPOption(EnumRegistry, IntEnum):
27-
"""[MPTCPOption] Multipath TCP options [:rfc:`6824`]"""
27+
"""[MPTCPOption] Multipath TCP options [:rfc:`8684`]"""
2828

2929
#: Multipath Capable [:rfc:`8684#section-3.1`]
3030
MP_CAPABLE = 0x0

‎pcapkit/protocols/transport/tcp.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ class TCP(Transport[Data_TCP, Schema_TCP],
398398
Enum_Option.Quick_Start_Response: 'qs', # [RFC 4782] Quick-Start Response
399399
Enum_Option.User_Timeout_Option: 'timeout', # [RFC 5482] User Timeout Option
400400
Enum_Option.TCP_Authentication_Option: 'ao', # [RFC 5925] TCP Authentication Option
401-
Enum_Option.Multipath_TCP: 'mp', # [RFC 6824] Multipath TCP
401+
Enum_Option.Multipath_TCP: 'mp', # [RFC 8684] Multipath TCP
402402
Enum_Option.TCP_Fast_Open_Cookie: 'fastopen', # [RFC 7413] Fast Open
403403
},
404404
) # type: DefaultDict[int, str | tuple[OptionParser, OptionConstructor]]
@@ -1530,7 +1530,7 @@ def _read_mode_ao(self, schema: 'Schema_Authentication', *, options: 'Option') -
15301530
def _read_mode_mp(self, schema: 'Schema_MPTCP', *, options: 'Option') -> 'Data_MPTCP': # pylint: disable=unused-argument
15311531
"""Read Multipath TCP option.
15321532
1533-
Structure of ``MP-TCP`` [:rfc:`6824`]:
1533+
Structure of ``MP-TCP`` [:rfc:`8684`]:
15341534
15351535
.. code-block:: text
15361536

‎pcapkit/vendor/tcp/mp_tcp_option.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020

2121
class MPTCPOption(Vendor):
22-
"""Multipath TCP options [:rfc:`6824`]"""
22+
"""Multipath TCP options [:rfc:`8684`]"""
2323

2424
#: Value limit checker.
2525
FLAG = 'isinstance(value, int) and 0 <= value <= 255'
Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
1+
# -*- coding: utf-8 -*-
2+
"""#2330 -- the MPTCP Option Subtypes table cites the RFC that defines it.
3+
4+
:class:`~pcapkit.const.tcp.mp_tcp_option.MPTCPOption` is generated from IANA's
5+
"MPTCP Option Subtypes" registry, whose own reference is
6+
``<xref type="rfc" data="rfc8684"/>``: RFC 8684 specifies MPTCP v1 and
7+
obsoletes the experimental v0 of RFC 6824, which the registry's note mentions
8+
only to point at for the v0 subtypes. Every assigned record in the registry
9+
carries an ``rfc8684`` spec -- the ``0x9-0xe`` *Unassigned* range carries an
10+
empty ``<spec/>`` and renders no member at all -- so every member comment the
11+
crawler renders cites RFC 8684, with a ``#section-...`` anchor wherever the
12+
record's spec names a section and without one for ``Reserved_for_Private_Use``
13+
(``0xf``), whose spec is a bare ``<xref type="rfc" data="rfc8684"/>``. The
14+
option kind that carries these subtypes cites RFC 8684 too --
15+
:attr:`Option.Multipath_TCP <pcapkit.const.tcp.option.Option.Multipath_TCP>`,
16+
kind 30, which is what IANA's "TCP Option Kind Numbers" registry gives for it.
17+
The class docstring, however, cited RFC 6824, in the crawler's template
18+
(:file:`pcapkit/vendor/tcp/mp_tcp_option.py`) and in the generated module
19+
alike, naming an obsoleted document as the authority for a table none of whose
20+
entries come from it.
21+
22+
The assertions are the invariants that defect broke, rather than the one string
23+
that was wrong:
24+
25+
* the class docstring names the registry's own reference, and names nothing
26+
else, so the table's attribution cannot sit on a document its entries do not
27+
come from
28+
(:meth:`MPTCPOptionCitationTests.test_the_class_docstring_cites_the_registrys_rfc`);
29+
* no citation anywhere in the generated table reaches back to the obsoleted v0
30+
document, which is what a partial regeneration would reintroduce
31+
(:meth:`MPTCPOptionCitationTests.test_the_table_cites_no_obsoleted_rfc`).
32+
This is deliberately not a pin on *every* role equalling RFC 8684: RFC 8684
33+
section 7.2 puts future assignments to this registry under Standards Action,
34+
"a reference to its specification" each, so a legitimate future subtype would
35+
add a citation to some other RFC;
36+
* the crawler's template and the module it generates carry that citation
37+
identically, since the generator rewrites the module from the template and a
38+
fix to one alone would be undone by the next Vendor Update
39+
(:meth:`MPTCPOptionCitationTests.test_the_template_and_the_module_agree`);
40+
* the parser's subtype-agnostic citations name the registry's RFC as well
41+
(:meth:`MPTCPOptionCitationTests.test_the_option_kind_map_cites_the_registrys_rfc`
42+
and
43+
:meth:`MPTCPOptionCitationTests.test_the_generic_mptcp_figure_cites_the_registrys_rfc`),
44+
for the reason the next paragraph gives.
45+
46+
:mod:`pcapkit.protocols.transport.tcp` carried the same defect, corrected
47+
alongside the table: the option-kind map's comment against
48+
``Option.Multipath_TCP``, which attributes the kind rather than any subtype's
49+
layout, and the ``Structure of MP-TCP`` figure in ``TCP._read_mode_mp``, which
50+
draws the subtype-agnostic option header that RFC 8684 figure 3 and RFC 6824
51+
figure 3 draw identically. Neither states anything v0-specific, so neither had
52+
reason to name the obsoleted document. The RFC 6824 citations that *remain* in
53+
that module and its schemas are deliberate and out of scope: those document the
54+
legacy v0 layouts the parsers still accept -- the ADD_ADDR ``IPVer`` nibble,
55+
which RFC 8684 redraws as reserved bits and ``E``, and the 4-octet MP_PRIO
56+
``AddrID`` form, which RFC 8684 section 5 removed -- which is a claim about the
57+
wire rather than about this registry. The assertions above therefore read the
58+
map entry and the docstring each name, never the whole module.
59+
60+
Every assertion here fails on the parent commit except
61+
:meth:`MPTCPOptionCitationTests.test_the_template_and_the_module_agree`, which
62+
passes there: the parent cites RFC 6824 in the template and in the module
63+
consistently, so the agreement check catches only a fix applied to one side of
64+
the pair.
65+
66+
The crawler is read as source rather than imported, so this runs on the legs
67+
where the ``vendor`` extra is absent: that extra is installed by the
68+
``project-tests`` and ``gate`` jobs, and by the ``test`` and ``integration``
69+
jobs only on their latest-interpreter cell.
70+
71+
"""
72+
from __future__ import annotations
73+
74+
import ast
75+
import importlib.util
76+
import pathlib
77+
import re
78+
import unittest
79+
80+
from tests._support import reimport_once_per_class
81+
82+
#: Test tier (#1958): a citation in committed source and in a docstring, which no
83+
#: interpreter difference can reach.
84+
__tier__ = 'LATEST'
85+
86+
#: Repository root, i.e. the grandparent of the directory holding this file.
87+
ROOT = pathlib.Path(__file__).resolve().parents[2]
88+
89+
#: The generated table.
90+
CONST = ROOT / 'pcapkit' / 'const' / 'tcp' / 'mp_tcp_option.py'
91+
92+
#: The crawler whose template renders it.
93+
VENDOR = ROOT / 'pcapkit' / 'vendor' / 'tcp' / 'mp_tcp_option.py'
94+
95+
#: The parser that dispatches on these subtypes.
96+
TRANSPORT = ROOT / 'pcapkit' / 'protocols' / 'transport' / 'tcp.py'
97+
98+
#: The RFC the registry gives as its reference: MPTCP v1, which obsoletes RFC 6824's v0.
99+
REGISTRY_RFC = '8684'
100+
101+
#: The obsoleted document the registry's note points at for the v0 subtypes only.
102+
OBSOLETED_RFC = '6824'
103+
104+
#: An :rst:role:`rfc` role's number, with or without an anchor fragment after it.
105+
RFC_ROLE = re.compile(r':rfc:`(?P<rfc>\d+)(?:#[^`]*)?`')
106+
107+
#: A plain comment's bracketed attribution, as the option-kind map spells it.
108+
RFC_BRACKET = re.compile(r'\[RFC (?P<rfc>\d+)\]')
109+
110+
#: The option-kind map's entry for the kind that carries the MPTCP subtypes.
111+
KIND_ENTRY = 'Enum_Option.Multipath_TCP:'
112+
113+
RUNTIME_DEPS = ('tbtrim', 'aenum', 'chardet', 'dictdumper')
114+
HAS_RUNTIME = all(importlib.util.find_spec(name) is not None for name in RUNTIME_DEPS)
115+
116+
117+
def _class_docstring(path: 'pathlib.Path', name: 'str') -> 'str':
118+
"""The docstring of class ``name`` in ``path``, read without importing it.
119+
120+
Args:
121+
path: Module to parse.
122+
name: Name of the class whose docstring is wanted.
123+
124+
Returns:
125+
The docstring, exactly as the source spells it.
126+
127+
"""
128+
tree = ast.parse(path.read_text(encoding='utf-8'))
129+
for node in ast.walk(tree):
130+
if isinstance(node, ast.ClassDef) and node.name == name:
131+
doc = ast.get_docstring(node, clean=False)
132+
if doc is None:
133+
raise AssertionError(f'{name} in {path} carries no docstring')
134+
return doc
135+
raise AssertionError(f'{path} defines no class {name}')
136+
137+
138+
def _method_docstring(path: 'pathlib.Path', cls: 'str', name: 'str') -> 'str':
139+
"""The docstring of ``cls.name`` in ``path``, read without importing it.
140+
141+
Args:
142+
path: Module to parse.
143+
cls: Name of the class holding the method.
144+
name: Name of the method whose docstring is wanted.
145+
146+
Returns:
147+
The docstring, exactly as the source spells it.
148+
149+
"""
150+
tree = ast.parse(path.read_text(encoding='utf-8'))
151+
for node in ast.walk(tree):
152+
if not (isinstance(node, ast.ClassDef) and node.name == cls):
153+
continue
154+
for child in node.body:
155+
if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)) and child.name == name:
156+
doc = ast.get_docstring(child, clean=False)
157+
if doc is None:
158+
raise AssertionError(f'{cls}.{name} in {path} carries no docstring')
159+
return doc
160+
raise AssertionError(f'{cls} in {path} defines no method {name}')
161+
raise AssertionError(f'{path} defines no class {cls}')
162+
163+
164+
class MPTCPOptionCitationTests(unittest.TestCase):
165+
"""The class, its members and its template name one RFC: the registry's."""
166+
167+
def setUp(self) -> 'None':
168+
reimport_once_per_class(self)
169+
170+
def test_the_table_cites_no_obsoleted_rfc(self) -> 'None':
171+
# Nothing in the generated table may reach back to the v0 document the
172+
# registry's note keeps only as a pointer. Deliberately not a pin on
173+
# every role equalling the registry's RFC: RFC 8684 section 7.2 admits
174+
# future assignments here under Standards Action, each with its own
175+
# specification reference, so a new subtype may legitimately cite a
176+
# document that does not exist yet.
177+
source = CONST.read_text(encoding='utf-8')
178+
self.assertNotIn(OBSOLETED_RFC, set(RFC_ROLE.findall(source)))
179+
180+
def test_the_class_docstring_cites_the_registrys_rfc(self) -> 'None':
181+
# The class docstring is the table's attribution, so it names the
182+
# registry's own reference and nothing besides -- a second citation
183+
# there would be a second answer to "what defines this table".
184+
doc = _class_docstring(CONST, 'MPTCPOption')
185+
self.assertEqual(set(RFC_ROLE.findall(doc)), {REGISTRY_RFC})
186+
187+
def test_the_template_and_the_module_agree(self) -> 'None':
188+
# Vendor.__init__ reads the crawler class's own docstring as DOCS and
189+
# renders it as "[{NAME}] {DOCS}", so the two are one string in two
190+
# places: correcting either alone lets the generator put the other back.
191+
template = _class_docstring(VENDOR, 'MPTCPOption')
192+
self.assertEqual(_class_docstring(CONST, 'MPTCPOption'),
193+
f'[MPTCPOption] {template}')
194+
195+
def test_the_option_kind_map_cites_the_registrys_rfc(self) -> 'None':
196+
# The map's comment attributes the option kind, not any one subtype's
197+
# wire layout, so it names the document IANA gives for kind 30. The
198+
# v0-specific citations elsewhere in this module are left alone, which
199+
# is why this reads the one entry rather than the whole file.
200+
lines = [line for line in TRANSPORT.read_text(encoding='utf-8').splitlines()
201+
if KIND_ENTRY in line]
202+
self.assertEqual(len(lines), 1, f'{TRANSPORT} must map {KIND_ENTRY} exactly once')
203+
self.assertEqual(RFC_BRACKET.findall(lines[0]), [REGISTRY_RFC])
204+
205+
def test_the_generic_mptcp_figure_cites_the_registrys_rfc(self) -> 'None':
206+
# ``_read_mode_mp`` draws the subtype-agnostic option header, which RFC
207+
# 8684 figure 3 and RFC 6824 figure 3 draw identically -- so the figure
208+
# carries no v0-specific claim and cites the current document.
209+
doc = _method_docstring(TRANSPORT, 'TCP', '_read_mode_mp')
210+
self.assertEqual(set(RFC_ROLE.findall(doc)), {REGISTRY_RFC})
211+
212+
@unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed')
213+
def test_the_imported_enums_docstring_carries_the_citation(self) -> 'None':
214+
from pcapkit.const.tcp.mp_tcp_option import MPTCPOption
215+
216+
self.assertEqual(MPTCPOption.__doc__,
217+
f'[MPTCPOption] Multipath TCP options [:rfc:`{REGISTRY_RFC}`]')
218+
219+
220+
if __name__ == '__main__':
221+
unittest.main()

0 commit comments

Comments
 (0)