Skip to content

fix(toolkit): pypcapfile tcp_reassembly sets last one past the segment - #1108

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1100-pypcapfile-tcp-last
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1100-pypcapfile-tcp-last

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner
  • Searched for similar pull requests
  • Followed the coding style (make pylint, make mypy, make isort) — ran pylint/mypy/isort directly on the touched files: isort clean; pylint/mypy messages are pre-existing patterns only (lazy pcapfile import, unused ignore at L617)
  • make test passes, and a test case covers the change — ran only the modules listed below
  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, added centrally after the wave

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Closes #1100. TCP_Packet.last is inclusive (foundation/reassembly/data/tcp.py); pypcapfile.tcp_reassembly now sets seqnum + len(payload) - 1, matching the scapy/pcap/pcapng/dpkt adapters. Other fields checked against pcap.py: unchanged.

  • Probe (21-byte payload, seq 1000): before last=1021, after last=1020.
  • New tests/toolkit/test_pypcapfile_tcp_last_unit.py stubs pcapfile.protocols.transport.tcp in sys.modules: 2 passed with fix, 2 failed with it reverted.
  • test_pypcapfile_unit.py's pypcapfile-gated test pinned the old 1021; now 1020 (skipped here, pypcapfile not installed).
  • test_pypcapfile_unit.py: 19 passed, 9 skipped. test_pcap_unit.py: 4 passed. tests/project: 379 passed, 1 skipped.

TCP_Packet.last is the inclusive sequence number of the last payload
octet. The pypcapfile adapter set it to seqnum + len(payload), one past
the segment; it now uses seqnum + len(payload) - 1, as the scapy, pcap,
pcapng and dpkt adapters do.

The pypcapfile-gated real-decoder test pinned the old value (1021) and
now expects 1020. A new stand-in test runs without pypcapfile installed.

Closes #1100
@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 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 86b366e6e: GOOD TO GO (reviewed on Sonnet; authored on Opus)

  • Fix: last = seqnum + len(payload) - 1. This matches pcap.py, pcapng.py, dpkt.py and scapy.py, and the other fields agree with pcap.py.
  • Reproduced against the real pypcapfile 0.12.0 decoders (in a throwaway venv): a 21-byte payload at seq 1000 gives last 1021 on main and 1020 on this PR.
  • Fake module: its attributes match real pypcapfile. data_offset is in bytes (4 * (b >> 4)), and the flag ints are wrapped in bool().
  • Fails without the fix: with the adapter reverted, the new module fails 2. The edited value of 1020 in the existing test is correct.

The real-library test cannot run on Python 3.12 or later, because pypcapfile 0.12.0 imports imp. The reviewer decoded the same segment by hand instead.

@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 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.76% (unit tier, Python 3.14, 86b366e6e, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1874 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2422 143 842 34 92.62%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15653 187 3942 162 98.19%
pcapkit/toolkit 487 65 144 1 85.42%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

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

@JarryShaw
JarryShaw merged commit b67f8ae into main Oct 6, 2026
41 checks passed
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 2026
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(toolkit): pypcapfile tcp_reassembly sets last one past the segment

1 participant