Skip to content

fix(transport): reject a bool port in TCP, UDP and SCTP make (#2287) - #2359

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2287-transport-bool-port
Oct 27, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2287-transport-bool-port

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — bug fix

Description

Closes #2287.

  • Fix: Transport._make_port now raises ProtocolError for a bool port on TCP, UDP and SCTP, instead of resolving True/False to port 1/0 through AppType.get.
  • Fail-without-fix: 12 subtests 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

Copy link
Copy Markdown
Owner Author

Review: NEEDS CHANGES at 8449ef83b (Sonnet) — two timed-context comments, code is correct.

@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
Transport._make_port handed any non-AppType value to AppType.get, which
looked a bool up as port 0 or 1, so UDP(srcport=True) packed port 1 and
TCP(dstport=False) port 0.

_make_port becomes a classmethod and raises ProtocolError
"<Protocol>: invalid port: True" for a bool, which covers SCTP as well.

Test: test_make_refuses_a_bool_port pins the message for True and False
on either port of TCP, UDP and SCTP.
@JarryShaw
JarryShaw force-pushed the fix/2287-transport-bool-port branch from 8449ef8 to 8ceea7e Compare October 27, 2026 03:02
@JarryShaw

Copy link
Copy Markdown
Owner Author

Amended 8449ef83b → 8ceea7e00: reworded the two timed-context comments in the new test to timeless statements. tests/protocols/transport/test_transport_unit.py + tests/test_docstring_contract.py give 20 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
@JarryShaw JarryShaw changed the title fix(transport): reject a bool port in TCP and UDP make (#2287) fix(transport): reject a bool port in TCP, UDP and SCTP make (#2287) Oct 27, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.58% (unit tier, Python 3.15, 8ceea7e00, 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 17444 218 4588 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

Copy link
Copy Markdown
Owner Author

Ready to merge at 8ceea7e00. Closes #2287.

  • Review: NEEDS CHANGES → fixed (two timed-context test comments) → self-certified, prose-only change.
  • CI: Required checks passed SUCCESS, 0 failures, KTLO clean.
  • Ordering leg: protocols gave 1969 tests with 0 failures, run with the PR merged into current main.
  • Merge state: mergeable.

@JarryShaw
JarryShaw merged commit d065087 into main Oct 27, 2026
63 checks passed
@JarryShaw
JarryShaw deleted the fix/2287-transport-bool-port branch October 27, 2026 14:58
@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
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) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(transport): TCP and UDP make() take a bool port as port 0 or 1

1 participant