Skip to content

fix(sctp): check parameter types in the Supported Address Types and Missing Mandatory Parameter makers (#2288) - #2363

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2288-sctp-param-type-bool-str
Oct 27, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2288-sctp-param-type-bool-str

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — bug fix

Description

Closes #2288.

  • Fix: a new SCTP._make_param_types helper backs both the Supported Address Types and Missing Mandatory Parameter makers, and raises ProtocolError for a bool or anything that is neither a Parameter nor an int.
  • Fail-without-fix: 10 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 5936220b8 (Sonnet) — two change-relative phrases in the test, 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
…issing Mandatory Parameter makers (#2288)

SCTP._make_param_addrtypes and _make_cause_missing_param shared the
conversion "Enum_Parameter.get(item) if isinstance(item, int) else item":
True became parameter type 1, False raised EnumValueError, and a str or
float item was stored unchecked.

Both makers now call one helper, _make_param_types, which raises
ProtocolError "SCTP: invalid parameter type: ..." for a bool or any item
that is neither a Parameter nor an int. The helper is documented in
sctp.rst, since both makers' docstrings refer to it.

Test: test_parameter_type_lists_refuse_a_bool_and_a_non_int pins the
message for True, False, '5', 5.0 and None in both makers.
@JarryShaw
JarryShaw force-pushed the fix/2288-sctp-param-type-bool-str branch from 5936220 to 0ca99b0 Compare October 27, 2026 04:24
@JarryShaw

Copy link
Copy Markdown
Owner Author

Amended 5936220b8 → 0ca99b039: reworded the test docstring and comment to timeless statements. test_sctp_unit.py + test_docstring_contract.py give 46 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 moved this to In review in PyPCAPKit Oct 27, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Ready to merge at 0ca99b039. Closes #2288.

  • Review: NEEDS CHANGES → fixed (two change-relative test phrases) → 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.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.58% (unit tier, Python 3.15, 0ca99b039, 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 17449 218 4590 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 merged commit ef4d93f into main Oct 27, 2026
63 checks passed
@JarryShaw
JarryShaw deleted the fix/2288-sctp-param-type-bool-str branch October 27, 2026 14:57
@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(sctp): Supported Address Types and Missing Mandatory Parameter makers take bool, str

1 participant