Skip to content

fix(protocols)!: pad two octets with one PadN, not two Pad1 (#1793) - #1812

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1793-padn-for-two-octets
Oct 16, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1793-padn-for-two-octets

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • 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

Summary

For two octets of padding, the HOPOPT, IPv6-Opts and MH padding builders emitted two Pad1 options. RFC 8200 §4.2, and RFC 6275 §6.2.2 for Mobility options, say a PadN should be used rather than multiple Pad1 options.

  • Fix: in each _make_pad_options (hopopt.py, ipv6_opts.py, mh.py; they share no helper), only one octet gives a Pad1. Two octets become a PadN of length 0 (01 00), and three or more are unchanged. The builders' docstrings cite the RFCs.
  • Breaking: a freshly built header with exactly two octets of padding has different bytes. Parsing is unaffected, and a parsed packet that carried two Pad1 options still rebuilds byte for byte.
  • Tests: tests/protocols/internet/test_padn_two_octets_1793_unit.py fails on main for all three protocols. Existing fresh-build byte pins move to 01 00; parsed-frame pins are unchanged.
  • Runs: tests/protocols/internet, the option and capture round-trip suites, and tests/project pass.

Closes #1793

HOPOPT, IPv6-Opts and MH each filled a two-octet alignment shortfall with
two Pad1 options. RFC 8200 section 4.2, and RFC 6275 section 6.2.2 for
mobility options, say a PadN should be used rather than multiple Pad1.

- `_make_pad_options` in hopopt.py, ipv6_opts.py and mh.py: two octets are
  now one PadN of length 0 (`01 00`); one octet stays a Pad1 and three or
  more keep the PadN they had. Docstrings follow.
- Parsed padding is untouched: a header carrying two Pad1 still rebuilds
  byte for byte, since parsed options are re-emitted as read.
- Tests: new test_padn_two_octets_1793_unit pins the helper and the built
  headers for all three protocols, plus the two-Pad1 parsed rebuild; the
  fresh-build pins in five existing modules move to `01 00`.

Closes #1793
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) breaking Breaks public-facing behaviour or API (apply alongside the type label) labels Oct 16, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 109638aa4. A Sonnet cross-review (author: Opus) checked 6fe698f6d; this head only adds ! to the subject, and the tree is identical. It checked the RFC text, and a grep found exactly three padding builders. The new tests fail on main. A property probe over 94 built headers found no adjacent Pad1 options on the fix, all round-tripping byte for byte. Old two-Pad1 captures still rebuild exactly, and EXPECTED_FAILURES is untouched (empty).

@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 16, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.60% (unit tier, Python 3.15, 109638aa4, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18862 954 2368 868 91.06%
pcapkit/corekit 2462 69 780 35 96.55%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2915 164 1016 34 93.59%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17197 224 4580 189 98.09%
pcapkit/toolkit 872 177 298 8 77.18%
pcapkit/utilities 512 2 158 4 99.10%
pcapkit/vendor 4417 2342 1010 157 43.38%

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

@JarryShaw
JarryShaw merged commit 517cd70 into main Oct 16, 2026
57 checks passed
@JarryShaw
JarryShaw deleted the fix/1793-padn-for-two-octets branch October 16, 2026 13:11
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 16, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 16, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaks public-facing behaviour or API (apply alongside the type label) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(protocols): two octets of option padding are written as two Pad1 options, not one PadN

1 participant