Skip to content

feat(application): dissect every whole DTLS record in a datagram - #2603

Merged
JarryShaw merged 2 commits into
mainfrom
feat/2535-dtls-chained
Nov 2, 2026
Merged

JarryShaw merged 2 commits into
mainfrom
feat/2535-dtls-chained

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • feat — adds a feature

Description of your pull request and other information

DTLS now dissects every whole record in a UDP datagram, chained the way #2517 does it for TLS. Each record after the first is the previous record's payload.

  • Malformed and cut-short records: a later record that fails to decode becomes Raw with the rest of the datagram and the error set (ruling A). Octets that open no whole record stay Raw with no error.
  • Stops: protocol='DTLS', layer='Application' and FRAME_LAYER_LIMIT stop the chain.
  • DTLS 1.3 unified-header records (RFC 9147 §4) chain by their L length. A header without L runs to the end of the datagram and ends the chain. A header with the C bit set stays Raw when it comes later, since its end cannot be found.
  • Docs: protocol-family-grouping.rst's Chained Records paragraph now covers DTLS.

Settled on the worker's recommendations, open to the owner:

  • a later record whose length runs past the datagram stays Raw silently, as in TLS;
  • the near-identical TLS/DTLS chaining helpers stay where they are rather than moving into SSL.

Follow-up once #2531 lands: the toolkit's _dtls_fragments reads only the first DTLS layer, so a fragmented handshake in a later record is dissected but not reassembled.

Closes #2535

@JarryShaw JarryShaw added feat Pull requests that add a new capability (feat: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 1, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 8dc5f7e95 (Sonnet review). Every new test fails on main, and a mutation check killed 7 of 9 mutants; the two survivors are equivalent. The datagram round trips are byte-exact. The member-order guard reports nothing for DTLS. Chained Records was rewritten in the stacked docs commit. The c51e6c8c7 exception-table commit is a no-op on current main.

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Coverage: 94.03% (unit tier, Python 3.15, 8dc5f7e95, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 20093 144 2712 125 98.78%
pcapkit/corekit 2605 74 818 37 96.52%
pcapkit/dumpkit 281 3 100 3 98.43%
pcapkit/foundation 3360 27 1186 32 98.70%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 20370 220 5382 204 98.34%
pcapkit/toolkit 961 28 342 10 96.32%
pcapkit/utilities 506 7 144 5 98.15%
pcapkit/vendor 5009 2222 1148 182 52.31%

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

@github-actions github-actions Bot added this to the PyPCAPKit v1.6 milestone Nov 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

resolve conflicts

@JarryShaw

Copy link
Copy Markdown
Owner Author

On it. A rebase onto main is already running and keeps both sides: the #2529 pending list for #2602, and Chained Records against #2585's SSLv2 paragraph for #2603. I'll push it with a lease and post the rebase note here, with the old and new heads, the patch-id check and the review status.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Rebased onto main (3ccdc3271), as asked: 8dc5f7e95 → 61ef6cf10.

  • c51e6c8c7 is dropped because its patch is already on main. The branch is now two commits.
  • Application docstring: the PR's DTLS wording and main's SSLv2 wording are merged ("as TLS, SSLv2 and DTLS do for a further record").
  • Chained Records (protocol-family-grouping.rst): main's SSLv2 paragraph and this PR's DTLS paragraph are now one paragraph. Each side's negative is dropped because it is no longer true: "DTLS does not chain" and "SSLv2 does not chain". The examples from both sides are kept.
  • Patch-id outside the two resolved files: unchanged (58f4287e). The application leg passes, and so do refactor(reassembly): bring DTLS reassembly to IP/TCP parity #2605's DTLS reassembly tests.
  • Merging the paragraph rewrote prose, so the verdict goes back to review and a delta review of the resolved hunks is running.
  • feat(application): dissect every HTTP/2 frame in a segment #2609 also edits the Application docstring. Whichever of the two merges second will need one more rebase.

@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 2, 2026
@JarryShaw JarryShaw added the review: running A cross-review is in flight against the current head - no verdict yet label Nov 2, 2026
…2535)

- DTLS._import_next_layer dissects a whole record after the first as a
  further DTLS layer, the previous record's payload, under beholder in
  DTLS._import_next_record; a record that fails to decode becomes Raw with
  everything after it.
- _opens_whole_record checks the header only: legacy content type, 0xFExx
  version and length; a unified header with no connection ID and a length
  that fits, or no length at all, which runs to the end of the datagram.
- protocol='DTLS', layer='Application' and FRAME_LAYER_LIMIT stop the chain.
- dtls.py and dtls.rst gain a "Records in One Datagram" section.
- Tests: two- and three-record datagrams, unified-header chaining, malformed
  later records, the stop conditions, and round trips both ways.
The Chained Records ruling on protocol-family-grouping.rst named DTLS as not
chaining; DTLS records sharing a datagram are now chained layers too.
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 61ef6cf10: a Sonnet delta review of the rebase resolution found no change needed.

  • The dropped c51e6c8c7 is redundant: main's 1bda0c0e2 had already emptied KNOWN_EXCEPTIONS.
  • The Application docstring names exactly the three subclasses that chain a further record.
  • Chained Records was checked against bytes the reviewer built: TCP:TLS 1.3:TLS 1.2, UDP:DTLS 1.2:DTLS 1.3 and TCP:SSLv2:SSLv2 all reproduce. No text left says DTLS or SSLv2 does not chain, and the doc-guard tests pass.
  • refactor(application): move SSLv3 into its own module #2602 and feat(application): dissect every HTTP/2 frame in a segment #2609 have since merged, so a mechanical rebase onto e78967773 is in progress. A rebase note will follow.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Nov 2, 2026
@JarryShaw
JarryShaw force-pushed the feat/2535-dtls-chained branch from 61ef6cf to 1b42427 Compare November 2, 2026 02:59
@JarryShaw

Copy link
Copy Markdown
Owner Author

Rebased onto main (e78967773) after #2602, #2609 and #2612 merged: 61ef6cf10 → 1b424272b.

  • The Application docstring is the union of both sides: "TLS, SSLv2 and DTLS do for a further record and HTTPv2 for a further frame".
  • Chained Records merges without a conflict. feat(application): dissect every HTTP/2 frame in a segment #2609's HTTP/2 paragraph now follows this PR's TLS/SSLv2/DTLS paragraph, unchanged.
  • The patch-id outside application.py and the .rst is unchanged (58f4287e), and the .rst also matches with zero context lines (a56a1b3a). The application leg passes.
  • The resolution is mechanical, so the good-to-go verdict at 61ef6cf10 stands.

@JarryShaw
JarryShaw merged commit f9d5f0d into main Nov 2, 2026
59 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 2, 2026
@JarryShaw
JarryShaw deleted the feat/2535-dtls-chained branch November 2, 2026 03:18
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 2, 2026
JarryShaw added a commit that referenced this pull request Nov 2, 2026
…6.0 (#2449)

- Folded into the TLS/DTLS entry: the hello and EncryptedExtensions
  extension lists, their registered body decoders, register_tls_extension
  and the pcapkit.const.tls extension types (#2595), and each whole DTLS
  record in a datagram as a further layer (#2603).
- The conventions entry records #2611's per-test purge rule; the
  member-order entry cites #2491.
- No entry for #2595's private ProtocolBase value helpers, as for the
  other private API.
- CHANGELOG.md regenerated; util/changelog_md.py --check passes.
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.

feat(application): dissect every DTLS record in a datagram, not only the first

1 participant