Skip to content

fix(pcap): Header.pack() writes the same magic for every endian/nanosecond setting; _make_data returns magic_number - #1143

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1125-pcap-header-magic
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1125-pcap-header-magic

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) — Makefile flags run on header.py only: isort clean, mypy 0 errors in the file, pylint only the existing W0201 on _byte/_nsec
  • make test passes, and a test case covers the change — the modules listed below, not the full suite
  • Added a changelog entry — 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 #1125. Header._make_data returned magic_number, which make() does not accept. from_data dropped it (with an UnknownFieldWarning) and rebuilt every header as the host default, d4c3b2a1. It now returns byteorder and nanosecond. Calling pack() with no arguments rebuilds from make() defaults for every protocol (Ethernet(src=...).pack() does the same), so I left that alone.

Probe: Header.from_data(Header(**kw).info).data == Header(**kw).data, with four byteorder×nanosecond cases plus the lilendian/bigendian flags. Before: only the d4c3b2a1 cases match, and every call warns. After: all 8 match with no warnings, and big-endian packs a1b2c3d4 00020004.

The new tests/protocols/misc/pcap/test_header_make_data_magic_unit.py passes 2 with the fix and fails 5 (incl. subtests) with it reverted. I also deleted the recorded-mismatch test from test_construction_keyword_check_unit.py: its only entry was Header's. And test_header_frame_unit.py's stub MagicNumber now carries the byteorder/nanosecond fields the real one has.

Results: new module 2 passed · keyword_check 21 passed · header_magic_protos 3 passed · header_frame 6 passed · option_roundtrip 6 passed · tests/project 385 passed, 1 skipped.

…econd setting; _make_data returns magic_number

- Header._make_data returned the raw `magic_number`, which make() does not
  take, so from_data dropped it with a warning and rebuilt every header as
  the host-order microsecond default (d4c3b2a1 on little-endian hosts). It
  now returns `byteorder` and `nanosecond`, so from_data(Header(...).info)
  is byte-identical for all four magics.
- A no-argument pack() rebuilding from make() defaults is the contract of
  every protocol (Ethernet behaves the same), so it is left as is.
- Drop the Header entry, the table's only one, from the recorded
  _make_data mismatches in test_construction_keyword_check_unit.
- test_header_frame_unit's stub MagicNumber gains the byteorder and
  nanosecond fields the real one carries.

Closes #1125
@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 ae5fffc90: GOOD TO GO (ran on Sonnet; author Opus)

  • The issue's part 1 is a misreading. ProtocolBase.pack() calls make(**kwargs) and ignores the instance's state. A bare pack() therefore rebuilds from make() defaults for every protocol, not just Header. Ethernet(src=…).pack() gives zeros too. This is an undocumented behaviour affecting all protocols, not a Header defect, so I have noted it for the docs: #719-style prose sweep after the #1090–#1102 defect wave #1103 docs sweep.
  • The real fix: _make_data now returns byteorder and nanosecond instead of the magic_number key that make() drops. All 8 constructions now round-trip with no warnings. On main, 3 of 8 do, and every call warns.
  • Tests:
    • with header.py reverted, the new module fails 5;
    • the deleted recorded-mismatch test has no references left;
    • test_construction_keyword_check_unit (21) and test_header_frame_unit (6) pass.

The single-flag cases pass only because both sides ignore a lone flag. That is #1144.

@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.75% (unit tier, Python 3.14, ae5fffc90, 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 1873 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2425 145 842 34 92.56%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15659 188 3944 163 98.18%
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 355ec25 into main Oct 6, 2026
41 checks passed
@JarryShaw
JarryShaw deleted the fix/1125-pcap-header-magic branch October 6, 2026 22:35
@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(pcap): Header.pack() writes the same magic for every endian/nanosecond setting; _make_data returns magic_number

1 participant