Repository navigation
fix(arp): prefix the hardware-address messages and reject a bool with ProtocolError (#2256) - #2257
Merged
Merged
Conversation
… ProtocolError (#2256) ARP._make_addr_resolve raised unprefixed, capitalised messages ("Invalid MAC address: ...", "Invalid hardware address: ..."), gave a wrong type the same message as a malformed string, and rejected a bool with a separate FieldValueError that "except ProtocolError" missed. Drop the bool check, as #2232 did for Ethernet and PCAP-NG: any address that is not a str, bytes or bytearray raises "ARP: invalid MAC address: must be a str, bytes or bytearray, not ..." ("hardware address" for a non-Ethernet htype), and a malformed string raises "ARP: invalid MAC address: ...". Accepted forms are unchanged. The 1.5.0 changelog entry for #2166 and the generated CHANGELOG.md are corrected to match. Test: TestARPHardwareAddressType pins the exact messages for bool, int, None, float and a malformed string on Ethernet and Fibre Channel.
JarryShaw
added
bug
Owner
Author
|
Review: GOOD TO GO at
|
Owner
Author
|
Ready to merge at
|
Contributor
|
Coverage: 91.56% (unit tier, Python 3.15,
Per-file detail: the |
JarryShaw
removed
the
review: good-to-go
This was referenced Oct 26, 2026
Merged
Closed
Closed
JarryShaw
added a commit
that referenced
this pull request
Oct 26, 2026
…2265) - #876 item 3: state why the no-else-raise pragma stays, not "the belief" - #652: the flagless HTTP/2 dump reads "Flags::0 [0]", as #648 renders it - #2172: section_context raises only on a block holding no section - add the missing #2222/#2232 MAC/EUI maker entry; cite #2257 on ARP - add the #1055/#2244 engine-tests and required-checks entry that #849's and #856's job descriptions now need - #800 names HTTP._guess_version, where the method lives; #631 states the old MANIFEST.in in the past tense - drop mentions of never-released names (pcapkit.utilities.timestamp, a test constant) - citation range runs to #2257; CHANGELOG.md regenerated
This was referenced Oct 27, 2026
JarryShaw
added a commit
that referenced
this pull request
Oct 27, 2026
) No ProtocolError in pcapkit.protocols.internet.esp carried the "<prefix>: <lowercase description>: ... {value!r}" form #2232 and #2257 gave Ethernet, PCAP-NG and ARP: none had an "ESP:" prefix, some put "ESP" mid-sentence, and most printed the value bare. Every message raised by _resolve, CipherSuite.get, IntegritySuite.get, SecurityAssociation (SPI, destination, AEAD plus integrity, ICV length, compute_icv, decrypt, encrypt, _split_key), ESPContext.register and ESP.make now reads "ESP: <description>: ...", with the offending value in !r form. SecurityAssociation.unavailable gives the same prefixed reason. The traceback quoted in the module docstring and esp.rst follows. Test: ESPErrorMessageTests pins each message whole; the HAS_CRYPTO gate count in test_tier_guard rises by its one gated test.
This was referenced Oct 27, 2026
Merged
JarryShaw
added a commit
that referenced
this pull request
Oct 27, 2026
IPv6_Ext's version gate raised "only valid for IPv6, got version=4" -- a
bespoke tail unlike the <prefix>: <lowercase description>: must be X, not
{value!r} form #2232/#2257 established, and it never named the version
that was wanted. The same method's docstring quoted the message verbatim
for the AH/ESP case, so it was stale by the same edit.
Message is now "invalid version: must be 6, not {version!r}".
Test: test_version_gate_message_is_in_the_house_form pins the message
whole on both routes into the gate -- a bare-bytes call and the
BytesIO/alias=/extension=True stream form.
JarryShaw
added a commit
that referenced
this pull request
Oct 27, 2026
…pe in option guards (#2398) #2300: the guards that reject a file's magic number or byte order had no PCAP:/PCAP-NG: prefix, unlike every other message in those modules, and PCAP's magic-number guard didn't say what it read. Four sites: schema/misc/pcap/header.py's magic_number_callback, misc/pcap/header.py's _make_magic (byte order unresolved/unknown), and two byte-order sites in schema/misc/pcapng.py that were the only two of nine messages in that file still missing PCAP-NG:. #2301: 128 PCAP-NG option guards in misc/pcapng.py ended in a full stop, unlike the house <prefix>: ... form (#2232, #2257), and the placement guard formatted self._type -- a plain int at make time, not the Enum_ BlockType it's cast to -- with bare {}, showing a raw number. A new _block_type_name() helper resolves either form through Enum_BlockType, idempotent for the enum case; applying the int form via .name directly would raise AttributeError, verified against pristine main. Also fixed: the if_tsresol/if_tzone option guards carried the identical full-stop defect, outside the issue's literal checklist but left would have made this consistency fix inconsistent with itself -- the file's full-stop count is now 0, which a new test asserts as an invariant. CHANGELOG.md regenerated (util/changelog_md.py) after correcting one docs/source/changelog/1.5.0.rst entry that quoted the old literal message. Tests: new tests/protocols/misc/test_capture_message_prefix_2300_2301_unit.py, 14 cases, 13 fail / 1 passes against pristine main (the one passing case is a characterisation pin of _type's runtime type, true on both trees).
JarryShaw
added a commit
that referenced
this pull request
Oct 27, 2026
…ne_name__ (#2399) The pcapkit.toolkit adapters put the engine name mid-sentence instead of leading with it, unlike the <prefix>: <lowercase description>: ... {value!r} form #2232/#2257 established, and hand-typed the name rather than reading it off the engine class's own __engine_name__. Each adapter module gains _ENGINE = <EngineClass>.__engine_name__ (plus the import), so a rename in pcapkit/foundation/engines/ cannot leave the adapter spelling the old name. Four modules, five sub-bugs: - pcap_ct.py/pypcap.py: "IPv4 reassembly is not supported by the pcap-ct engine: ..." -> "PCAP_CT: IPv4 reassembly is not supported: ..." (and the PyPCAP sibling). - pypcapfile.py: "invalid PyPCAPFile IPv4 address: 5" -> "PyPCAPFile: invalid IPv4 address: 5" (same for MAC address); the IPv6 UnsupportedCall and the "Frame N: TCP over IPv6 ..." warning get the same treatment. - pyshark.py: boolean-field, frame.time_epoch and no-frame-layer messages all reordered the same way. - scapy.py: the import-time warning reordered; the quoted package name is lowercased to 'scapy' to match the actual failing import and the sibling dependency warnings elsewhere in the codebase, which quote the installable name. The engine classes' toolkit imports are all function-local, so the new toolkit -> foundation.engines import introduces no cycle. Tests: new tests/toolkit/test_engine_message_prefix_2304_unit.py (9 tests) pins each message whole and asserts each module's _ENGINE is the engine class's own attribute. Existing pins in the four engines' test files updated for the new text.
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.
make testpasses, and a test case covers the change1.5.0.rstis corrected, andCHANGELOG.mdregeneratedWhat is the purpose of your pull request?
fix— bug fixDescription
Closes #2256. As ruled, this drops the
boolcheck on ARP's hardware-address path, as #2232 did for Ethernet and PCAP-NG.ARP._make_addr_resolve, a single type check runs before thehtypedispatch.str,bytesorbytearray, aboolincluded, raisesProtocolError: ARP: invalid MAC address: must be a str, bytes or bytearray, not 5. A non-Ethernethtypesays "hardware address".ARP: invalid MAC address: 'zz'.alias. ARP'saliasreads_acnm, which is unset whilemake()runs, as_make_proto_resolve's Notes already explain._make_proto_resolveand itsboolcheck stay, because its IP branches can coerce anint.TestARPHardwareAddressTypepins the exact messages for bool, int, None, float and a malformed string, on Ethernet and Fibre_Channel.origin/main, 12 failed (unprefixed messages, andFieldValueErrorfor a bool).tests/protocols/link, the docstring contract and the changelog readers all pass.