Skip to content

fix(toolkit): a copied DecimalTimestamp keeps orig_len under _pydecimal (#1647) - #1653

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1647-decimaltimestamp-copy-on-315
Oct 13, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1647-decimaltimestamp-copy-on-315

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

Description of your pull request and other information

Cause. copy.copy and copy.deepcopy look up __copy__ and __deepcopy__ before __reduce_ex__ (Lib/copy.py:80, :136 at v3.15.0), and Decimal defines both. The C _decimal returns self. _pydecimal returns self.__class__(str(self)) (Lib/_pydecimal.py:3716-3724), so the clone takes the constructor's defaults. 3.15 "Removed implicit fallback to the bundled copy of the libmpdec library" (What's New, Build changes). The actions/setup-python 3.15.0 build has no _decimal (MODULE__DECIMAL_STATE: 'missing'), so only that leg runs on _pydecimal. Pickle was never affected, because it reaches the class's own __reduce__.

Fix. RecordTimestamp answers __copy__, __deepcopy__ and __reduce_ex__ from each subclass's __reduce__, and comes before the numeric base in the MRO (pcapkit/toolkit/dpkt.py:122-134). Its own __reduce__ raises NotImplementedError.

Tests. New tests/toolkit/test_dpkt_timestamp_copy_1647_unit.py runs both classes through copy, deepcopy and pickle protocols 0..HIGHEST, in process and under _pydecimal in a fresh interpreter. Without the fix, 5 tests fail on 3.15, 3.14 and 3.11 (resolution 1000000 → 1000000000, orig_len 78 → None). With it, 4 pass (37 subtests) on each. #1645's failing test also passes with _decimal blocked.

Closes #1647

…al (#1647)

- copy.copy and copy.deepcopy ask for __copy__ and __deepcopy__ before
  __reduce_ex__, and decimal.Decimal defines both. The C _decimal returns
  self; the pure-Python _pydecimal returns self.__class__(str(self)), so a
  DecimalTimestamp came back with the default resolution and no orig_len.
  CPython 3.15 dropped its fallback to the bundled libmpdec, and the
  actions/setup-python 3.15.0 build has no _decimal, so only that leg failed.
- RecordTimestamp now answers __copy__, __deepcopy__ and __reduce_ex__ from
  each subclass's own __reduce__, ahead of the numeric base in the MRO; its
  own __reduce__ raises NotImplementedError rather than guessing.

New tests/toolkit/test_dpkt_timestamp_copy_1647_unit.py round-trips both
classes through copy, deepcopy and pickle protocols 0..HIGHEST, in process
and under _pydecimal in a fresh interpreter. Without the fix 5 fail on
3.15, 3.14 and 3.11; with it, 4 passed (37 subtests) on each.
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 13, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at a3f779f93. Sonnet cross-review re-derived the cause from the v3.15.0 sources and the CI log. 3.15 dropped the bundled libmpdec fallback, so CI's setup-python 3.15 has no C _decimal. _pydecimal's __copy__/__deepcopy__ then rebuild a subclass from its value alone, so orig_len is lost; pickle was never affected. I re-ran the new test and test_engine_pcap_trace_runtime.py myself on 3.15.0, 3.14 and 3.11: all green. The fix stays inside RecordTimestamp and merges cleanly with the open dpkt work.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.67% (unit tier, Python 3.14, a3f779f93, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18858 954 2366 868 91.06%
pcapkit/corekit 2366 67 760 35 96.48%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 2884 145 1010 35 94.14%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17166 226 4580 191 98.06%
pcapkit/toolkit 1046 166 386 5 82.61%
pcapkit/utilities 494 4 146 4 98.75%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw
JarryShaw merged commit 1c5efaf into main Oct 13, 2026
38 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 13, 2026
@JarryShaw
JarryShaw deleted the fix/1647-decimaltimestamp-copy-on-315 branch October 13, 2026 14:45
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 13, 2026
JarryShaw added a commit that referenced this pull request Oct 14, 2026
Third-party engines are best effort (#1687), so the dpkt code that exists
only to match the default engine comes out:

- revert #1662, #1655, #1624, #1622, #1614 and the dpkt hunks of #1605:
  the octet walk and its link-type stash, IPv6 in IPv4 and IPv4 in IPv6,
  refusing MPLS and ISL, the tunnel and extension-header walks, the IPv4
  header checks and the layer bounds
- keep #1690's timestamps, #1594's parser choice, #1621 and #1653
- re-land as guards: a layer dpkt's own lengths misplace (#1646) is
  re-serialised, not refused with ProtocolError; a Total Length below the
  IHL keeps the header whole (#1616), and a header the capture cuts is no
  input; packet2dict and packet2chain take a str payload and a raw frame
- delete the modules #1662 and #1655 added; the 1615/1616 module keeps
  only what its #1616 tests use, as no scapy test imports it now; dpkt's
  comparisons with the default engine assert constants or the capture's
  octets, or are dropped
- drop pypcapfile's VLAN_TAGS, VLAN_TAG_LEN and IPV6_EXTENSION_HEADERS,
  which only dpkt imported, and the 1668 module's _Agreement base, which
  nothing subclasses now
- the docs drop the depth claims and name what dpkt leaves out silently

The dpkt modules, the shared modules that run dpkt, tests/foundation/engines
and the project guards pass on 3.14 and 3.11 (dpkt 1.9.8, pypcapfile 0.12.0).
JarryShaw added a commit that referenced this pull request Oct 14, 2026
Third-party engines are best effort (#1687), so the dpkt code that exists
only to match the default engine comes out:

- revert #1662, #1655, #1624, #1622, #1614 and the dpkt hunks of #1605:
  the octet walk and its link-type stash, IPv6 in IPv4 and IPv4 in IPv6,
  refusing MPLS and ISL, the tunnel and extension-header walks, the IPv4
  header checks and the layer bounds
- keep #1690's timestamps, #1594's parser choice, #1621 and #1653
- re-land as guards: a layer dpkt's own lengths misplace (#1646) is
  re-serialised, not refused with ProtocolError; a Total Length below the
  IHL keeps the header whole (#1616), and a header the capture cuts is no
  input; packet2dict and packet2chain take a str payload and a raw frame
- delete the modules #1662 and #1655 added; the 1615/1616 module keeps
  only what its #1616 tests use, as no scapy test imports it now; dpkt's
  comparisons with the default engine assert constants or the capture's
  octets, or are dropped
- drop pypcapfile's VLAN_TAGS, VLAN_TAG_LEN and IPV6_EXTENSION_HEADERS,
  which only dpkt imported, and the 1668 module's _Agreement base, which
  nothing subclasses now
- the docs drop the depth claims and name what dpkt leaves out silently

The dpkt modules, the shared modules that run dpkt, tests/foundation/engines
and the project guards pass on 3.14 and 3.11 (dpkt 1.9.8, pypcapfile 0.12.0).
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): a copied or unpickled DecimalTimestamp loses orig_len on Python 3.15

1 participant