Repository navigation
fix(engines): the pypcapfile engine reads the PCAP records pypcapfile drops (#1512) - #1569
Conversation
|
Verdict: NEEDS CHANGES at Blocking:
Also folded in here: pypcapfile stops at a zero-length record, filed as #1571. I measured it on Confirmed:
|
|
Coverage: 89.63% (unit tier, Python 3.14,
Per-file detail: the |
… drops (#1512) pypcapfile 0.12.0's _read_a_packet drops records the default engine reads: it tests `hdrp[0].byteorder == 'big'`, bytes against str, so a big-endian savefile ends at its first record (#1512); and it ends the savefile at an empty or short frame, so a zero-length record drops itself and every later one, and a truncated final record is lost (#1571). - PyPCAPFile.run: pypcapfile still loads the header; the records are read by the new _read_records, never by pypcapfile's generator. - PyPCAPFile._read_records: reads each record in the header's byte order into pypcapfile's own pcap_packet, as _read_a_packet builds it with layers=0; stops at a tail shorter than a record header, and keeps a record whose capture length runs past the data, with a ProtocolWarning, as the default engine does. - PyPCAPFile._decode: a zero-length frame is left undecoded, unwarned. - tests: both byte orders, over stand-ins and the real pypcapfile; the #1512 Gap row is deleted; test_core's pypcapfile stand-in reads the header and carries a ctypes header, as the real one does. - docs: the engine note and Internal Definitions cover the reader.
8722317 to
67d20e9
Compare
|
Verdict: GOOD TO GO at
Not this PR's: the review saw the default engine read only part of a capture from a non-seekable stream. confirmed and filed as #1586. |
…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.
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)make testpasses, and a test case covers the change — every module that mentions pypcapfile, plustests/interface, on 3.14 and 3.11, 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
Closes #1512. Closes #1571.
savefile.py):_read_a_packettestshdrp[0].byteorder == 'big'(:214). The field reads back asb'big'(:98), so a big-endian record header is unpacked little-endian (:217) and the file ends at its first record.:221-222ends the file at any empty or short frame: a zero-length record drops itself and everything after it, and a truncated final record is lost._read_records, into pypcapfile's ownpcap_packetbuilt as_read_a_packetbuilds it withlayers=0. It stops at a tail shorter than 16 octets, and keeps a record that runs past the data with aProtocolWarning, asFrame.readdoes._decodeleaves a zero-length frame undecoded and unwarned.defaultfor zero-length middle/first and truncated-final records in both byte orders, and for both big-endian samples. The fix(engines): pypcapfile engine reads zero frames from a big-endian PCAP, without warning #1512Gaprow is deleted; no row cited fix(engines): the pypcapfile engine stops reading at a zero-length PCAP record #1571.main.test_pypcapfile_engine.py, stand-ins (3.14 and 3.11) plus a real-pypcapfile check against an independentstructreader (3.11). Onmain, 12 stand-in subtests fail on 3.14 and 14 real-pypcapfile subtests fail on 3.11.test_core.py's stand-in now consumes the header and carries a ctypes header, as the real one does. Engine coverage goes from 93.53% to 94.34%.