fix(sctp): rebuild a last chunk short of its padding as captured (#1474)

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change: a test covers it, and the legs below pass; the full suite was not run
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible: N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

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

Closes #1474. A last chunk with fewer pad octets than its length calls for, or none, now rebuilds as captured through both from_data(info) and from_data(info.to_dict()). Before, padding_length (pcapkit/protocols/schema/transport/sctp.py:84) always packed the full width, so 17 octets in gave 20 out.

Why the existing padding key. #1223 already keeps a chunk's, parameter's or cause's padding in info when a fresh build would not reproduce it (non-zero). A short pad is the same situation, so _keep_padding now also keeps it when it is short, possibly as b''. It is the same key, holding the octets as captured, and it already round-trips through to_dict(). A __short_read__-style dunder would duplicate that record. trailer cannot carry it either, because it follows the chunk, which would still pad itself. No new key, so no !.

On rebuild, _restore_padding gives the schema a __padding__ width that padding_length packs to. The pre_pack of Chunk, Parameter and ErrorCause resets the key for each schema, because Schema.pack shares one context across siblings. Only the last item of a list keeps the record: an earlier chunk, and anything nested in it, packs to the full aligned width even if an edited info says otherwise. make takes no padding keyword, as the data-model docstrings now say.

The five #1474 Gap rows are deleted from the transport edge table. New module tests/protocols/transport/test_sctp_unpadded_last_chunk_unit.py: on origin/main sources, 14 failed and 6 passed; with this fix, 8 passed (12 subtests). The SCTP, transport-edge, capture-runtime and tests/project legs pass. The per-frame to_dict() hashes over examples/captures (1616 frames) match origin/main.

Out of scope: an INIT or ABORT whose final parameter or cause is also unpadded is still rejected at parse ("option that runs past the end of the data"), not mis-rebuilt.

[JarryShaw]

Verdict: NEEDS CHANGES at e583895e4. Cross-reviewed on Sonnet; the author ran on Opus.

Required: honour the short-padding record only on the last item in the packet (chunk, parameter or cause), or document and test the alternative.

• Today a non-last chunk whose padding is shortened in its info is packed verbatim, and the output is malformed.
• Repro, which I re-ran myself: parse COMMON + chunk(0x3f, b'a') + chunk(0xbf, b'b'), set the first chunk's padding to b'' via __update__, then rebuild with SCTP.from_data(d).
• The result is 3f00000561 bf00000562 000000: the second chunk starts unaligned, and re-parsing it swallows that chunk's header as padding.
• main zero-fills in this case.

Confirmed fine:

• Root cause and mechanism are as described.
• The per-schema pre_pack reset works for interleaved short and full chunks.
• No padding key appears on ordinary traffic: the 34 SCTP frames in examples/captures and synthetic full-width cases carry none, and per-capture to_dict() hashes match main.
• All five fix(sctp): a last chunk sent without its padding rebuilds with the padding zero-filled #1474 Gap rows close.
• 8 threads × 200 rebuilds produced no cross-packet leak.
• The new test fails on main's sources (14) and passes here; I confirmed this myself.

Minor: make(chunks=[...]) ignores a padding keyword, while the from_data path honours it. Please state this in the docstrings.

Separate defect, filed as its own issue: an INIT or ABORT whose final parameter or cause is unpadded at the end of the packet is rejected at parse time. This happens on main too.

[JarryShaw]

Verdict: NEEDS CHANGES at 3c064da75. This is the round-2 cross-review, run on Sonnet; the author ran on Opus. The source change is correct. What is missing is a test.

• The gap. Nothing tests the nested half of _last_item_only: the for name in ('parameters', 'error') loop at pcapkit/protocols/transport/sctp.py:305. I disabled that loop (for name in ()) and test_sctp_unpadded_last_chunk_unit.py still passed, 8 of 8. So the loop is untested.
• Why it matters. The reviewer probed a non-last INIT whose last parameter has padding=b'', followed by a SHUTDOWN_ACK. With the loop it rebuilds aligned at 44 octets; without it, misaligned at 41. A non-last ABORT with a short last cause gives 28 octets with the loop and 25 without.
• Fix. Add one test for each of those two cases, and show that both fail when the loop is disabled.

Everything else checked out:

• Single-chunk and last-chunk cases, and no mutation of the input.
• The fix(sctp): a last chunk sent without its padding rebuilds with the padding zero-filled #1474 Gap rows are removed.
• SCTP tests: 65 passed, 257 subtests.
• to_dict() hashes are identical across all 23 captures against origin/main.
• The three new tests fail when _last_item_only is a no-op.

[JarryShaw]

Verdict: GOOD TO GO at efa879309. This is the round-3 delta review, run on Sonnet; the author ran on Opus. The commit adds 34 lines, to test_sctp_unpadded_last_chunk_unit.py only. I then rebased it onto main at bbfd4bf64: range-diff shows both commits as =, and the patch-id is unchanged.

• The round-2 gap is closed. There are two new tests: a non-last INIT whose last parameter is short (44 octets), and a non-last ABORT whose last cause is short (28 octets).

• Each test pins its own half of the loop at sctp.py:305. I re-ran this:

  Loop at sctp.py:305	Result
  for name in ()	2 failed
  ('parameters',) only	the ABORT test fails
  ('error',) only	the INIT test fails
  unchanged	10 passed

• Packet layout: the chunk length excludes the last item's padding, but the pad octets are still on the wire, so the next chunk stays aligned.

• SCTP and transport edge files: 67 passed, 257 subtests.

Non-blocking: test_short_record_on_a_non_last_parameter_rebuilds_aligned iterates .values() over two parameters of the same type, so it only strips the first one. It still checks the case it is named for.

[github-actions]

Coverage: 89.56% (unit tier, Python 3.14, efa879309, 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	17079	240	4626	193	97.96%
pcapkit/toolkit	539	74	168	3	84.58%
pcapkit/utilities	429	4	122	4	98.55%
pcapkit/vendor	4409	2342	1006	157	43.25%

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