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.

… id() (#2524) - HTTP.id() is ('HTTP/1', 'HTTP/2'), members only like IP.id(). - HTTPv1.id() is ('HTTP/1',) and HTTPv2.id() ('HTTP/2',), each narrowed to itself. Annotations, docstrings (RFC 9112/9113) and versionchanged 1.6.0 on all three. - protocol-family-grouping.rst: HTTP moves to the members-only id() form and the narrow-to-self shape, gains a Canonical Names paragraph, and Selector Matching records that a parsed HTTP layer (the base) matches either version's name and no longer equals 'HTTP'. - L2TP, L2TPv2 and VLAN id() docstrings, l2tp.rst and a test_link_unit.py comment stop citing HTTP's old id() as their worked example. - test_http_id_2524_unit.py pins the three id() tuples and in, [], protochain, string == and protocol= on parsed HTTP/1.1 and HTTP/2 layers and on directly built versioned layers; test_http_unit.py's id() pins follow. BREAKING CHANGE: HTTP.id() was ('HTTP', 'HTTPv1', 'HTTPv2'), HTTPv1.id() ('HTTP', 'HTTPv1') and HTTPv2.id() ('HTTP', 'HTTPv2'); they are now ('HTTP/1', 'HTTP/2'), ('HTTP/1',) and ('HTTP/2',). String == compares against the class and its id() only, so == 'HTTP', == 'HTTPv1' and == 'HTTPv2' stop matching on every HTTP layer. in, [] and protocol= keep selecting through the HTTP, HTTPv1 and HTTPv2 registry keys.

…les (#2529) - httpv1.HTTP and httpv2.HTTP become HTTPv1 and HTTPv2, and the top-level HTTP class of the httpv1/httpv2 schema and data modules follows; their sub-structure classes keep their names. id(), alias and the dispatched layer (the base HTTP, data key 'http') are unchanged. - Package __init__ files import the new names directly; imports, mock and registrar targets, autoclass targets, cross-references, ext.rst rows and the Mermaid click anchors follow. - versionremoved 1.6.0 on each old path (module docstring and docs page), versionchanged 1.6.0 on each renamed class; register_protocol's docstring drops the HTTP name collision the rename removes. - Tests: the #1744 ledger takes a removal version and registers the six paths; the collision and symmetry tests build their own class named HTTP; new test_http_member_names_2529_unit.py fails before the rename. BREAKING CHANGE: HTTP in pcapkit.protocols.application.httpv1/httpv2, pcapkit.protocols.schema.application.httpv1/httpv2 and pcapkit.protocols.data.application.httpv1/httpv2 is renamed HTTPv1/HTTPv2 with no alias (#1715, #1744), so `from ...httpv1 import HTTP` raises ImportError. pcapkit.HTTPv1, pcapkit.protocols.HTTPv1 and the schema/data package names are unchanged. A member built directly reports info_name 'httpv1'/'httpv2' (was 'http'), and register_protocol(HTTPv1/HTTPv2) keeps the class under its own key instead of displacing HTTP under 'HTTP'.

…2524) Owner ruling on #2524 (option 2): a parsed HTTP layer narrows to its version. - HTTP._dispatch identifies a payload (connection preface, HTTP/1 start line, plausible HTTP/2 frame header) and picks HTTPv1, HTTPv2 or Raw, as IGMP._dispatch does; Transport resolves the HTTP base its ports bind through it, when parsing and in analyze(). - HTTPv2 reads a leading connection preface itself (info.preface, counted in length) and make() takes preface, so that opening still round-trips. - Docs: the HTTP selector exception goes from protocol-family-grouping.rst, http.rst gains versionchanged 1.6.0, the renamed classes' duplicate versionchanged go (#2530), sslv2.py's HTTP refs retarget to HTTPv2. - Tests: new test_http_dispatch_2524_unit.py; the #2524, #2529 and TCP dispatch tests flip to option 2; key, layer-set and dispatch-probe pins follow (a target defining _dispatch counts as a dispatcher). BREAKING CHANGE: a TCP or UDP HTTP payload is an HTTPv1 or HTTPv2 layer, not HTTP, so its data moves from the "http" key to "httpv1"/"httpv2". The version names and keys ('HTTP/1', 'HTTP/2', HTTPv1, HTTPv2) now select, equal and stop (protocol=) only their own version; 'HTTP', protocol='HTTP' and the HTTP class still match both. A payload neither version identifies is Raw with no error (was 'HTTP: unknown version'). HTTP(...) built directly still parses either version.

…dispatch (#2524) The composite-proto audit in test_const_apptype_split_unit.py reads every _dispatch call in the transport modules as AppType._dispatch(key, proto), so Transport's HTTPBase._dispatch(file_, length) call looked like a lookup passing proto=length. In the transport layer _dispatch is AppType's registry hook (registry-protocol.rst); IGMP's _dispatch is not flagged only because Internet sits outside the audited modules. - HTTP._dispatch becomes HTTP._dispatch_version, after the _dispatch_linktype precedent; its docstring names IGMP._dispatch as its counterpart and says why the name differs. - Callers, docs (http.rst automethod, the conventions page, UDP and TCP notes), the dispatch probe's dispatcher rule and the HTTP tests follow. The audit, unchanged, passes again; tests/const, the HTTP, dispatch, registry and transport modules, tests/project and the const and protocols/application legs pass.

…er too (#2524) - HTTP.id's docstring names _dispatch_version, the method HTTP has; the _dispatch_version docstring drops timed wording. - A connection preface with nothing after it is HTTP/2, but the HTTP/2 data model holds one frame per packet, so HTTPv2 refuses it and it stays Raw, as on main. _dispatch_version now returns Raw for it rather than picking HTTPv2, so the dispatcher and the parse agree; both docstrings say why. - test_http_dispatch_2524_unit.py pins the preface-alone outcome (Raw, byte-exact rebuild) and that every member the dispatcher picks parses its payload; both fail before this change.

) - _dispatch_version's Note drops a timed word, and says a preface alone can be real traffic rather than asserting it is. - test_the_dispatcher_picks_only_a_class_that_parses_the_payload claimed more than its fixed list shows; it becomes test_a_picked_member_parses_these_payloads_or_refuses_those, and the new REFUSED table pins the payloads the dispatcher gives HTTPv2 and HTTPv2 refuses (a preface and 1-8 octets, an empty PRIORITY frame with or without a preface): ProtocolError directly, Raw with the parser's error under TCP.

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.

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 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 mentioned this pull request Nov 1, 2026

refactor(protocols): every versioned family base dispatches on read and make (IGMP, IP, ICMP, L2TP, LinuxSLL) #2574

Open

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

This was referenced Nov 1, 2026

refactor(registry): split foundation.registry.protocols into registry.protocols.<layer> submodules #2578

Open

refactor(registry): every class-level registry map gets its foundation.registry wrapper #2511

Closed

JarryShaw added 8 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

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 wants to merge 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!

This branch has not been deployed

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!

This branch has not been deployed

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