fix(protocols): let Application carry an undissected remainder (#719)

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort) — measured per file: mypy reports 0 errors in application.py on this change and on main alike. An earlier revision of this branch introduced 5 there by dropping a load-bearing # type: ignore[override]; that is fixed. mypy is not a clean baseline on main either way — 305 errors in 33 other files on this invocation, none on a line this change touches.
• A test case covers the change — tests/protocols/application/test_application_dispatch_unit.py, new here. make test itself was not run: the full suite reaches ~56 GB on this host and gets OOM-killed, so the selections below were run instead.
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md

What is the purpose of your pull request?

• fix — corrects a defect
• feat — adds a feature
• perf — changes performance, not behaviour
• refactor — changes neither behaviour nor performance
• test — tests only
• docs — documentation only
• ci — workflows or build tooling
• chore — anything else

---

Description of your pull request and other information

Implements the ruling on #719: loosen Application itself rather than work around it per class. Application forbade payload dispatch outright, which is stricter than the invariant it means to encode — "no further protocol layer above this one". Undissected trailer bytes are not a protocol layer, so refusing them was refusing something the invariant permits.

• _decode_next_layer and _import_next_layer raised UnsupportedCall unconditionally. They now accept the -1 sentinel — the rest of the packet, undissected — and delegate to ProtocolBase, which resolves it to Raw, or to NoPayload when nothing remains. Any other proto is still refused, and the message now names the protocol number instead of claiming the attribute does not exist.
• __post_init__ unconditionally overwrote _next with NoPayload() and rebuilt _protos after read(), discarding whatever a dispatch had just set. It now fills them only when read left _next unset.
• The class docstring said "transport layer protocol family" and said nothing about the dispatch contract. Both corrected.

Why it matters beyond these two methods: this is what blocked a functionally application-layer protocol that still carries a trailer from naming Application as its base at all. The OSPF/RARP layer move rides on top of this and is a separate pull request.

Breaking in two ways a third party can observe. A subclass calling either method with -1 changes from raising to succeeding; and a subclass whose read assigns _next itself used to have it overwritten with NoPayload after read() returned, and now keeps it. Neither needs a code change by the caller to observe, which is the convention page's test. No subclass in the package does either — the five are FTP, HTTP, HTTPv1, HTTPv2 and NGAP, enumerated by walking Application.__subclasses__() recursively, and none overrides the two dispatch methods or __post_init__ (FTP_DATA subclasses Raw, not Application) — so nothing in the tree changes behaviour.

Verified on 0e6e5f3cf: tests/protocols/application + tests/project give 399 passed, 1 skipped, 1305 subtests. Three of the new test's five cases fail when application.py is reverted to main — the two sentinel dispatches and the _import_next_layer acceptance — and the other two are regression guards on what must not change: a real protocol number still refused, and an application protocol that never dispatches still ending in NoPayload.

[JarryShaw]

Cross-review verdict on 55dc55b4b: NEEDS CHANGES (ran on Sonnet; I authored the branch). A real code defect, not prose — fixed in 0e6e5f3cf, now pushed.

The defect: I deleted a # type: ignore[override] that was doing real work. The diff moved the ignore from _import_next_layer onto _decode_next_layer, and the reviewer measured five new mypy errors in application.py as a result. I reproduced it before acting:

tree	errors in pcapkit/protocols/application/application.py
main	0
55dc55b4b	5 — :109 unused-ignore, :133 [override], and three at :155 ([misc], two [call-arg])
0e6e5f3cf	0

The mechanism is worth recording: ProtocolBase._import_next_layer is wrapped in @beholder, which mypy sees as a no-argument method, so both the [override] on the signature and [call-arg,misc] on the super() call are load-bearing — protocol.py:598 carries the same pair for the same reason. _decode_next_layer genuinely needs none, because widening proto to Optional[int] is LSP-legal, so that ignore really was unused. Three edits: drop the unused one, restore the override ignore, add [call-arg,misc] to the super() call.

And the PR body's mypy sentence was false as written — I claimed none of mypy's errors sit on a line this change touches. Corrected.

On the breaking label, which the review called defensible but weakly justified, I am keeping it and strengthening the reason. It found a second observable I had missed and which is better than the one I gave: a third-party subclass whose read assigns _next itself used to have it overwritten with NoPayload after read() returned, and now keeps it. That is a behaviour change requiring no code change by the caller, which is exactly the convention page's test. Both the body and the commit message now name it.

What it confirmed independently, re-deriving rather than accepting: the sentinel accepted and every other proto refused with UnsupportedCall naming the number; -1 with bytes giving Raw and without giving NoPayload; and — the claim I most wanted attacked — the subclass enumeration, walking Application.__subclasses__() recursively to exactly five (FTP, HTTP, HTTPv1, HTTPv2, NGAP), none overriding either dispatch method or __post_init__, with FTP_DATA subclassing Raw rather than Application. It also mutation-tested the new test four ways, which is stronger than the pass/fail split I reported: dropping the proto check in _decode_next_layer gives 8 failures, in _import_next_layer 4, and inverting the __post_init__ guard either way gives 1 each.

One correction it raised against my own prose on #719, which belongs to the move rather than here: my 00:11Z comment said Link and Application define "precisely the same three names beyond ProtocolBase". They do not — Link adds seven and Application eight, so OSPF would also lose Link.__proto__, register and _read_protos on moving. It checked the behaviour is unaffected (a -1 lookup misses in either registry and falls back to Raw, and OSPF is reached by IP protocol rather than EtherType), but the wording is wrong and I will correct it on the move's pull request rather than leave it standing.

Resetting review: since the head moved. tests/protocols/application + tests/project on the fix: 399 passed, 1 skipped, 1305 subtests.

[JarryShaw]

Cross-review verdict on 0e6e5f3cf: GOOD TO GO (ran on Sonnet; I authored the branch).

The mypy regression is fixed and re-measured independently: no line in application.py, ending Found 305 errors in 33 files — matching the main baseline of 0 in this file and the same 305 elsewhere. The 5 errors at 55dc55b4b are gone.

The diff scope held, which is what let the rest of the first pass carry forward untouched. git diff 55dc55b4b 0e6e5f3cf is one file, 3 insertions and 3 deletions, all three # type: ignore comments:

-    def _decode_next_layer(..., *,  # type: ignore[override]      <- unused, removed
+    def _import_next_layer(..., *,  # type: ignore[override]      <- restored
+        return super()._import_next_layer(-1, length, packet=packet)  # type: ignore[call-arg,misc]

It also agreed with my reading of the removed one: widening proto to Optional[int] is LSP-legal, so there is no [override] to suppress on _decode_next_layer, and mypy no longer reports it as unused.

One cosmetic staleness it caught, now fixed: the body's "Verified on" line still named 55dc55b4b. The figures hold, since the only change since is comment-only, but a stale sha in a description is exactly the drift I have let through three times before — it now reads 0e6e5f3cf.

Swapping to review: good-to-go. The one red check will be Plain unittest ordering (protocols), which is red on main too and not required — tracked as #1029, not this change's doing.