Skip to content

fix(application): rebuild an adopted SSLv2 or DTLS 1.3 record from its to_dict() (#2762) - #2764

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2762-to-dict-adopted-records
Nov 4, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2762-to-dict-adopted-records

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

from_data(info.to_dict()) failed on three kinds of record, because their dispatch keyed on the data model's class and a plain dict carries none:

  • an SSLv2 record adopted by TLS: AttributeError: 'TLS' object has no attribute 'content_type';

  • a DTLS 1.3 unified-header record;

  • a DTLS 1.2 tls12_cid record: invalid connection ID: None.

  • TLS.from_data now also sends a dict that carries two_byte_header to SSLv2.

  • DTLS._make_data keys on the tls12_cid content type, then on sequence_long (the unified header's S bit). This follows the field-based dispatch that IGMPv3._make_data uses.

  • Only method bodies change, and nothing is added or moved, so this is safe alongside Reorder protocol class members to the member-order convention #2490.

  • The new test rebuilds every SSLv2, SSLv3, TLS 1.0–1.3 and DTLS 1.0/1.2/1.3/CID record from info, from to_dict(), and through TCP/UDP. It fails on main with 33 errors and passes here.

  • Not changed: a direct TLS(sslv2_hello) still logs its discarded TLS attempt at CRITICAL level, even though the parse succeeds. That gets its own issue.

Closes #2762

…s to_dict() (#2762)

A to_dict() export carries no data model, so TLS.from_data read an adopted
SSLv2 record's dict as a TLS record, and DTLS._make_data read a unified-header
or tls12_cid record's dict as a legacy one; each rebuild raised.

- TLS.from_data also hands a dict carrying SSLv2's two_byte_header to SSLv2.
- DTLS._make_data dispatches on the fields, as IGMPv3._make_data does: the
  tls12_cid content type, then sequence_long, the unified header's S bit.
- test_ssl_family_to_dict_roundtrip_2762_unit rebuilds each SSLv2, TLS and
  DTLS version from info, from to_dict() and through its TCP or UDP layer.
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Nov 4, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.6 milestone Nov 4, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Coverage: 94.45% (unit tier, Python 3.15, f937c2955, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 106 4 20 2 95.24%
pcapkit/const 20918 175 2908 156 98.57%
pcapkit/corekit 2895 76 930 37 96.84%
pcapkit/dumpkit 286 3 102 3 98.45%
pcapkit/foundation 3507 27 1216 32 98.75%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 22767 155 5918 127 99.00%
pcapkit/toolkit 961 26 342 8 96.62%
pcapkit/utilities 559 7 150 5 98.31%
pcapkit/vendor 5590 2286 1280 214 55.75%

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

@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at f937c2955 (Sonnet cross-review, and my own run of the new test)

  • Routing keys: two_byte_header appears only in the SSLv2 package, and sequence_long only in the DTLS 1.3 unified header. Both are checked at the top level only, so nested handshake dicts can't trigger them.
  • Check order: the tls12_cid check comes first. A CID record with sequence_long injected still rebuilds byte-exact.
  • No regressions: the TLS, SSLv3 and DTLS 1.0/1.2 rebuilds through info and to_dict() match, including over TCP/443 and UDP/5684. The TLS, DTLS and SSLv2 suites and the schema round trips all pass.
  • CI: the new module fails on main with 33 errors. Project tests, Lint and deploy-pages pass. The first deploy-pages attempt never got a macOS runner and was re-run.

@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 4, 2026
@JarryShaw
JarryShaw merged commit 8c30106 into main Nov 4, 2026
74 of 75 checks passed
@JarryShaw
JarryShaw deleted the fix/2762-to-dict-adopted-records branch November 4, 2026 17:06
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 4, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 4, 2026
JarryShaw added a commit that referenced this pull request Nov 4, 2026
- #2766 (Fixed): a cut list of plain values ends with the data, where 1.5.x
  read zero values past the cut (a cut Type 0 Routing header, measured on
  v1.5.1), into the ListField entry; IGMPv3's sources and the Query's
  num_sources, into the IGMP entry.
- #2767: a cut header rebuilt on its own reads back its parsed values, a
  regression of the 1.6.0 rebuild path, into the cut-header Fixed entry; the
  round-trip test rule, into the conventions entry.
- #2764: SSLv2, TLS and DTLS records rebuild from their to_dict(), into the
  TLS/DTLS Added entry, as those classes never shipped.
- #2765: the further member reorders and guard rules, into the member-order
  entry, which records the earlier #2490 reorders.
- 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

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(application): an adopted SSLv2 or DTLS 1.3 record cannot be rebuilt from its to_dict()

1 participant