Skip to content

fix(protocols): set NoPayload on terminal AH extension layer - #2486

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2484-ah-membership-attributeerror
Oct 31, 2026
Merged

fix(protocols): set NoPayload on terminal AH extension layer#2486
JarryShaw merged 1 commit into
mainfrom
fix/2484-ah-membership-attributeerror

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner
  • 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
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #2484

  • AH from IPv6.extension_headers returned before _decode_next_layer, so _next was unset and 'ESP' in ah raised AttributeError.

  • The extension-mode AH layer now holds NoPayload: membership is False for non-matching selectors. ah.payload still raises UnsupportedCall in extension mode.

  • Matching selectors, ESP extension layers and IPv4 AH are unchanged. Other extension headers (e.g. HOPOPT) have the same gap and are not changed here.

  • test_ah_unit.py and test_ipv6_ext_unit.py (AH, IPv6-extension, ESP membership): 44 passed

  • examples/generators/make_samples.py runs

  • isort --check -l100 -p pcapkit clean on changed files

@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 31, 2026
@JarryShaw JarryShaw mentioned this pull request Oct 31, 2026
Closed
@JarryShaw

JarryShaw commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES (Sonnet, 120e15521). Root cause and the four new behaviour tests verified; 80 of 1105 cells differ, all on the four extension-mode AH layers.

  • ah.py # NOTE: is 4 lines; cut to 2. Drop "and payload": ah.payload still raises UnsupportedCall in extension mode.
  • test_ah_unit.py AHMembershipTests docstring: "used to leave _next unset" is timed; state what holds now.
  • PR body: use the repo template (checklist and description sections); tick only what ran.

@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 31, 2026
@JarryShaw

JarryShaw commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Held: Actions queue at 40 (at the hold line). Fix round dispatches when the queue drops below 40.

@JarryShaw
JarryShaw force-pushed the fix/2484-ah-membership-attributeerror branch from 120e155 to de622ab Compare October 31, 2026 03:15
@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 31, 2026
@JarryShaw JarryShaw mentioned this pull request Oct 31, 2026
Merged
4 of 8 tasks
@JarryShaw

JarryShaw commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES (Sonnet, de622abc7). Behaviour verified: the NOTE is 2 lines, and the two behaviour tests fail on origin/main with the AttributeError.

  • test_ah_unit.py AHMembershipTests docstring: "False for any selector but its own" is wrong; 'IPsec' in ah and 'IPv6_Ext' in ah are True. Say it is False for a selector that does not match AH.
  • PR body: restore the changelog line's "under docs/source/changelog/ and regenerated CHANGELOG.md" text, put "N/A" on the box, and add the --- before Description. Keep 15 lines by dropping the trailing N/A line.

@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 31, 2026
@JarryShaw
fix(protocols): set NoPayload on terminal AH extension layer
566fd36 
- AH in extension mode returns before _decode_next_layer, so it never set
  _next. Protocol.__contains__ reads _next, so 'ESP' in an AH taken from
  IPv6.extension_headers raised AttributeError.
- The layer now holds NoPayload, as read() gives an undecoded layer: the
  upper layer belongs to the enclosing IPv6 header, so membership walks
  nothing past AH and returns False.
- Tests build the frames in memory: AH membership for non-matching
  selectors, ESP after AH, ESP extension layer and IPv4 AH are unchanged.
@JarryShaw
JarryShaw force-pushed the fix/2484-ah-membership-attributeerror branch from de622ab to 566fd36 Compare October 31, 2026 03:33
@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 31, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 31, 2026
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet labels Oct 31, 2026
@JarryShaw

JarryShaw commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Sanity check, this applies to all other IPv6 extension headers when being used as extension headers already?

@JarryShaw

JarryShaw commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Yes. The same early-return gap is on the other extension headers: HOPOPT, IPv6_Opts, IPv6_Route, IPv6_Frag, HIP, MH, and the Shim6 generic fallback. That fix is #2488 (abbc6ca2e, in review); this PR covers AH only.

@github-actions

github-actions Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Coverage: 94.50% (unit tier, Python 3.15, 566fd36e2, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19677 133 2628 120 98.85%
pcapkit/corekit 2605 74 818 37 96.52%
pcapkit/dumpkit 281 3 100 3 98.43%
pcapkit/foundation 3070 24 1072 26 98.79%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 18054 194 4784 180 98.35%
pcapkit/toolkit 924 36 334 10 95.55%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4449 1854 1020 167 55.11%

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

@JarryShaw
JarryShaw merged commit 2b5ed38 into main Oct 31, 2026
61 checks passed
@JarryShaw
JarryShaw deleted the fix/2484-ah-membership-attributeerror branch October 31, 2026 05:19
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 31, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

PyPCAPKit
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Protocol membership raises AttributeError on a terminal AH layer

1 participant

@JarryShaw