Skip to content

fix(protocols)!: keep pack() from rewriting the instance it is called on (#1769) - #1791

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1769-pack-fresh-header
Oct 15, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1769-pack-fresh-header

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

pack() assigned the header it built to the instance, so UDP(srcport=1234).pack() left schema.srcport at 0 while info.srcport kept 1234. 1.4.1.post2 did the same. Per the owner's choice (option A on #1769), pack() with no keywords now builds a fresh header and leaves a parsed or constructed instance unchanged.

  • protocol.py: once the instance exists, _packing_target() gives pack() a shallow copy, so neither the header nor state make() sets (TCP's connection flags) reaches the instance. Construction still keeps its header for unpack().
  • frame.py, pcapng.py, httpv2.py: their overrides had the same defect. PCAP-NG's copy also gets its own option counter, so re-packing a block with an only-one option no longer raises.
  • Docs: pack() states the contract, with versionchanged:: 1.5.0.
  • Breaking: code that relied on pack() re-syncing schema now has to call make() or reconstruct.
  • Bite: with pcapkit/ reverted the new module fails 17 tests; reverting each override fails its own test.

Closes #1769

… on (#1769)

- protocol.py: pack() assigned the header it made to the instance, so
  UDP(srcport=1234).pack() left schema.srcport at 0 while info kept 1234.
  Once the instance has been parsed or constructed, the new
  _packing_target() hands pack() a shallow copy, so neither the header nor
  what make() sets on the way (TCP's connection flags) reaches it. The
  construction call in __post_init__ still keeps its header for unpack().
- frame.py, pcapng.py: their pack overrides make on the same target;
  PCAPNG's copy gets its own option counter, so re-packing a block with an
  only-one option no longer raises.
- httpv2.py: the header is dropped only while constructing.
- Docs: pack() states the contract with a versionchanged; _packing_target
  and Frame.pack are documented.
- Tests: new unit module over Ethernet, IPv4, UDP, TCP, Frame, PCAPNG and
  HTTP/2; the MPTCP stale-flags case now calls make() directly.

Closes #1769
@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 15, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 4cef30e54. A Sonnet cross-review (author: Opus) checked f4a0384ca; this head adds only ! to the subject, and the tree is identical. It matches the owner's option A. Reverting each fix fails its own test. An AST scan and a vars() snapshot over 30 protocols found no override the fix missed. The returned bytes are unchanged. 1.4.1.post2 mutated too, which justifies the versionchanged.

@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 15, 2026
@JarryShaw
JarryShaw merged commit 36b82bb into main Oct 15, 2026
23 checks passed
@JarryShaw
JarryShaw deleted the fix/1769-pack-fresh-header branch October 15, 2026 22:16
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 15, 2026
@JarryShaw JarryShaw moved this to Done in PyPCAPKit Oct 15, 2026
JarryShaw added a commit that referenced this pull request Oct 15, 2026
…1803)

Entries:
- add #1791 (breaking: pack() leaves its instance unchanged), #1778
  (Protocol in protocol.__all__), #1781 (PortEnumField miss with a
  nameless proto), #1779 (vendor.ipx _path), #1780 (examples READMEs to
  Markdown, build leftovers) and #1790 (36 ambiguous cross-references)
- fold #1776 into the RPL 0x23 entry and #1381 into the dump-schema one

Accuracy:
- the benchmark and legacy_smoke READMEs are README.md since #1780
- EngineBase stays out of __all__, as #1016 settled
- the #575 peek calls get() only once the value resolves
- the #647 and #764 entries point at #775/#860, under which nothing mints
- the #861 entry names #869 and #874 for the EnumRegistry move

Figures:
- preamble: 685 citations, #340 to #1791
- process.rst pin: 298 entries; CHANGELOG.md regenerated

Refs #1103
JarryShaw added a commit that referenced this pull request Oct 15, 2026
Prose only: with every string expression statement blanked, each changed
module's AST is identical to origin/main's.

- RFC 959: the case-folding rule sits in Section 5.3, not 4.1, in the
  #896 and #903 modules (RFC text, line 2559).
- Counts: the release-gate mutation table's #966 block is six rows, not
  five; the comment now sums to the table's 21.
- pack(): the GH-452 HTTP docstring describes pack() as it is after
  #1791, building on _packing_target(); the MPTCP stale-flags case says
  make, which it calls.
- Line references re-measured against origin/main: 23 pointers in 14
  files, into protocol.py, frame.py, field.py, byteorder.py, the five
  UnsupportedCall sites, tcp.py, ipv6.py, extension_header.py, the
  pcapng schema, apptype.py and vendor/__main__.py.

Refs #1103
@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): ProtocolBase.pack() without keywords replaces the instance's header

1 participant