Skip to content

fix(foundation): reject a bool in _check_code(number=True) (#2269) - #2344

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2269-check-code-bool
Oct 27, 2026
Merged

fix(foundation): reject a bool in _check_code(number=True) (#2269)#2344
JarryShaw merged 1 commit into
mainfrom
fix/2269-check-code-bool

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 27, 2026

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — bug fix

Description

Closes #2269.

  • Fix: _check_code(number=True) now excludes bool, so register_tcp(True, X) and the UDP/SCTP/AppType equivalents raise RegistryError instead of registering port 1 or 0.
  • Fail-without-fix: 9 failed on origin/main. Passing on the rebased head.

@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 27, 2026
@JarryShaw JarryShaw added bug Issues reporting a defect (set by the bug report template; a default, not an assessment) fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 27, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 27, 2026
@JarryShaw

JarryShaw commented Oct 27, 2026

Copy link
Copy Markdown
Owner Author

Review: NEEDS CHANGES at 1f465ba6 (Sonnet) — docs only, code and test are correct.

  • Confirmed: True/False now raise RegistryError on all four registrars; every other code (ints, enum members, out-of-range ints) is unaffected. Fail-without-fix: 9 failed on origin/main. On the head, 57+106 passed across the registry and caller tests.
  • Required: the four Raises: docstrings still say code must be "neither an int nor a ... member", which is false now that bool (an int subclass) is rejected. Add ", or a :class:bool" to each, matching how fix(foundation): registrars accept a member of the wrong enum as the code #2224 worded the same sentence.
  • Non-blocking: no positive-control test for 0/1 still registering; register_protocol_code, class X(..., code={...: True}) and .register(True, ...) still accept a bool (that's fix(foundation): register_protocol_code's mapping form skips _check_code, replaces entries #2268's territory, already open); port range (-1, 70000) is unchecked, pre-existing, possibly worth its own issue.

@JarryShaw 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 Oct 27, 2026
@JarryShaw
fix(foundation): reject a bool in _check_code(number=True) (#2269)
705c623 
_check_code let any non-enum int through with number=True, and bool
is an int subclass, so register_tcp(True, X) registered port 1 (and
likewise register_udp, register_sctp and register_apptype).

Exclude bool from the int branch, so it falls through to the existing
RegistryError: "code must be an int or a member of AppType, not True".

Test: test_number_registrars_reject_a_bool checks True and False on
every number registrar with each registry unchanged; fails on main.
@JarryShaw
JarryShaw force-pushed the fix/2269-check-code-bool branch from 1f465ba to 705c623 Compare October 27, 2026 02:06
@JarryShaw

JarryShaw commented Oct 27, 2026

Copy link
Copy Markdown
Owner Author

Amended 1f465ba6 → 705c623c9: added ", a bool is not accepted, even though it is one" to the Raises: of register_apptype, register_tcp, register_udp and register_sctp. tests/foundation/registry and tests/test_docstring_contract.py give 57 passed.

@JarryShaw 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 Oct 27, 2026
@github-actions

github-actions Bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Coverage: 91.58% (unit tier, Python 3.15, 705c623c9, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19599 881 2618 867 92.11%
pcapkit/corekit 2596 74 818 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 3029 23 1050 25 98.82%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17442 218 4586 185 98.15%
pcapkit/toolkit 914 36 334 10 95.51%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4432 1988 1018 167 51.87%

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

@JarryShaw

JarryShaw commented Oct 27, 2026

Copy link
Copy Markdown
Owner Author

Ready to merge at 705c623c9. Closes #2269.

  • Review: NEEDS CHANGES → fixed (a missing docstring mention) → self-certified, since the change was purely additive prose the reviewer had already said was the only gap.
  • CI: Required checks passed SUCCESS, 0 failures, KTLO clean.
  • Ordering leg: foundation gave 751 tests with 0 failures, run with the PR merged into current main.
  • Merge state: mergeable.

@JarryShaw
JarryShaw merged commit 48ad928 into main Oct 27, 2026
62 checks passed
@JarryShaw
JarryShaw deleted the fix/2269-check-code-bool branch October 27, 2026 04:25
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 27, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 27, 2026
@JarryShaw JarryShaw mentioned this pull request Oct 28, 2026
Merged
4 tasks done
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

bug Issues reporting a defect (set by the bug report template; a default, not an assessment) fix Pull requests that fix a defect (fix: subject prefix)

Projects

PyPCAPKit
Status: Done

Milestone

PyPCAPKit v1.5

Development

Successfully merging this pull request may close these issues.

fix(foundation): _check_code(number=True) takes a bool, so register_tcp(True) adds port 1

1 participant

@JarryShaw