Summary
pcapkit/corekit/fields/ipaddress.py lets a bare ValueError from the
stdlib ipaddress module escape when an address value is malformed, even
though the very next statement in the same method raises the library's own
FieldValueError for a value that is merely the wrong IP version. So a caller
cannot rely on pcapkit.utilities.exceptions.BaseError to catch a bad field
value: whether the exception is in-library depends on how the value is wrong.
Reproduction
Measured on 0283a6d59 (current main):
import ipaddress
from pcapkit.corekit.fields.ipaddress import IPv6AddressField
from pcapkit.utilities.exceptions import BaseError
f = IPv6AddressField()
try:
f.pre_process('not-an-address', {})
except Exception as e:
print(type(e).__module__, type(e).__name__, isinstance(e, BaseError))
# builtins ValueError False
try:
f.pre_process(ipaddress.IPv4Address('1.2.3.4'), {})
except Exception as e:
print(type(e).__module__, type(e).__name__, isinstance(e, BaseError))
# pcapkit.utilities.exceptions FieldValueError True
All four public field classes behave the same way for a malformed string —
IPv4AddressField, IPv6AddressField, IPv4InterfaceField and
IPv6InterfaceField each give builtins.ValueError, in-library=False.
Where
The two halves of the inconsistency are adjacent lines of the same method,
_IPAddressField.pre_process:
if isinstance(value, (ipaddress.IPv4Address, ipaddress.IPv6Address)):
ip = value # type: IPv4Address | IPv6Address
else:
ip = ipaddress.ip_address(value) # :71 -- bare ValueError escapes
if ip.version != self.version:
raise FieldValueError(...) # :74 -- in-library
FieldValueError(BaseError, ValueError) is defined at
pcapkit/utilities/exceptions.py:372, and the module already imports it at
ipaddress.py:9 — so the intent to raise an in-library error is present, the
unguarded conversion just predates or bypasses it.
The same unguarded-conversion pattern appears at these sites in that module:
:71 — _IPAddressField.pre_process, demonstrated reachable above
:186, :254 — _IPInterfaceField.pre_process (also caller-supplied,
demonstrated above via IPv4InterfaceField / IPv6InterfaceField)
:88 — _IPAddressField.post_process
:210, :211, :282 — interface post_process
I have not demonstrated a reachable failure for the post_process sites:
those take wire bytes whose length is fixed by the field, so a 4- or 16-octet
slice always converts. They are listed as the same pattern, not as verified
defects.
Why it matters
pcapkit documents its own exception hierarchy and raises from it everywhere
else in this module, so except BaseError is the natural way to handle a bad
field value. Here it silently misses the malformed-address case, and the
distinction is invisible from the call site.
Suggested direction
Wrap the conversions and re-raise as FieldValueError, preserving the original
message. Note that FieldValueError subclasses ValueError, so any
surrounding except ValueError must put except FieldValueError: raise (or the
BaseError equivalent) first, or it will re-wrap the library's own
exception and lose the original detail — the same ordering trap that
ProtocolError (exceptions.py:356) already carries.
Provenance
Surfaced while verifying #464 (the fix for #448). Not a regression from that
PR — I measured both trees. On main an invalid MN-ID identifier builds a
schema with a wrong declared length and then dies at pack() with
builtins.ValueError; on #464 it raises ipaddress.AddressValueError (also a
ValueError subclass, also not in-library) earlier and more clearly. #464 makes
the failure better, not worse, and this issue is filed separately rather than
folded into it.
Same family as #438 (bare struct.error, closed) and #458 (bare EOFError,
open).
Summary
pcapkit/corekit/fields/ipaddress.pylets a bareValueErrorfrom thestdlib
ipaddressmodule escape when an address value is malformed, eventhough the very next statement in the same method raises the library's own
FieldValueErrorfor a value that is merely the wrong IP version. So a callercannot rely on
pcapkit.utilities.exceptions.BaseErrorto catch a bad fieldvalue: whether the exception is in-library depends on how the value is wrong.
Reproduction
Measured on
0283a6d59(currentmain):All four public field classes behave the same way for a malformed string —
IPv4AddressField,IPv6AddressField,IPv4InterfaceFieldandIPv6InterfaceFieldeach givebuiltins.ValueError,in-library=False.Where
The two halves of the inconsistency are adjacent lines of the same method,
_IPAddressField.pre_process:FieldValueError(BaseError, ValueError)is defined atpcapkit/utilities/exceptions.py:372, and the module already imports it atipaddress.py:9— so the intent to raise an in-library error is present, theunguarded conversion just predates or bypasses it.
The same unguarded-conversion pattern appears at these sites in that module:
:71—_IPAddressField.pre_process, demonstrated reachable above:186,:254—_IPInterfaceField.pre_process(also caller-supplied,demonstrated above via
IPv4InterfaceField/IPv6InterfaceField):88—_IPAddressField.post_process:210,:211,:282— interfacepost_processI have not demonstrated a reachable failure for the
post_processsites:those take wire bytes whose length is fixed by the field, so a 4- or 16-octet
slice always converts. They are listed as the same pattern, not as verified
defects.
Why it matters
pcapkitdocuments its own exception hierarchy and raises from it everywhereelse in this module, so
except BaseErroris the natural way to handle a badfield value. Here it silently misses the malformed-address case, and the
distinction is invisible from the call site.
Suggested direction
Wrap the conversions and re-raise as
FieldValueError, preserving the originalmessage. Note that
FieldValueErrorsubclassesValueError, so anysurrounding
except ValueErrormust putexcept FieldValueError: raise(or theBaseErrorequivalent) first, or it will re-wrap the library's ownexception and lose the original detail — the same ordering trap that
ProtocolError(exceptions.py:356) already carries.Provenance
Surfaced while verifying #464 (the fix for #448). Not a regression from that
PR — I measured both trees. On
mainan invalid MN-ID identifier builds aschema with a wrong declared length and then dies at
pack()withbuiltins.ValueError; on #464 it raisesipaddress.AddressValueError(also aValueErrorsubclass, also not in-library) earlier and more clearly. #464 makesthe failure better, not worse, and this issue is filed separately rather than
folded into it.
Same family as #438 (bare
struct.error, closed) and #458 (bareEOFError,open).