Skip to content

fix(const): answer IANA's transport-less port spans with the registry's transport (#2048) - #2052

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2048-apptype-unassigned-fallback
Oct 23, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2048-apptype-unassigned-fallback

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner
  • 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

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #2048. The IANA port spans that name no transport (unassigned and reserved ranges, and the few named services like x11) now answer with the asking registry's own transport, as cls.__transport__, instead of TransportProtocol.undefined. So every AppType answer's .proto is the registry it came from.

  • Unchanged, per the owner's ruling on this PR: a port that is in no span on a registry still falls back to unknown [<transport>]. That covers get() and PortEnumField alike, so for example 8011 answers unknown [dccp] / unknown [sctp]. numbers.py is untouched.
  • Spans that name a transport: unchanged (vendor(apptype): duplicated _missing_ range guards mint members with the wrong transport #760).
  • Measured on all four registries: every one of 0–65535 answers with the registry's own transport. The unknown answers are exactly the ports with no entry and no span.
  • Tests: test_const_apptype_unassigned_2048_unit, the template/module agreement test and the split invariants. On main, the span cases fail (the 8012 and 226 ranges on all four registries).
  • Results: the project leg ran 1945 tests with 0 failures, and the AppType, PortEnumField, dumpkit and changelog readers all pass.

@JarryShaw JarryShaw added bug Issues reporting a defect (set by the bug report template; a default, not an assessment) const Regenerated IANA or vendor constant tables; members keep their numeric values review: running A cross-review is in flight against the current head - no verdict yet labels Oct 22, 2026
@JarryShaw
JarryShaw force-pushed the fix/2048-apptype-unassigned-fallback branch from 1da668b to c8a587b Compare October 22, 2026 20:27
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 22, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Sonnet cross-review at c8a587b9e: GOOD TO GO, with one regression being fixed before ready.

  • Confirmed:
    • DCCP and SCTP get(8011) return unassigned [undefined]; TCP and UDP are unchanged.
    • AppType.get(80, proto=4) returns DCCP.unassigned, and out-of-range ports raise.
    • A sweep of every 7th port on all four registries minted nothing.
    • A get member and a parsed member are equal and hash equal.
    • The new tests fail on main (7 failed). The PR passes tests/const plus vendor (368), and tests/corekit, tests/dumpkit and tests/protocols/transport (988).
  • Regression, reproduced: register_apptype(TCP.get(54321), H) with no explicit transport now raises RegistryError: unknown transport protocol: undefined, because the transport defaults to code.proto. Span members such as TCP.get(8012) already raised on main.
    • Fix in progress: register_apptype takes an undefined member's transport from its own registry, with tests for both kinds.
  • Note: the dynamic range 49152–65535 has no IANA rows (the CSV ends at 49151), so it now reads unassigned where it read unknown. IANA calls the range dynamic/private. That follows the ruling, so I'm noting it here rather than blocking on it.

@JarryShaw
JarryShaw force-pushed the fix/2048-apptype-unassigned-fallback branch from c8a587b to 30b14e7 Compare October 22, 2026 20:47
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 30b14e79d (was c8a587b9e), which fixes the regression the review found. With no transport given, register_apptype now takes the transport of an undefined member from that member's own registry, so TCP.get(54321) and TCP.get(8012) register onto TCP. A base AppType member with no single transport fails with a clear message. An explicit transport still overrides.

The new tests/foundation/registry/test_register_apptype_unassigned_2048_unit.py mocks the registries, so nothing global changes. It fails on the old protocols.py (10 failed) and passes here (112 passed, with the related tests). The changelog entry gains one sentence, and CHANGELOG.md is regenerated. A delta re-review is in progress.

@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 30b14e79d (Sonnet delta re-review). The reviewer used a subprocess probe on real registries:

  • Members with an undefined proto register onto their own registry: register_apptype(TCP.get(54321), HTTP) lands in TCP's registry, and UDP's stays empty. TCP.get(8012) and UDP.get(54322) also land on their own registries.
  • An explicit 'udp' still overrides the default.
  • SCTP and DCCP members raise unknown transport protocol: sctp/dccp, the same as the explicit path on main.

The test's mocks assert that the other registry was not called, so a registration onto the wrong registry would fail it. tests/foundation/registry, tests/const and test_changelog_md: 481 passed.

@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 Oct 22, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Ordering check on 30b14e79d: the project leg (util/run_unittest_leg.py project) ran 1841 tests, OK (71 skipped). CI: 60 checks green, 0 failing. KTLO 3.8, 3.9 and 3.10 legs: 0 failing tests. Ready to merge.

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 91.54% (unit tier, Python 3.15, 990d037a9, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19599 880 2618 866 92.12%
pcapkit/corekit 2565 71 812 37 96.56%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 2930 23 1022 25 98.79%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17414 219 4574 185 98.14%
pcapkit/toolkit 869 36 298 10 95.20%
pcapkit/utilities 473 7 136 5 98.03%
pcapkit/vendor 4432 1988 1018 167 51.87%

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

@JarryShaw

Copy link
Copy Markdown
Owner Author

no, when owner._missing_(key) returns None, it should keep unknown with owner.__transport__ protocol.

for those ports that are partially assigned, each protocol should then return the ranged response with the corresponding protocol. so basically, i think, in _missing_, unless a port is assigned in all protocols (TCP/UDP/SCTP/DCCP), it should remain in the branching keep its original missing logic as a proper fallback.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Understood. This PR goes back for rework. My plan, to confirm or correct:

  1. Revert the fallback. When owner._missing_(key) returns None, get answers unknown with owner.__transport__, as on main. This applies in AppType.get and in PortEnumField parsing, and the tests that pinned unassigned move back to unknown.
  2. Keep partially assigned ports in their original span, per registry. IANA's CSV no longer holds the original span: it now lists 8012–8014 as unassigned and gives 8011 rows only for TCP and UDP. So the generator has to get the history from the table it is about to replace.
    • Carrying the span forward: when a regeneration splits an all-transport span because a port gained rows for only some transports, the transports with no row keep the old span. For 8011, 8011 <= value <= 8014 stays live for DCCP and SCTP.
    • Transport in the answer: a span kept this way applies to that registry only, so it answers unassigned with that registry's transport: DCCP.get(8011) → unassigned [dccp]. Spans that still cover every transport keep undefined.
    • Coverage: ports assigned before our tables tracked them never sat in such a span, so they stay unknown [dccp]. 8009 and 8015 are examples.
    • Only all-transport assignments shrink a span: once a port has a row for all four transports, it leaves the span everywhere.
  3. register_apptype: with unknown members carrying their transport again, the regression the review found goes away. I'd keep the part where a span member whose transport is undefined registers onto its own registry, because main already raises on, for example, TCP.get(8012). Say if you'd rather drop it.

Needs you: is (2) what you mean, including the registry's own transport in the answer? Or should a kept span answer undefined, like the shared spans do?

@JarryShaw 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 Oct 22, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

i think it's (2).

for example, originally 8000-8100 are all reserved. now TCP and UDP assigned port 8000. we would like to keep 8000 still reserved for DCCP and STCP.

also, just noticed one issue, in AppType._missing_, the return cls._unregistered_member(value, 'unassigned', TransportProtocol.undefined) call should've been returning the actual transport protocol. If being called from TCP, then TransportProtocol.tcp, etc.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Recorded, two parts:

  1. Carry the span forward per registry. When IANA assigns a port in a span to only some transports, the others keep the span's label. In your example, 8000 stays reserved for DCCP and SCTP after TCP and UDP take it.
  2. Spans answer with the asking registry's transport. AppType._missing_'s spans return cls.__transport__, not TransportProtocol.undefined: TCP.get(8012) → unassigned [tcp], DCCP.get(8011) → unassigned [dccp].

The fallback for a port in no span stays unknown [<registry transport>], as on main. Since every member then carries a real transport, I'll drop the register_apptype change unless something still reaches it with undefined. The rework is in progress on this PR.

@JarryShaw

JarryShaw commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner Author

i suppose when 8000-8100 are reserved, DCCP and SCTP would still get the entires from IANA.

It's only that those are genuine unassigned, then they are omitted from the entries. In that case, yes, we'd still prefer to have it properly return an unassigned [dccp] or ... [sctp].

@JarryShaw

Copy link
Copy Markdown
Owner Author

Recorded: the carry-forward covers genuinely unassigned ranges only. Those drop out of the DCCP and SCTP registries, so they answer unassigned [dccp] / [sctp]. reserved ranges already have their own IANA rows for those transports, so they need nothing. I'll check that against the CSV before relying on it.

JarryShaw added a commit that referenced this pull request Oct 22, 2026
…2048)

IANA assigned 8011 on TCP and UDP only, and its CSV dropped the port from
the unassigned span 8011-8014, so DCCP and SCTP answered `unknown`. Per the
owner's rulings on #2052:

- pcapkit/vendor/reg/apptype/apptype.py: the base crawler reads the spans
  of the module it replaces (AST of `AppType._missing_`) and carries an
  unassigned or reserved span forward, per registry, for each transport the
  CSV now gives the port no row and no span on. Carried branches are marked
  so the next regeneration carries them again; a port leaves them as IANA
  assigns it on each transport. Spans naming no transport answer with
  `cls.__transport__` instead of `TransportProtocol.undefined`.
- pcapkit/const/reg/apptype/apptype.py: regenerated offline from the current
  CSV; the history includes the modules before 1.5.0b13 and 1.5.0b14, which
  restores 4930 and 8011 for DCCP and SCTP.
- A port in no span still answers `unknown` with its registry's transport.
- Changelog entry; CHANGELOG.md regenerated. Tests for the carry-forward,
  the generator's edge cases, and the committed module as a fixed point.
@JarryShaw
JarryShaw force-pushed the fix/2048-apptype-unassigned-fallback branch from 30b14e7 to 1ec7690 Compare October 22, 2026 22:53
@JarryShaw

Copy link
Copy Markdown
Owner Author

Rework pushed as 1ec76907a: fallback unknown [<registry>], spans answering with the registry's transport, and per-registry carry-forward. The table is in the PR body. Three points need you:

  1. reserved is carried too, not only unassigned. I checked IANA's CSV: 3 of its 4 Reserved range rows (225-241, 249-255, 1011-1020) name no transport, the same as all 755 Unassigned ranges. Only port 8282 has single-port Reserved rows for DCCP or SCTP. So DCCP and SCTP lose a reserved span exactly as they lose an unassigned one, which is your 8000-8100 example. Nothing reserved is carried today, so the committed file is the same either way. I recommend keeping both. Unassigned only is a one-line change.
  2. Backfill further back? Before the AppType split, the older history has 27 more ports that left spans and now have rows only on TCP and UDP. I recommend leaving them, since that would mean mining the pre-split module format once. The rule then covers every port from 1.5.0b13 on.
  3. Breaking? A span member's proto changes from undefined to the registry's transport, and a direct DCCP(8011) now resolves instead of raising. I recommend not marking it breaking, because this is the fix you ruled for and the old undefined was the defect.

@JarryShaw JarryShaw added needs: decision Waiting on the maintainer to decide — not blocked by other work 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 Oct 22, 2026
@JarryShaw JarryShaw moved this from In review to Needs decision in PyPCAPKit Oct 22, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

so like i ruled, by default 0-65535 should be unassigned [transport]. if any transport has an override, it takes that out and design it. otherwise, the unassigned [transport] stays.

@JarryShaw
JarryShaw force-pushed the fix/2048-apptype-unassigned-fallback branch from 1ec7690 to 261fd46 Compare October 22, 2026 23:22
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 261fd4630, which implements the 22:57Z rule and is rebased onto main. The PR body has the table and evidence. A fresh cross-review is in progress.

@JarryShaw 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 Oct 22, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 261fd4630 (Sonnet cross-review). On all four registries, for every port 0–65535, an answer's .proto is now the asking registry's transport (0 mismatches against main). Every changed answer is either unknown→unassigned or a shared span's undefined→that transport. Spot checks match the 22:57Z rule: 8011 is reserved on TCP, bier-echo on UDP and unassigned on SCTP and DCCP. 9 is discard on all four, and -1 and 65536 still raise. Sweeping the ports minted no members. With main's apptype.py and numbers.py swapped back in, the new tests fail.

Two small things, both being fixed in one amend:

  • Wrong comment: the get() fallback comment (template and generated module) has 8011 backwards. IANA assigns it on TCP and UDP, not on DCCP and SCTP.
  • Unchecked version: numbers.py's "Measured on CPython 3.14.7" → "3.14.9" was not re-measured. It gets re-measured, or goes back to 3.14.7.

@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 Oct 22, 2026
@JarryShaw
JarryShaw force-pushed the fix/2048-apptype-unassigned-fallback branch from 261fd46 to 78f2c51 Compare October 22, 2026 23:35
JarryShaw added a commit that referenced this pull request Oct 22, 2026
…lists nothing (#2048)

Per the owner's ruling on #2052, every port in 0-65535 is `unassigned` on
a transport, carrying that transport, unless IANA lists an entry for it
there. IANA assigned 8011 on TCP and UDP only, so DCCP and SCTP had read
`unknown`; they now read `unassigned [dccp]` / `[sctp]`.

- AppType.get (template and generated module, identically): the fallback
  for a port `_missing_` holds no span for answers `unassigned` with the
  registry's own transport instead of `unknown`. Still unregistered (#860);
  out-of-range ports still raise.
- AppType._missing_: spans naming no transport answer with
  `cls.__transport__` instead of `TransportProtocol.undefined`; spans
  naming a transport (#760) are unchanged. The generated module renders
  byte-identically from the template and the current IANA CSV.
- PortEnumField.post_process: its miss member reads `unassigned` too.
- Tests pinning `unknown` or `undefined` for AppType holes and spans move to
  `unassigned [<registry>]`; new rule and template-agreement tests.
- Changelog entry; CHANGELOG.md regenerated.
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 78f2c518a (amend of 261fd4630). The only change is the corrected get() fallback comment, made identically in the template and the generated module: 8011 is reserved on TCP and bier-echo on UDP, so it answers unassigned on DCCP and SCTP. The 3.14.9 note in numbers.py stays, because it was re-measured on CPython 3.14.9: port 53406 round-trips through pickle with the per-instance __reduce_ex__, and raises the quoted ValueError without it. Template agreement, the AppType const tests and the PortEnumField tests give 112 passed. The diff since the review is comments only, so the GOOD TO GO carries to 78f2c518a.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at 78f2c518a: the review is GOOD TO GO and CI is complete (56 passed, 0 failed). The ordering leg ran 1945 tests with 0 failures. The 3.8–3.10 KTLO legs show 0 failing tests, and the PR is based on current main (c98541f84).

Comment thread pcapkit/vendor/reg/apptype/apptype.py
Comment thread pcapkit/corekit/fields/numbers.py
@JarryShaw 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 Oct 23, 2026
…'s transport (#2048)

Per the owner's rulings on #2052, an IANA port span naming no transport
answers each AppType registry with that registry's own transport rather
than `TransportProtocol.undefined`, so `TCP.get(8012)` reads
`unassigned [tcp]`. A port in no span keeps `get`'s `unknown` fallback,
which already carried the registry's transport: IANA assigned 8011 on TCP
and UDP only, so DCCP and SCTP read `unknown [dccp]` / `unknown [sctp]`.

- AppType._missing_ (template and generated module, identically): spans
  naming no transport answer with `cls.__transport__`; spans naming a
  transport (#760) are unchanged. Outside its generated blocks the
  committed module matches the rendered template byte for byte.
- AppType.get and PortEnumField: unchanged (`unknown`, registry's own
  transport); comments and docstrings name 8011 as the no-span case.
- test_every_transport_named_span_is_claimed_by_one_registry: no span
  answers `undefined`; new rule and template-agreement tests.
- Changelog entry; CHANGELOG.md regenerated.
@JarryShaw
JarryShaw force-pushed the fix/2048-apptype-unassigned-fallback branch from 78f2c51 to 990d037 Compare October 23, 2026 02:06
@JarryShaw JarryShaw changed the title fix(const): answer unassigned for a port a transport's registry has no row for (#2048) fix(const): answer IANA's transport-less port spans with the registry's transport (#2048) Oct 23, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 990d037a9, which applies the 01:48Z ruling and is rebased onto main (fd0d2e724). The get() fallback and PortEnumField are back to unknown on the registry's transport, exactly as on main, so numbers.py no longer changes. What remains is the span-transport fix. The title, body, changelog and tests now say so. A fresh cross-review is running against this head.

@JarryShaw 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 Oct 23, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 990d037a9 (Sonnet cross-review). numbers.py and the get() fallback are exactly main's.

  • What changed: across every port 0–65535 on all four registries, the only differences are span members whose .proto goes from undefined to the registry's own transport. Name and value are unchanged, and every answer's .proto now equals its registry's transport. Their repr and string value change with it: reserved [225 - undefined] becomes reserved [225 - tcp].
  • unknown answers: the set of ports answering unknown, and their reprs, are identical to main's. 8011 is unknown on SCTP and DCCP.
  • Tests: the new tests fail on main on the span cases and pass here. The AppType, PortEnumField, round-trip, TCP/UDP and dumpkit tests pass.
  • One side effect worth knowing: register_apptype with a span member as code and no transport argument used to raise unknown transport protocol: undefined. It now registers under that member's own registry. That is arguably better, but it isn't in the changelog. Should I add a line for it?

@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 Oct 23, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at 990d037a9: the review is GOOD TO GO and CI is complete (56 passed, 0 failed). The ordering leg ran 1991 tests with 0 failures. The 3.8–3.10 KTLO legs show 0 failing tests, and the PR is based on current main (fd0d2e724). The question above about a changelog line for register_apptype is still open; it doesn't block the merge.

@JarryShaw
JarryShaw merged commit 4cdccb5 into main Oct 23, 2026
63 checks passed
@JarryShaw
JarryShaw deleted the fix/2048-apptype-unassigned-fallback branch October 23, 2026 04:00
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 23, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 23, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issues reporting a defect (set by the bug report template; a default, not an assessment) const Regenerated IANA or vendor constant tables; members keep their numeric values

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

AppType: DCCP/SCTP answer 'unknown' for ports IANA assigns to TCP/UDP only, where RFC 6335 says Reserved

1 participant