Skip to content

fix(protocols): an IPv4 Total Length of 0 is TSO, and one inside the header is bogus (#1547) - #1554

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1547-ipv4-tso-total-length
Oct 10, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1547-ipv4-tso-total-length

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • 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 — tests added; ran tests/protocols/internet, tests/protocols/schema, tests/foundation/reassembly, tests/protocols/test_capture_roundtrip_runtime.py and tests/project/test_isort_clean.py, not the full suite

  • 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?

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

Description of your pull request and other information

Closes #1547. Follows the owner's ruling to match Wireshark (tshark 4.6.9, measured).

  • total_length == 0 is TSO. IPv4.read (ipv4.py:363) passed 0 - hdr_len to the next layer, so TCP fell back to Raw. The payload is now the rest of the captured frame with no trailer, as Wireshark ("presumed TSO") and dpkt read it. On tcp.pcap with the IPv4 lengths zeroed, the default engine now traces [(1, 2, 6, 7), (3, 4, 5)], as dpkt and scapy do.
  • 0 < total_length < hdr_len is bogus. tshark reports "Bogus IP length" and stops at eth:ethertype:ip. pcapkit now gives Ethernet:IPv4 with a NoPayload. No negative length reaches the next layer, and the octets after the header go to trailer.
  • Rebuilds. Both cases write total_length back unchanged. parse→rebuild, make→parse and from_data(info.to_dict()) are byte-exact, with the checksum fixed or stale. A total_length longer than the capture is unchanged; the test pins it.
  • The schema payload length (schema/internet/ipv4.py:736) now handles both cases explicitly, so it no longer depends on a negative length being read as "unset".

internet.py is untouched; the fix is local to IPv4. Not fixed here: IPv4 reassembly of a TSO datagram is still PARTIAL with no payload, in every engine, because the toolkit adapters pass tl=ipv4.len (0). That needs a follow-up outside this PR's files.

Tests: tests/protocols/internet/test_ipv4_tso_total_length_1547_runtime.py, which fails on main.

…header is bogus (#1547)

- A Total Length of 0, as TCP segmentation offload leaves it on the sending
  host, handed the next layer 0 - hdr_len octets, so TCP fell back to Raw and
  the flow was never traced. The payload is now the rest of the captured
  frame with no trailer, as Wireshark and dpkt read it.
- Any other Total Length shorter than the header is Wireshark's "Bogus IP
  length": no payload layer and no negative length reaching one; the octets
  after the header are kept as the trailer.
- Both write the Total Length back as it was, so parse -> rebuild,
  make -> parse and from_data(info.to_dict()) stay byte-exact. A Total
  Length past the capture is unchanged.

tests/protocols/internet, schema, foundation/reassembly and the capture
round trip pass.
@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 10, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 821aa1003. Cross-reviewed on Sonnet; the author ran Opus. The PR follows the owner's Wireshark ruling on #1547.

  • TSO (total_length 0): all four IPv4 frames of the zeroed tcp.pcap parse as IPv4:TCP. Their fields match tshark, and every rebuild is byte-exact with fixed and stale checksums. The default, dpkt and scapy engines now all trace [(1,2,6,7),(3,4,5)].
  • Bogus length (1, 10, 19): this matches tshark 4.6.9's "Bogus IP length" stop. The result is Ethernet:IPv4 plus NoPayload, with the octets in trailer and a byte-exact rebuild. That matches how a total_length equal to the header already behaves.
  • Sweep: 7,476 sample IPv4 frames rewritten to lengths 0, 1 and hdr_len−1 give 0 round-trip failures and agree with tshark throughout. The same sweep on main reports 2,458 warnings per mode.
  • Unchanged cases: a total_length past the capture, and make() with or without a length, are byte-identical to main. Reassembly and trace output on all 22 samples is identical to main.
  • Fail-before: on the new module I measured 6 passed with the fix and 22 failed with main's two files. Each file alone is needed: 10 tests fail without the ipv4.py change and 19 without the schema change.

Not blocking:

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

Copy link
Copy Markdown
Contributor

Coverage: 90.01% (unit tier, Python 3.14, 821aa1003, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18795 842 2356 756 92.09%
pcapkit/corekit 2204 67 712 34 96.26%
pcapkit/dumpkit 258 2 90 2 98.85%
pcapkit/foundation 2797 132 986 48 94.18%
pcapkit/interface 108 7 36 5 91.67%
pcapkit/protocols 16994 230 4548 191 98.02%
pcapkit/toolkit 635 98 198 5 83.31%
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 4e3f406 into main Oct 10, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1547-ipv4-tso-total-length branch October 10, 2026 03:37
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 10, 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(protocols): an IPv4 header with total_length 0 (TSO) parses its payload at length -20

1 participant