Repository navigation
fix(toolkit): dpkt and pypcapfile hand over the correctly rounded timestamp (#1668) - #1690
Merged
Merged
Conversation
…estamp (#1668) dpkt on a microsecond PCAP, and pypcapfile on any PCAP, handed reassembly and flow tracing ts_sec + frac / divisor, which rounds twice: at ts_sec=1 it gave 1.0036909999999999 for 1.003691, so the inputs, the flow label and the trace file name differed from the default engine's. - dpkt: the four adapters read a RecordTimestamp's exact value; any other timestamp is handed on as before. The reader, and so a stored frame's value, is unchanged, but it now computes a microsecond PCAP's exact value at 64 digits, as the default engine does, not in the caller's decimal context. - pypcapfile: packet2timestamp joins the halves in one int division. - tests: captures built at ts_sec=1 (3691 us, 3691000 ns) and 415-frac sweeps, and a microsecond PCAP at ts_sec=1 and 1500000000 read at a decimal precision of 10 and 12; both engines' inputs and trace names equal the default's.
JarryShaw
added
fix
Owner
Author
|
Verdict: GOOD TO GO at |
Contributor
|
Coverage: 89.47% (unit tier, Python 3.14,
Per-file detail: the |
JarryShaw
removed
the
review: good-to-go
This was referenced Oct 14, 2026
JarryShaw
added a commit
that referenced
this pull request
Oct 14, 2026
…1721) Third-party engines are best effort (#1687), so the pypcapfile code that exists only to match the default engine comes out: - revert #1682, #1600, #1575 and #1535, and the pypcapfile hunks of #1655, #1608 and #1605: reading loopback and raw-IP link types, the frame layer bound, the VLAN and tunnel walks, keying tunnelled TCP by the innermost header, stepping over AH and extension headers, and checking options with the default engine's parsers - keep the helpers the dpkt and scapy adapters import, unchanged, until their own reverts drop them; their docstrings no longer say pypcapfile asks the default engine's parsers too - re-land #1576, #1577, #1590 and #1591 as guards in _network and _transport, #1513's once-per-capture warning for TCP straight over IPv6, and #1667's packet2dict fix: an undecoded frame is reported as its octets, not as hex text twice as long - keep #1569's record reader and #1690's packet2timestamp - delete the three test modules those reverts added, the pypcapfile timestamp agreement class, and #1682's link-type warning test; the re-landed guards' tests assert constants - drop what the reverts removed from the docs, READ_LINKTYPES included, and say that TCP behind a VLAN tag, in a tunnel or behind an extension header (an IPv6 one, or AH in IPv4) is now left out without a warning The pypcapfile modules, test_new_engine_parity_runtime.py, test_engine_runtime.py, the timestamp module and the project guards pass on 3.11 (pypcapfile 0.12.0) and 3.14.
3 of 13 tasks
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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please follow the guide below
You will be asked some questions, please read them carefully and answer honestly
Put an
xinto 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) — run on the changed files; no new findingsmake testpasses, and a test case covers the change — new module added; related modules run, not the whole suiteAdded a changelog entry under
docs/source/changelog/and regeneratedCHANGELOG.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) #657What is the purpose of your pull request?
Tick the commit type your subject line carries.
fix— corrects a defectfeat— adds a featureperf— changes performance, not behaviourrefactor— changes neither behaviour nor performancetest— tests onlydocs— documentation onlyci— workflows or build toolingrelease— bumps the version or rolls up a distributionchore— anything elseDescription of your pull request and other information
dpkt on a microsecond PCAP, and pypcapfile on any PCAP, handed reassembly and flow tracing
ts_sec + frac / divisor. That rounds twice, so atts_sec=1they gave1.0036909999999999where the default engine gives1.003691. The flow label and trace file name were off by the same amount._capture_time, which returnsfloat(timestamp.exact)for aRecordTimestampandfloat(timestamp)for anything else, so a plain float no longer risks anAttributeError. A stored frame keeps dpkt's own value, as theTimestampdocstring says. The reader now computes a µs PCAP's exact value underlocalcontext(prec=64), as the default engine does: in the caller's context, a precision of 12 gave1500000000.12for1500000000.123456.packet2timestampcomputes(sec * divisor + frac) / divisor, which rounds once.tests/toolkit/test_dpkt_pypcapfile_timestamp_rounding_1668_unit.pybuilds its captures withstructatts_sec=1: frac 3691 µs and 3691000 ns, plus 415-frac seeded sweeps with 62 µs and 66 ns double-rounding cases, and a µs PCAP atts_sec=1and1500000000read at decimal precision 10 and 12. They assert that the IPv4/IPv6/TCP/trace inputs and trace file names equal the default engine's, for dpkt on PCAP and PCAP-NG and for pypcapfile on PCAP. A stand-in check ofpacket2timestampruns without pypcapfile. Every test method that runs fails onorigin/main(23 subtest failures on 3.11.15; 17 on 3.14.8, where the pypcapfile engine class skips becausepcapfilecannot be imported) and passes with the fix.1.003691and write…_9-1.003691.json, sweeps included._upstreamreader fallback (it warnsDPKTWarning) still yields dpkt's own float on a µs PCAP, the same as onmain. dpkt on a ns PCAP, and scapy on PCAP and PCAP-NG, already lose digits at a low decimal precision onmain; that is fix(engines): dpkt on a nanosecond PCAP and scapy truncate the timestamp under a low decimal precision #1689.Closes #1668