Skip to content

feat(vendor): add the TLS content-type/handshake-type crawlers (#2495) - #2503

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2495-tls-vendor-crawler
Nov 1, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2495-tls-vendor-crawler

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort)
  • make test passes, and a test case covers the change
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible

What is the purpose of your pull request?

  • feat — adds a feature

Description of your pull request and other information

Closes #2495.

pcapkit.const.tls.content_type.ContentType and
pcapkit.const.tls.handshake_type.HandshakeType (#2441) cite a live IANA
registry and claim full coverage, but had no matching pcapkit/vendor/tls/
crawler — the house rule in docs/source/contributing/conventions/const-vendor-pairing.rst
requires 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-scrape Vendor shape, not the DATA-attribute one.

What's added:

  • pcapkit/vendor/tls/content_type.py — crawls tls-parameters-5.csv
  • pcapkit/vendor/tls/handshake_type.py — crawls tls-parameters-7.csv,
    additionally stripping IANA's _RESERVED suffix from the member identifier
    (kept out of the comment too)
  • pcapkit/vendor/tls/__init__.py, pcapkit/vendor/__init__.py — register
    TLS_ContentType / TLS_HandshakeType

Both registries are a 5-column CSV (Value, Description, DTLS-OK, Reference, Comment) rather than the 3-column shape Vendor.process() assumes, so each
crawler overrides process()/count() accordingly, and renders the
Comment column into the generated comment alongside the Reference RFCs.

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 const files. They are not a
byte-identical no-op, for three reasons, all pre-existing in the committed
files from #2441 rather than something this PR introduces:

  1. The committed files carry a hand-written narrative docstring; the
    generated template instead reads "automatically generated from
    :class:..." — the same tradeoff pcapkit.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.
  2. ContentType.invalid = 0 has no IANA counterpart — IANA leaves value 0
    inside its unnamed 0-19 Unassigned band. Nothing elsewhere in the
    codebase references .invalid (checked), so it's unused, but it is a
    genuine divergence from the live registry.
  3. IANA lists HandshakeType values 7 and 255 as single-row Unassigned
    entries (not ranges), so the crawler renders them as explicit
    unassigned_7 / unassigned_255 members; the committed file instead
    leaves every unassigned value to a single flat _missing_ fallback.

Per the convention doc's existing nuance for ARPHRD, I left the committed
const/tls/*.py files untouched rather than forcing a match — their
docstrings 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 clean
  • tests/vendor/, tests/const/ — 580 passed, 26581 subtests passed, no
    census-pin changes needed (the new crawlers build miss via .append(),
    not a hard-coded list literal, so tests/vendor/test_vendor_missing_body_unit.py's
    EXPECTED_CRAWLERS_WITH_CONSTANT_MISS_LINES pin is unaffected)
  • tests/protocols/application/test_ssl_tls_dtls_unit.py,
    test_tls_dtls_roundtrip_unit.py — 64 passed, 351 subtests passed
  • Targeted tests/project/ doc/convention-claim tests — 111 passed, 1317
    subtests passed
  • isort, pylint, mypy clean on the new files (pylint's only flag is
    duplicate-code between the two new crawlers' near-identical process()
    bodies, the same pattern already present between e.g.
    pcapkit.vendor.http.error_code and .frame)

@JarryShaw JarryShaw added feat Pull requests that add a new capability (feat: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 31, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 31, 2026
@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 31, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review (Sonnet) at 8912f553b: NEEDS CHANGES. Crawler code (process()/count() against the real 5-column CSV) is correct and no-op claim reproduced independently, but the 'three discrepancies, mostly prose' framing in both the PR and const-vendor-pairing.rst understates real drift:

  1. The committed ContentType's 32-63 band resolves to Unassigned via its flat _missing_ fallback, but IANA (and the crawler) correctly call it Reserved (RFC 9147) -- a behavior difference, not just prose, and the committed docstring even SAYS 'Reserved' while the code returns 'Unassigned'.
  2. RFC citations differ for 7+ HandshakeType members (committed cites 5246/6066/4680, IANA/crawler cites 9846) -- not 'richer prose', a different citation set.

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 ContentType.invalid = 0 (confirmed unused anywhere), and accept the crawler's unassigned_7/unassigned_255/Reserved output as correct. That's the only option that doesn't leave this as a weaker precedent than ARPHRD/sslv2's byte-identical no-ops. Dispatching that fix now.

… 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.
@JarryShaw
JarryShaw force-pushed the fix/2495-tls-vendor-crawler branch from 8912f55 to 14be18a Compare October 31, 2026 20:55
@JarryShaw

Copy link
Copy Markdown
Owner Author

Regenerated both const modules from the new crawlers, closing the pairing gap cross-review found. New head 14be18aa8.

Behaviour fix (not cosmetic): ContentType resolved values 32-63 to Unassigned via a flat _missing_ fallback; IANA's registry -- and the crawler -- call that band Reserved (RFC 9147). The committed docstring already said "Reserved", the code did not. Regenerating fixes the resolved name.

Dropped ContentType.invalid = 0: no IANA counterpart, and grep -rn "ContentType.invalid" found no reference anywhere in the codebase. Value 0 now resolves into the 0-19 Unassigned band like every other unassigned value there. Checked the one place that could have depended on it by name -- SSL._validate_tls_record's _WIRE_CONTENT_TYPES allowlist -- and it never referenced .invalid; it excludes value 0 by construction (the allowlist names only the 7 genuine wire types), so this is a behavioural no-op in the protocol layer. Updated two stale comments in ssl.py that named the now-removed member.

HandshakeType citation and member-set changes: several members now cite RFC 9846 (IANA's current reference) instead of the superseded 5246/6066/4680, and the registry's two single-row Unassigned entries (values 7 and 255) are now explicit members (unassigned_7, unassigned_255) instead of falling through to _missing_.

Docstrings: the hand-written protocol-specific prose (record-layer header citations, Unassigned/Reserved band rationale, _RESERVED-suffix handling) moved from the const modules into the crawlers' own module docstrings, matching pcapkit.vendor.linux.arphrd's pattern from #2497. The generated const modules now carry only the plain templated docstring.

No-op verification: regenerated via python -m pcapkit.vendor tls (importing the submodule normally through pcapkit/vendor/__main__.py, not python -m pcapkit.vendor.tls.content_type directly, which would set __module__ to __main__ and reproduce the cosmetic .. module:: __main__ artifact). Ran the regeneration twice in a row and diffed the SHA-256 of both const files -- byte-identical, confirming it's now deterministic and a true no-op against what's committed.

New test: tests/const/test_const_tls_vendor_pairing_2495_unit.py pins the Reserved-band fix, the dropped invalid member, and the two new unassigned_* members. Verified all five of its assertions fail against the pre-fix const files (stashed them, ran the new test, got 5 failures, restored).

Test results: tests/vendor/, tests/const/, tests/protocols/application/test_ssl_tls_dtls_unit.py, tests/protocols/application/test_tls_dtls_roundtrip_unit.py -- 651 passed, 26949 subtests passed, 0 failures.

Docs: rewrote the TLS section of docs/source/contributing/conventions/const-vendor-pairing.rst -- running either crawler is now a true no-op against the committed const files, same guarantee as ARPHRD (#2497) and the sslv2 restack, replacing the earlier "documented residual gap" framing.

@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment review: running A cross-review is in flight against the current head - no verdict yet labels Oct 31, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review (Sonnet) at 14be18aa8: GOOD TO GO. No-op independently reproduced by tampering both const files and re-running the crawler -- tamper overwritten, tree clean. ContentType(32..63) confirmed resolving to Reserved against live IANA data (RFC 9147), a real behavior fix. ContentType.invalid drop confirmed safe (_WIRE_CONTENT_TYPES unchanged, 0 still rejected via Unassigned instead). Full suite clean: 651 tests/26949 subtests on the targeted set, 2050/10340 on tests/project (2 unrelated PYTHONSAFEPATH artifacts, confirmed passing without that flag; 3 files excluded as this session's known host-contention flakes). const-vendor-pairing.rst correctly describes a closed, true no-op pairing now. Ready for merge -- your call.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 94.34% (unit tier, Python 3.15, 14be18aa8, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19713 135 2644 122 98.83%
pcapkit/corekit 2605 74 818 37 96.52%
pcapkit/dumpkit 281 3 100 3 98.43%
pcapkit/foundation 3070 24 1072 26 98.79%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 18303 195 4818 181 98.37%
pcapkit/toolkit 924 36 334 10 95.55%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4587 1944 1052 170 54.35%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw
JarryShaw merged commit 081fc07 into main Nov 1, 2026
65 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 1, 2026
@JarryShaw
JarryShaw deleted the fix/2495-tls-vendor-crawler branch November 1, 2026 00:02
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat Pull requests that add a new capability (feat: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Write a pcapkit/vendor/tls crawler for ContentType and HandshakeType

1 participant