Skip to content

fix(pcapng): keep out-of-range if_tzone as captured and catch every fromtimestamp failure (#1340) - #1429

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1340-pcapng-tzone-tsoffset
Oct 8, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1340-pcapng-tzone-tsoffset

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Value origin/main this PR
if_tzone ±86400, −2³¹, 2³¹−1 ValueError from datetime.timezone kept as a timedelta + ProtocolWarning; packets render in UTC; IDB rebuilds byte-exact
if_tzone ±86399 timezone unchanged
if_tsoffset ±2⁶², −2⁶³ OSError existing invalid timestamp warning, epoch-0 datetime, epoch Decimal kept
if_tsoffset 2⁶³−1 OverflowError same as above

This follows the existing _read_timestamp fallback and the "kept as captured" precedent (#1412, #1425). A ProtocolError would drop the whole IDB and could not round-trip. IF_TZoneOption.timezone is widened to timezone | timedelta. The timedelta only appears for values that used to raise, so this is not marked breaking.

The new tests/protocols/misc/test_pcapng_tzone_tsoffset_range_1340_unit.py has 5 tests and 26 subtests, covering the min, max and just-out-of-range values plus the largest EPB timestamp on top of a ±2⁶³ offset. Without the fix it fails 12 (6 ValueError, 4 OSError, 2 OverflowError). Legs: each tests/protocols/misc module run separately, all passed; run_unittest_leg.py protocols --exclude protocols/internet 1081 OK; tests/project 404 passed.

Closes #1340

…romtimestamp failure (#1340)

- An if_tzone of 24 hours or more, which no datetime.timezone can hold, is
  kept as captured as a timedelta with a ProtocolWarning instead of raising
  ValueError; the interface's timestamps render in UTC, and the option
  rebuilds byte-exactly.
- _read_timestamp now falls back to the epoch with its existing warning on
  OverflowError and OSError as well as ValueError, so an extreme if_tsoffset
  no longer escapes from datetime.fromtimestamp.

New tests/protocols/misc/test_pcapng_tzone_tsoffset_range_1340_unit.py.
@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 8, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at fba8a2b59 (Sonnet cross-review; the author ran on Opus).

  • Tests bite, re-derived by me: the new module passes 5/5 (26 subtests) on this head. With only the two pcapng source files put back to main, 12 subtests error.
  • if_tzone:
    • On main, −2³¹, ±86400, ±86401 and 2³¹−1 raise ValueError. On this head they are kept as a timedelta with a ProtocolWarning, and the IDB rebuilds byte-exact in both byte orders.
    • ±86399, ±3600 and 0 are unchanged, with no warning.
  • UTC fallback is right: datetime.timezone cannot hold ≥24h, and if_tzone only affects rendering. The instant and the Decimal epoch are unchanged, and later EPBs rebuild byte-exact.
  • if_tsoffset:
    • On main, −2⁶³ and ±2⁶² raise OSError and 2⁶³−1 raises OverflowError, each also tried with the max EPB timestamp. On this head each warns, falls back to epoch-0 UTC and rebuilds byte-exact.
    • Catching OSError is safe: the try at pcapng.py:1742-1746 holds only the division and fromtimestamp.
  • Consumers: _make_option_if_tzone and the dumper (dumpkit/common.py:430) already handle a timedelta. extract to json, tree and plist succeeds on these files.
  • Not breaking: a timedelta only appears for inputs that used to raise.
  • Neighbours: 16 tests/protocols/misc modules, the dpkt pcapng interfaces test and tests/project (404 passed) all pass.
  • Out of scope, checked: pcap/frame.py:271 is not reachable. ts_sec and ts_usec are UInt32Field, so the epoch tops out at 2106-02-07, which is within range.

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

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Coverage: 89.45% (unit tier, Python 3.14, fba8a2b59, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18791 968 2352 869 90.90%
pcapkit/corekit 2038 89 634 25 94.76%
pcapkit/dumpkit 234 2 84 2 98.74%
pcapkit/foundation 2637 98 928 48 94.78%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 16621 221 4474 185 98.04%
pcapkit/toolkit 539 74 168 3 84.58%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2342 1006 157 43.25%

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

@JarryShaw
JarryShaw merged commit ec9bcad into main Oct 8, 2026
41 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 8, 2026
@JarryShaw
JarryShaw deleted the fix/1340-pcapng-tzone-tsoffset branch October 8, 2026 22:25
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 8, 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(pcapng): out-of-range if_tzone and if_tsoffset raise raw ValueError, OSError and OverflowError

1 participant