Skip to content

docs(sctp): a parsed Host Name may lack its null terminator - #1486

Merged
JarryShaw merged 1 commit into
mainfrom
docs/sctp-hostname-comment
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/sctp-hostname-comment

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Follow-up to #1482 and #1483, which made a captured Host Name without a null terminator rebuild as captured. Two comments still described the old rule:

  • pcapkit/protocols/schema/transport/sctp.py:596 said the name always includes at least one null terminator. It now says the name is as captured: RFC 9260 asks for the terminator, but a parsed name may lack it.
  • pcapkit/protocols/data/transport/sctp.py:363 said "null terminator included; a parsed name may lack one", which contradicts itself. It now says "with any null terminator".

Comments only; no behaviour change. #1483's cross-review flagged both lines.

#1483 made a captured Host Name without a NUL rebuild as captured, but the
schema field comment still said the name always includes one, and the data
comment read as a contradiction. Both now say the name is as captured.
@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 352df98b2. Cross-reviewed on Sonnet; the author ran on Opus. The change is comments only.

  • Both comments match the code on main. A parsed name=b'abcdefgh' rebuilds as length=12 with the name unchanged, and make(name=b'abc') still raises ProtocolError.
  • The :rfc:9260#section-3.3.2.1.4`` reference matches the three existing uses in transport/sctp.py.
  • Nothing else is stale. No other comment or doc still claims a Host Name always ends in NUL. The name: keyword docstring of _make_param_hostname describes user input and is correct.
  • tests/test_docstring_contract.py: 7 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 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Coverage: 89.57% (unit tier, Python 3.14, 352df98b2, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 958 2356 871 90.99%
pcapkit/corekit 2107 93 672 27 94.60%
pcapkit/dumpkit 242 2 88 2 98.79%
pcapkit/foundation 2648 98 930 48 94.80%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 17086 239 4628 192 97.97%
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 merged commit 8edcb8d into main Oct 9, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the docs/sctp-hostname-comment branch October 9, 2026 19:17
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 9, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Pull requests that change documentation only (docs: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant