Skip to content

fix(sctp): rebuild a Host Name parameter without a NUL terminator as captured (#1482) - #1483

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1482-sctp-hostname-nul
Oct 9, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1482-sctp-hostname-nul

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

The reader keeps a Host Name Address parameter's name as captured. _make_param_hostname rejected any name without a NUL terminator, so SCTP.from_data(info) raised ProtocolError: [Param 11] invalid format on a packet that parsed.

The fix follows the existing if <record> is not None: … else: <validate keyword> maker idiom (e.g. ipv4.py:1522). The terminator check now runs only on a user name keyword (pcapkit/protocols/transport/sctp.py:2970-2973), and a parsed record rebuilds as read. A hand-built make() without a terminator still raises. Docstrings in transport/sctp.py and data/transport/sctp.py are updated to match. schema/transport/sctp.py (#1481) is untouched.

New tests/protocols/transport/test_sctp_hostname_nul_unit.py covers the #1482 repro, a terminated name, every cut at offsets 44–66 of the #1481 multi-chunk packet, and make() with abcdefgh. Through both info and info.to_dict(), all rebuild cases fail on origin/main and pass here. Results: test_sctp*.py 66 passed; test_transport_edge_roundtrip_unit.py 6 passed; tests/project 419 passed, 1 skipped. repr(info.to_dict()) hashes over the 23 files in examples/captures are identical to origin/main.

Closes #1482

…captured (#1482)

The reader keeps a Host Name Address parameter's name as captured, but
_make_param_hostname rejected any name without a NUL terminator, so
from_data(info) raised on a packet that parsed. The terminator check
now applies only to a user-supplied ``name`` keyword; a parsed record
carries its name through unchanged. Docstrings updated to match.

Tests: the #1482 repro, a terminated name, every cut after the INIT
chunk of a multi-chunk packet, and a direct make() without a
terminator, which still raises.
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 9, 2026
@JarryShaw JarryShaw added 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 9, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

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

  • The fix: a captured Host Name now rebuilds as captured. I re-ran the fix(sctp): a Host Name parameter without a NUL terminator parses but cannot be rebuilt #1482 repro (abcdefgh, no terminator): it raises on main and rebuilds byte-exactly at head. The new tests give 25 failed on origin/main sources and 5 passed, 29 subtests at head.
  • Names tried: abcde, abcdefgh, a\0b and empty all rebuild byte-exactly at head through .data, from_data(info) and from_data(info.to_dict()), with the captured length field. abcd\0 and ab\0\0\0 were already byte-exact on main and still are.
  • The user path still checks: a hand-built make() with an unterminated or empty name still raises. When param is given, it takes precedence over the name keyword, as in _make_param_ipv4.
  • Regression checks: the per-capture to_dict() hashes are identical across all 23 captures. The SCTP and edge tests give 72 passed, 286 subtests; tests/project gives 419 passed.
  • Overlap: its files are disjoint from fix(sctp): accept a final parameter or cause short of its padding (#1479) #1481's.

Non-blocking: the comment on pcapkit/protocols/schema/transport/sctp.py:593 still says the name includes a NUL terminator. That file belongs to #1481, so I'll correct the comment in whichever of the two lands second.

@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, dcc08bdc0, 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 00d2328 into main Oct 9, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1482-sctp-hostname-nul branch October 9, 2026 18:22
@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 added a commit that referenced this pull request Oct 9, 2026
#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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): a Host Name parameter without a NUL terminator parses but cannot be rebuilt

1 participant