Repository navigation
feat(vendor): add the TLS content-type/handshake-type crawlers (#2495) - #2503
Conversation
|
Cross-review (Sonnet) at
Recommend regenerating both const files from the crawler (option (a) from the review): keep the hand-written prose in the crawler's own docstring/comments (ARPHRD's pattern), drop or justify |
… pairing gap (#2495) - pcapkit/vendor/tls/content_type.py, handshake_type.py: new LINK-and-scrape crawlers against IANA's tls-parameters-5.csv and -7.csv. Both override process()/count() for the registry's 5-column CSV, lower-case the wire name into the member identifier, and for HandshakeType strip IANA's _RESERVED suffix from identifiers while keeping it out of the comment. - Regenerated pcapkit/const/tls/content_type.py, handshake_type.py from the crawlers. Fixes a real behaviour bug: ContentType resolved 32-63 to Unassigned via a flat _missing_ fallback where IANA (and the crawler) call that band Reserved (RFC 9147) -- the committed docstring already said "Reserved", the code did not. Drops ContentType.invalid=0 (no IANA counterpart, unused anywhere); value 0 now resolves into the 0-19 Unassigned band like every other unassigned value there, which SSL's wire-type allowlist already excluded by construction, so this is a behavioural no-op in the protocol layer. HandshakeType gains explicit unassigned_7/unassigned_255 members (IANA's two single-row Unassigned entries) instead of leaving them to _missing_, and several members' RFC citations now match IANA's current RFC 9846 instead of the superseded 5246/6066/4680. - Moved the hand-written protocol-specific prose (record-layer header citations, Unassigned/Reserved band rationale, _RESERVED-suffix handling) from the const modules' docstrings into the crawlers' own docstrings, matching pcapkit.vendor.linux.arphrd's pattern; the generated const modules now carry only the plain templated docstring. - pcapkit/protocols/application/ssl.py: updated two comments that named the now-removed invalid(0) member. - docs/.../const-vendor-pairing.rst: rewrote the TLS section -- running either crawler is now a true no-op against the committed const files, closing the gap the previous revision left open as follow-up work. - tests/const/test_const_tls_vendor_pairing_2495_unit.py: new, pins the Reserved-band fix, the dropped invalid member and the two new unassigned members; all five assertions fail against the pre-fix files. Verified: tests/vendor, tests/const, test_ssl_tls_dtls_unit.py and test_tls_dtls_roundtrip_unit.py all green (651 passed, 26949 subtests). Regenerating both const modules twice in a row produces byte-identical output.
8912f55 to
14be18a
Compare
|
Regenerated both const modules from the new crawlers, closing the pairing gap cross-review found. New head Behaviour fix (not cosmetic): Dropped
Docstrings: the hand-written protocol-specific prose (record-layer header citations, Unassigned/Reserved band rationale, No-op verification: regenerated via New test: Test results: Docs: rewrote the TLS section of |
|
Cross-review (Sonnet) at |
|
Coverage: 94.34% (unit tier, Python 3.15,
Per-file detail: the |
Please follow the guide below
make pylint,make mypy,make isort)make testpasses, and a test case covers the changedocs/source/changelog/and regeneratedCHANGELOG.md, if the change is user-visibleWhat is the purpose of your pull request?
feat— adds a featureDescription of your pull request and other information
Closes #2495.
pcapkit.const.tls.content_type.ContentTypeandpcapkit.const.tls.handshake_type.HandshakeType(#2441) cite a live IANAregistry and claim full coverage, but had no matching
pcapkit/vendor/tls/crawler — the house rule in
docs/source/contributing/conventions/const-vendor-pairing.rstrequires one with no exception. Unlike ARPHRD (#2497) or SSLv2 (#2466), TLS's
source genuinely is a live, web-published IANA table, so this is the normal
LINK-and-scrapeVendorshape, not theDATA-attribute one.What's added:
pcapkit/vendor/tls/content_type.py— crawlstls-parameters-5.csvpcapkit/vendor/tls/handshake_type.py— crawlstls-parameters-7.csv,additionally stripping IANA's
_RESERVEDsuffix from the member identifier(kept out of the comment too)
pcapkit/vendor/tls/__init__.py,pcapkit/vendor/__init__.py— registerTLS_ContentType/TLS_HandshakeTypeBoth registries are a 5-column CSV (
Value, Description, DTLS-OK, Reference, Comment) rather than the 3-column shapeVendor.process()assumes, so eachcrawler overrides
process()/count()accordingly, and renders theCommentcolumn into the generated comment alongside theReferenceRFCs.No-op verification — ran, does not pass byte-identical. I have network
access to IANA in this environment and ran both crawlers for real (not
simulated). They fetch successfully, are deterministic across repeated runs,
and every value IANA actually assigns resolves to the same member name and
the same integer as the committed
constfiles. They are not abyte-identical no-op, for three reasons, all pre-existing in the committed
files from #2441 rather than something this PR introduces:
generated template instead reads "automatically generated from
:class:
..." — the same tradeoffpcapkit.vendor.linux.arphrd(feat(vendor): add the ARPHRD crawler, DATA-attribute shape (#2495) #2497)made, moving the explanatory prose into the crawler's own docstring.
ContentType.invalid = 0has no IANA counterpart — IANA leaves value 0inside its unnamed 0-19
Unassignedband. Nothing elsewhere in thecodebase references
.invalid(checked), so it's unused, but it is agenuine divergence from the live registry.
HandshakeTypevalues 7 and 255 as single-rowUnassignedentries (not ranges), so the crawler renders them as explicit
unassigned_7/unassigned_255members; the committed file insteadleaves every unassigned value to a single flat
_missing_fallback.Per the convention doc's existing nuance for ARPHRD, I left the committed
const/tls/*.pyfiles untouched rather than forcing a match — theirdocstrings don't claim any hand-curated/exempt status (they already
correctly describe themselves as IANA-sourced), so there's nothing to
correct there, and overwriting them to force a literal no-op is a bigger
editorial call (losing the richer per-member RFC commentary) than this PR
makes unilaterally.
docs/.../const-vendor-pairing.rst's "Known Violation"section is rewritten to describe the crawler and this residual gap instead
of removed outright, since the gap is real and worth a maintainer's call.
Testing:
examples/generators/make_samples.py— regenerated cleantests/vendor/,tests/const/— 580 passed, 26581 subtests passed, nocensus-pin changes needed (the new crawlers build
missvia.append(),not a hard-coded list literal, so
tests/vendor/test_vendor_missing_body_unit.py'sEXPECTED_CRAWLERS_WITH_CONSTANT_MISS_LINESpin is unaffected)tests/protocols/application/test_ssl_tls_dtls_unit.py,test_tls_dtls_roundtrip_unit.py— 64 passed, 351 subtests passedtests/project/doc/convention-claim tests — 111 passed, 1317subtests passed
isort,pylint,mypyclean on the new files (pylint's only flag isduplicate-codebetween the two new crawlers' near-identicalprocess()bodies, the same pattern already present between e.g.
pcapkit.vendor.http.error_codeand.frame)