refactor(application)!: dispatch HTTP to HTTPv1/HTTPv2 under the RFC names (#2524) by JarryShaw · Pull Request #2544 · JarryShaw/PyPCAPKit

JarryShaw · 2026-11-01T09:23:50Z

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 — N/A, centralised in chore(docs): centralise changelog entries for unreleased versions #2483

What is the purpose of your pull request?

Description of your pull request and other information

Closes #2524, part of #2529. The final state is option 2 (owner on #2524); commits A and B predate that ruling.

HTTP.id() is ('HTTP/1', 'HTTP/2'), while HTTPv1.id() is ('HTTP/1',) and HTTPv2.id() is ('HTTP/2',).
httpv1.HTTP/httpv2.HTTP become HTTPv1/HTTPv2, with schema and data following. The old paths keep no re-export, and versionremoved:: 1.6.0 marks them.
A TCP/UDP HTTP payload is dispatched to HTTPv1 or HTTPv2 as IGMP's is. 'HTTP/1'/'HTTP/2' select only their own version, and the family name HTTP still selects both.
Breaking: the data key moves from http to httpv1/httpv2. == 'HTTP', == 'HTTPv1' and == 'HTTPv2' no longer match, and the old import paths are gone.
An HTTP/2 layer reads a leading connection preface into info.preface, so that a dispatched opening rebuilds byte for byte.
A payload neither version recognises is Raw with no error, where it was 'HTTP: unknown version' (decision 1 below). A payload with a plausible HTTP/2 frame header that HTTPv2 refuses keeps the parser's own error, e.g. HTTP/2: [Type 2] invalid format.
A bare HTTP/2 connection preface, with no frame after it, is Raw with no error, where it carried 'HTTP/2: connection preface with no frame'. The HTTP/2 data model holds a frame per layer, and a preface-only layer is feat(application): dissect every HTTP/2 frame in a segment, not only the first #2536's to add.

JarryShaw · 2026-11-01T09:24:13Z

Decisions needed. The PR follows each recommendation and stays open while you decide.

A port-80 payload neither version recognises. (a) Raw with no error, as IGMP does; (b) Raw with error='HTTP: unknown version', as before; (c) fall back to the base's read, which raises. Recommendation: (a), because mid-stream segments are common and each one used to log a warning.
The base HTTP class. (a) It stays concrete, so HTTP(...) built directly still parses either version under the key http; (b) it becomes abstract, like IGMP's. Recommendation: (a): HTTP.make and from_data are widely used, so (b) is a much larger break.
The dispatch probe. A class that defines _dispatch now counts as a dispatcher, so the TCP/UDP 80/8080 pins still name the base, as IGMP's do. The alternative is to re-pin them to HTTPv1. Recommendation: keep the probe change.

JarryShaw · 2026-11-01T09:39:51Z

Cross-review (Sonnet) at 36e183a4c: GOOD TO GO.

Dispatch: examples/captures/http.pcap yields HTTPv1 on the frame path and the tcp=True path. Other transports are untouched.
Fuzz: across 9,234 TCP/UDP payloads, no payload switches between HTTP and Raw. Parsed layers match main apart from the new preface key. The 5,117 unrecognised payloads lose 'HTTP: unknown version' (decision 1), and 199 that HTTPv2 refuses now carry the parser's own error. The PR body now lists both.
Round trips: byte-exact on 911 payloads both ways. The selector table matches option 2, and old paths raise AttributeError with versionremoved present.
Tests: the re-pins are legitimate. Non-blocking: two new tests also pass before the fix, and "Raw with no error" gets an assertion once decision 1 is answered.

github-actions · 2026-11-01T11:17:43Z

Coverage: 93.95% (unit tier, Python 3.15, 4f8118016, Unit Tests run success)

Package	Statements	Missed	Branches	Partial	Cover
`pcapkit (top level)`	104	4	20	2	95.16%
`pcapkit/const`	20087	144	2710	125	98.78%
`pcapkit/corekit`	2605	74	818	37	96.52%
`pcapkit/dumpkit`	281	3	100	3	98.43%
`pcapkit/foundation`	3194	27	1112	31	98.65%
`pcapkit/interface`	138	4	52	3	96.32%
`pcapkit/protocols`	19939	213	5250	207	98.32%
`pcapkit/toolkit`	924	36	334	10	95.55%
`pcapkit/utilities`	502	7	144	5	98.14%
`pcapkit/vendor`	5009	2222	1148	182	52.31%

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

JarryShaw · 2026-11-01T11:19:07Z

NEEDS CHANGES, from CI rather than the review. Python 3.15 fails tests/const/test_const_apptype_split_unit.py::AppTypeSplitTests::test_no_lookup_call_site_in_the_library_builds_a_composite_proto. Its audit of _dispatch(..., proto) call sites finds a new one in pcapkit/protocols/transport/transport.py, the HTTP._dispatch(data, length) call, and reads it as AppType._dispatch(key, proto=length). The review ran no tests/const, so it missed this. A fix round is dispatched.

JarryShaw · 2026-11-01T12:16:27Z

Pushed 4f8118016, three commits on 36e183a4c. They fix the CI regression and the delta reviews' findings. Cross-review (Sonnet) of each round: GOOD TO GO.

Rename: HTTP's classifier is HTTP._dispatch_version, after _dispatch_linktype. The AppType call-site audit passes again, unedited; it reads _dispatch in the transport files as AppType's. The const and protocols/application legs report 0 failures.
Bare preface: the dispatcher returns Raw for a frameless preface, so the dispatcher and the parse agree. Main also gave Raw, but with an error, so this falls under decision 1. The payloads HTTPv2 refuses after dispatch are now pinned.
Decision 3: the dispatch probe now looks for _dispatch_version.

JarryShaw · 2026-11-01T15:33:16Z

A
A. And this should be a standing convention, applying to ALL other versioned base family protocol classes. So basically, on versioned protocol family, like HTTP/1-2, IGMPv1-3, their base class should be a dispatcher for both read and make path.
Keep the probe change

JarryShaw · 2026-11-01T15:35:30Z

Owner rulings, paraphrased and reconciled:

(a): a payload neither version recognises stays Raw with no error, as built. The fix round adds the assertion that pins it.
(a), widened into a standing convention: on every versioned protocol family, the base class is a dispatcher on both paths. Parsing bytes through the base and building through it (constructor or make) both yield the matching member, never a base-class layer. This covers HTTP/1 and HTTP/2, IGMPv1–3, and the page's other versioned families.
Keep the dispatch-probe change.

This changes the PR: HTTP(...) built directly still parses either version under http. A fix round makes HTTP dispatch to HTTPv1/HTTPv2 on read and make, and writes the convention onto protocol-family-grouping.rst. It is labelled review: needs-changes until that lands. The other families are tracked in a new issue.

Owner rulings on #2544: an unrecognised payload stays Raw with no error, and a versioned family's base dispatches on both paths. - HTTP.__new__ returns the member: on the read path the one an explicit version names or _dispatch_version identifies, on the make path the one version names or _dispatch_fields picks from the fields only one member's make takes (HTTPv1 when there are none). Octets neither version takes raise, as before; mixed fields, preface for HTTP/1 and a bad version raise ProtocolError. - HTTP.from_data rebuilds through the member its data model belongs to. - protocol-family-grouping.rst gains Dispatching Bases (#2574 tracks the other families); http.py and http.rst document the hooks. - Tests: new test_http_base_dispatch_2544_unit.py; the dispatch tests pin Raw with no error and the preface layer's member; two _guess_version tests drive the base's own read on a bare instance. BREAKING CHANGE: HTTP(...) and HTTP.from_data give an HTTPv1 or HTTPv2 instance, never an HTTP one, so the data sits under "httpv1"/"httpv2" rather than "http". HTTP(...) built from HTTP/2 fields without version gives HTTPv2, where it built HTTP/1; fields of both versions raise.

…rst (#2544) - HTTP.from_data, a public method, moves into the Methods group ahead of __new__; the helpers only __new__ reaches (_dispatch_version, _dispatch_fields, _member_of_version) go last, after read()'s own. - protocol-family-grouping.rst drops httpv1.HTTP and httpv2.HTTP from the #2529 pending list, now renamed; protocol-member-order.rst's _decode_text() example names HTTPv1.

JarryShaw · 2026-11-01T16:27:59Z

Pushed 519d9eb53: the owner's rulings applied, rebased onto main, conflicts resolved.

Ruling 2: HTTP dispatches on read, make and from_data (HTTP.__new__). HTTP(octets), HTTP(fields) and HTTP.from_data(...) give HTTPv1/HTTPv2. Fields from both versions raise. protocol-family-grouping.rst gains "Dispatching Bases", citing refactor(protocols): every versioned family base dispatches on read and make (IGMP, IP, ICMP, L2TP, LinuxSLL) #2574 for the other families.
Ruling 1: an assertion now pins it, and the two tests that passed before the fix now fail there.
Rebase: the refactor!: remove every surviving re-export of a moved name or module #1744 ledger keeps every entry from main plus DRARP's and HTTP's. refactor(internet): name IGMP member classes after their modules (#2529) #2542's pending-rename list drops httpv1.HTTP and httpv2.HTTP, which this PR completes. feat(registry): foundation.registry wrappers for IPX and SCTP sub-maps; record the convention (#2511) #2521's registry code is kept. HTTP's members now satisfy main's member-order guard, with no exception added.
Tests: the const, protocols/application and both project legs pass, apart from main's known red (the guard table and radiotap's wrapper), which is identical on main.
Review: a Sonnet review of the dispatcher round and the rebase is running.

JarryShaw · 2026-11-01T16:45:54Z

NEEDS CHANGES at 519d9eb53 (Sonnet review). The dispatcher works on read, make and from_data, the rebase is faithful, and decision 1 holds on the transport path. Four fixes are needed:

In test_http_unit, base_read sits between skipUnless and HTTPUnitTests, so the class has lost its skip guard.
_dispatch_fields reads a falsy preface as an HTTP/2 field. HTTP(method='GET', uri='/', preface=False) built HTTP/1 at the merge base and now raises.
Protocol Family Grouping, "Dispatching Bases", does not state decision 1: transport gives Raw with no error, and direct HTTP(...) raises.
Rendered text cites :pr:2544``; it should cite the issue.
The fix round is with the original worker.

- HTTP._dispatch_fields counts preface only when true, as its version check does, so HTTP(method=..., uri=..., preface=False) builds HTTP/1 again and HTTP(preface=False) picks HTTPv1; a new test pins both. A true preface beside HTTP/1 fields alone keeps the message "HTTP/1: connection preface is HTTP/2 only". - test_http_unit.py: base_read moves above the skip decorator, which guards HTTPUnitTests again. - protocol-family-grouping.rst's Dispatching Bases records that octets neither version takes are Raw with no error on the transport path, and raise ProtocolError through a direct HTTP(...), while octets HTTPv2's parser refuses after dispatch are Raw with the parser's error. - Rendered text cites #2524 rather than the pull request.

JarryShaw · 2026-11-01T17:21:20Z

The fix round 363a6e938 is ready, one commit on 519d9eb53:

a falsy preface names no version;
base_read sits above the skip guard;
"Dispatching Bases" states decision 1 and the HTTPv2-refused case;
rendered text cites :issue:2524``.

Its local delta review (Sonnet) is GOOD TO GO. The push is held until the Actions queue is below 40 (it is at 68). The owner has ruled in session that pushes to open PRs go at < 40 and new PRs at < 20, so this goes out first.

JarryShaw · 2026-11-01T17:50:51Z

I cancelled every queued and pending run on 519d9eb53 (Unit Tests, Experimental, Python Compatibility and Project Status). The owner has ruled that a PR whose verdict is needs-changes gets all its CI cancelled, to free runners and drain the queue. CI runs again when the fix round 363a6e938 is pushed.

JarryShaw · 2026-11-01T17:58:48Z

GOOD TO GO at 363a6e938. A Sonnet delta review covered f871b74c3, and I checked the nit fold on top of it myself.

All four fixes are in: the skip guard is restored; a falsy preface names no version (the test fails before the fix and passes after); Dispatching Bases states decision 1 and the HTTPv2-refused case; rendered text cites :issue:2524``.
HTTP(method=..., preface=True) again reports that the preface is HTTP/2 only.
The application leg (693 tests) and const (651) pass. The guard and wrapper-coverage modules pass on a merge with main.
__keywords__ = None is read only by a test and the docs, so it is left for refactor(protocols): every versioned family base dispatches on read and make (IGMP, IP, ICMP, L2TP, LinuxSLL) #2574.

…ons into 1.6.0 (#2449) - Breaking: HTTP dispatches to HTTPv1/HTTPv2 on read, make and from_data, and so does a transport payload, whose data key moves from http to httpv1/httpv2; the members and their schema and data classes are renamed from httpv1.HTTP/httpv2.HTTP with no re-export; id() takes the RFC names; unrecognised transport payloads are Raw with no error (#2544). - Changed: conventions record the dispatching base (#2544) and the case-fold exceptions (#2583); a project test guards every shipped module's documentation entry (#2584). Summary names the HTTP change. - CHANGELOG.md regenerated; util/changelog_md.py --check passes.

…s into 1.6.0 (#2449) - Folded into the TLS/DTLS entry: DTLSContext resolves a unified-header record's connection ID as a tls12_cid one's, an ID NewConnectionId issues coming only from the caller; every unified-header record's info carries cid, None when C is clear, the one-data-model ruling on #2625; make() takes cid (#2643). The entry drops the stale "stays Raw" for those records. - 1.5.0's httpv1.HTTP and httpv2.HTTP roles point at HTTPv1 and HTTPv2, renamed by #2544, so the nitpicky build no longer warns on them; 1.5.0's text unchanged. - No entry for the test-only #2644. - CHANGELOG.md regenerated; util/changelog_md.py --check passes.

…les (#2628) (#2651) - `HTTP.__new__` and `HTTP.make`: building from fields takes `version` and raises `ProtocolError` without it. `HTTP._dispatch_fields` and its `_make_fields` helper go, so no field picks a member and HTTPv1 is no default. Octet dispatch through `HTTP._dispatch_version` is kept. - `HTTP.from_data` dispatches by data-model class and refuses a dict, rather than picking by its `receipt` or `sid` key. - `HTTP.__init__`: from fields, `version` must be the member's own, and HTTPv1 refuses `preface`, which `HTTP.pack` would write ahead of an HTTP/1 message. `HTTP._member_of_version` refuses a bool or a float. - Docs: Dispatching Bases lists HTTP; the HTTP page records the change. - Tests: `test_http_base_dispatch_2628_unit.py` pins each refusal; the #2544, #2298, HTTP/2 round-trip and #1447 tests pass `version` or rebuild a dict through its member. Breaking against v1.5.1: there `HTTP(...)` from fields built HTTP/1.* and `HTTP.from_data` took a dict, and each raises here.

JarryShaw added breaking Breaks public-facing behaviour or API (apply alongside the type label) refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Nov 1, 2026

JarryShaw added the needs: decision Waiting on the maintainer to decide — not blocked by other work label Nov 1, 2026

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 1, 2026

JarryShaw added this to PyPCAPKit Nov 1, 2026

JarryShaw moved this to Needs decision in PyPCAPKit Nov 1, 2026

JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 1, 2026

JarryShaw added 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 labels Nov 1, 2026

JarryShaw removed the needs: decision Waiting on the maintainer to decide — not blocked by other work label Nov 1, 2026

JarryShaw added 2 commits November 1, 2026 11:09

JarryShaw force-pushed the refactor/2524-http-rfc-names branch from 4f81180 to 519d9eb Compare November 1, 2026 16:28

JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Nov 1, 2026

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 Nov 1, 2026

JarryShaw added 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 labels Nov 1, 2026

JarryShaw merged commit 3e076e6 into main Nov 1, 2026
61 checks passed

github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 1, 2026

JarryShaw deleted the refactor/2524-http-rfc-names branch November 1, 2026 18:52

JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 1, 2026

This was referenced Nov 1, 2026

feat(application): dissect every HTTP/2 frame in a segment, not only the first #2536

Closed

refactor(corekit): move the Internet checksum into corekit.checksum.internet #2586

Merged

This was referenced Nov 1, 2026

chore(docs): centralise changelog entries for unreleased versions #2483

Open

feat(transport): dissect DCCP, with its checksum over the IP pseudo-header #2593

Merged

feat(application): dissect every HTTP/2 frame in a segment #2609

Merged

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

This was referenced Nov 2, 2026

refactor(protocols): a protocol class takes its module's name (igmpv1.IGMPv1, httpv1.HTTPv1) #2529

Closed

refactor(application): the HTTP base follows #2574's version= rule, with no default member #2628

Closed

Navigation Menu

Repository navigation

refactor(application)!: dispatch HTTP to HTTPv1/HTTPv2 under the RFC names (#2524) - #2544

refactor(application)!: dispatch HTTP to HTTPv1/HTTPv2 under the RFC names (#2524)#2544
JarryShaw merged 9 commits into
mainfrom
refactor/2524-http-rfc-names

JarryShaw commented Nov 1, 2026 •

edited

Loading

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

github-actions Bot commented Nov 1, 2026 •

edited

Loading

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

JarryShaw commented Nov 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of your pull request?

Description of your pull request and other information

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

github-actions Bot commented Nov 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

JarryShaw commented Nov 1, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

JarryShaw commented Nov 1, 2026 •

edited

Loading

github-actions Bot commented Nov 1, 2026 •

edited

Loading