Skip to content

refactor(corekit): share one PortEnumField across TCP, UDP and SCTP (#1516) - #1526

Merged
JarryShaw merged 1 commit into
mainfrom
refactor/1516-port-enum-field
Oct 9, 2026
Merged

refactor(corekit): share one PortEnumField across TCP, UDP and SCTP (#1516)#1526
JarryShaw merged 1 commit into
mainfrom
refactor/1516-port-enum-field

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 9, 2026

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

  • refactor — changes neither behaviour nor performance

Description of your pull request and other information

The three PortEnumField classes in schema/transport/{tcp,udp,sctp}.py were AST-identical apart from the TransportProtocol member in post_process. They become one public PortEnumField in corekit/fields/numbers.py, beside EnumField, with a keyword-only proto. Each schema passes its own. It is documented on the numbers page and in the fields class diagram.

  • Byte-identical: all 1398 TCP, UDP and SCTP layers in examples/captures/* (1261/103/34) give the same octets and port members before and after. That covers parse→rebuild, from_data(info.to_dict()), and make→parse from members and from bare ports. Digest sha256 010bd641… both times.
  • Tests: tests/corekit/test_fields_numbers_port_enum_field_1516_unit.py has 7 tests, and all 7 fail on main. A mutant that ignores proto fails 5 subtests. Updated: the existing callers of the old classes, the field round-trip table, and the composite-proto sweep in tests/const.
  • Not re-exported from corekit.fields: doing so forces PortEnumField into corekit/__init__.py's __all__, which refactor(corekit)!: Info.to_dict() returns an OrderedMultiDict; add MultiInfo and OrderedMultiInfo (#1484) #1523 has in flight (test_public_api fails otherwise). SwitchField sets the precedent.
  • Selections run, not the whole suite: tests/corekit, tests/protocols/transport, each tests/protocols/test_*roundtrip*, tests/project, tests/test_docstring_contract.py, tests/test_tier_guard.py, plus the touched tests/const and tests/dumpkit modules. All pass.

Refs #1516

@JarryShaw
refactor(corekit): share one PortEnumField across TCP, UDP and SCTP (#…
c68689e 
…1516)

- corekit.fields.numbers: add a public PortEnumField beside EnumField. It
  takes a keyword-only proto, the TransportProtocol member its ports belong
  to; its pre_process and post_process are the three copies' bodies, with
  that member in place of the hard-coded one.
- schema/transport/{tcp,udp,sctp}: drop the local PortEnumField classes,
  which an AST diff shows differed only in that member, and pass each
  schema's own transport to the shared field.
- docs: document it on the numbers page and in the fields class diagram;
  repoint the cross-references to the removed classes.
- tests: new unit tests pin the per-transport lookup, an unassigned port,
  pack and unpack; the existing callers pass proto, the field round-trip
  table gets one case per transport, and the composite-proto sweep follows
  the lookups into numbers.py and pins the six constructor calls.

Behaviour is byte-identical on the 1398 TCP, UDP and SCTP layers of
examples/captures/*.

Refs #1516
@JarryShaw JarryShaw added 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 Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Coverage: 90.10% (unit tier, Python 3.14, c68689e63, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 842 2356 756 92.09%
pcapkit/corekit 2186 67 710 34 96.24%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2665 98 934 42 95.00%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 17021 229 4598 189 98.04%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 431 4 124 4 98.56%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw

JarryShaw commented Oct 9, 2026

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at c68689e63. Cross-reviewed on Sonnet; the author ran Opus.

  • The three old classes are AST-identical once docstrings are stripped. The only difference is the Enum_TransportProtocol.<proto> member.
  • Field behaviour is identical. A sweep of post_process over every port 0–65535 and out-of-range values, plus unpack, pack, pre_process and pickling, compared 2,163,609 entries per run with transports in both orders. The only differences are the class's __module__ and the new _proto. 900 TCP/UDP ports and 6,155 TCP/SCTP ports resolve differently, so the sweep is not vacuous. No registry entries are minted.
  • Captures are byte-identical. On 1,398 TCP/UDP/SCTP layers, canonical to_dict(), from_data, pack({}) and three make→parse paths give identical output. The 69 json/tree/plist dumps are identical too.
  • The lazy AppType import is not a hot-path cost. extract(http.pcap) makes 0 pre_process calls.
  • Fail-before and mutation. The new tests give 29 failures. A mutant that ignores proto fails exactly 5 subtests.
  • The other changed tests are updated, not weakened.

Non-blocking:

@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 9, 2026
@JarryShaw
JarryShaw merged commit f96368b into main Oct 9, 2026
41 of 42 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 9, 2026
@JarryShaw
JarryShaw deleted the refactor/1516-port-enum-field branch October 9, 2026 23:47
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
@JarryShaw JarryShaw mentioned this pull request Oct 26, 2026
Merged
5 tasks done
JarryShaw added a commit that referenced this pull request Oct 26, 2026
@JarryShaw
fix(corekit): range-check ports in PortEnumField, and document Number…
841014d 
…Field's masking (#2314)

Problem: a TCP or UDP schema built directly packed an out-of-range port by
masking it to 16 bits (srcport=70000 -> 0x1170, -1 -> 0xffff), because
PortEnumField.pre_process handed a bare int to NumberField's mask unchecked.

Fix: the UDP, TCP and SCTP port fields already use PortEnumField (#1516,
#1526), so the fix is the range check there: its pre_process raises
FieldValueError for a port outside the field's width. NumberField keeps
masking as the documented schema contract (#1488); the NumberField and
Schema docstrings say direct construction does not range-check and name
where the checks live.

Test: test_tcp_udp_schema_port_range_unit.py rejects -1/65536/70000 on both
ports of both schemas, and round-trips ports 0 and 65535 byte-exact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

refactor Restructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

Projects

PyPCAPKit
Status: Done

Milestone

PyPCAPKit v1.5

Development

Successfully merging this pull request may close these issues.

1 participant

@JarryShaw