Skip to content

fix(corekit): reject a signed= that contradicts a field's fixed sign (#545) #549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JarryShaw merged 2 commits into from
Sep 20, 2026
+269 −15
Merged

fix(corekit): reject a signed= that contradicts a field's fixed sign (#545) #549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b95b919
fix(corekit): reject a signed= that contradicts a field's fixed sign …
JarryShaw Sep 20, 2026
a53f9a0
Merge branch 'main' into fix/545-contradicting-signed
JarryShaw Sep 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 94 additions & 15 deletions pcapkit/corekit/fields/numbers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import aenum

from pcapkit.corekit.fields.field import Field, NoValue
from pcapkit.utilities.exceptions import IntError
from pcapkit.utilities.exceptions import FieldValueError, IntError

__all__ = [
'NumberField',
Expand Down Expand Up @@ -38,12 +38,28 @@ class NumberField(Field[int], Generic[_T]):
length: Field size (in bytes); if a callable is given, it should return
an integer value and accept the current packet as its only argument.
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; :data:`None` defers to the
class-level ``__signed__``, which this class leaves unset and so
means unsigned.
byteorder: Field byte order.
bit_length: Field bit length.
callback: Callback function to be called upon
:meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.

Raises:
IntError: If no ``length`` is given and ``__length__`` fixes none either.
FieldValueError: If ``signed`` contradicts a sign already fixed by
``__signed__`` -- never from this class, which fixes none.

Notes:
A subclass such as :class:`UInt32Field` fixes the sign through
``__signed__``, so ``signed`` there is at best redundant. It used to be
discarded outright, in both directions, which meant
``UInt32Field(signed=True)`` handed back an unsigned field whose values
only looked wrong once the high bit was set -- see GitHub issue #545. A
contradicting value is now rejected instead; omitting it, or passing the
sign the class already fixes, stays legal.

"""

__length__ = None # type: Optional[int]
Expand All @@ -56,7 +72,7 @@ def bit_length(self) -> 'int':
return self._bit_length

def __init__(self, length: 'Optional[int | Callable[[dict[str, Any]], int]]' = None,
default: 'int | NoValueType' = NoValue, signed: 'bool' = False,
default: 'int | NoValueType' = NoValue, signed: 'Optional[bool]' = None,
byteorder: 'Literal["little", "big"]' = 'big',
bit_length: 'Optional[int]' = None,
callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None':
Expand All @@ -72,15 +88,35 @@ def __init__(self, length: 'Optional[int | Callable[[dict[str, Any]], int]]' = N
else:
self._bit_length, self._bit_mask = -1, -1

self._signed = signed if self.__signed__ is None else self.__signed__
# NOTE: ``__signed__`` fixes the sign for a subclass such as
# :class:`UInt32Field`, and used to *discard* the ``signed`` argument to
# do it -- in both directions, so ``UInt32Field(signed=True)`` returned
# an unsigned field and ``Int8Field(signed=False)`` a signed one, both
# without a word. ``None`` is what "not given" looks like, which is what
# lets a contradicting value be told apart from the default and rejected
# while leaving an agreeing one alone. See #545.
if self.__signed__ is None:
self._signed = False if signed is None else bool(signed)
elif signed is None or bool(signed) == self.__signed__:
self._signed = self.__signed__
else:
raise FieldValueError(
f'{type(self).__name__}: field is fixed as '
f'{"signed" if self.__signed__ else "unsigned"}, '
f'but signed={signed!r} was given'
)
self._byteorder = byteorder
self._need_process = False

endian = '>' if byteorder == 'big' else '<'
if self.__template__ is not None:
struct_fmt = self.__template__
else:
struct_fmt = self.build_template(self._length, signed)
# NOTE: ``self._signed``, not the ``signed`` argument. A subclass
# that fixes ``__signed__`` without also fixing ``__template__``
# would otherwise build its template from the argument and parse
# with the opposite sign to the one it declared.
struct_fmt = self.build_template(self._length, self._signed)
self._template = f'{endian}{struct_fmt}'

def __call__(self, packet: 'dict[str, Any]') -> 'Self':
Expand Down Expand Up @@ -194,12 +230,17 @@ class Int32Field(NumberField):
Args:
length: Field size (in bytes).
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; fixed as :data:`True` here, so a
contradicting :data:`False` is rejected rather than ignored.
byteorder: Field byte order.
bit_length: Field bit length.
callback: Callback function to be called upon
:meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.

Raises:
FieldValueError: If ``signed`` is given as :data:`False`, contradicting
the sign this class fixes.

"""

__length__ = 4
Expand All @@ -213,12 +254,17 @@ class UInt32Field(NumberField):
Args:
length: Field size (in bytes).
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; fixed as :data:`False` here, so a
contradicting :data:`True` is rejected rather than ignored.
byteorder: Field byte order.
bit_length: Field bit length.
callback: Callback function to be called upon
:meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.

Raises:
FieldValueError: If ``signed`` is given as :data:`True`, contradicting
the sign this class fixes.

"""

__length__ = 4
Expand All @@ -232,12 +278,17 @@ class Int16Field(NumberField):
Args:
length: Field size (in bytes).
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; fixed as :data:`True` here, so a
contradicting :data:`False` is rejected rather than ignored.
byteorder: Field byte order.
bit_length: Field bit length.
callback: Callback function to be called upon
:meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.

Raises:
FieldValueError: If ``signed`` is given as :data:`False`, contradicting
the sign this class fixes.

"""

__length__ = 2
Expand All @@ -251,12 +302,17 @@ class UInt16Field(NumberField):
Args:
length: Field size (in bytes).
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; fixed as :data:`False` here, so a
contradicting :data:`True` is rejected rather than ignored.
byteorder: Field byte order.
bit_length: Field bit length.
callback: Callback function to be called upon
:meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.

Raises:
FieldValueError: If ``signed`` is given as :data:`True`, contradicting
the sign this class fixes.

"""

__length__ = 2
Expand All @@ -270,12 +326,17 @@ class Int64Field(NumberField):
Args:
length: Field size (in bytes).
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; fixed as :data:`True` here, so a
contradicting :data:`False` is rejected rather than ignored.
byteorder: Field byte order.
bit_length: Field bit length.
callback: Callback function to be called upon
:meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.

Raises:
FieldValueError: If ``signed`` is given as :data:`False`, contradicting
the sign this class fixes.

"""

__length__ = 8
Expand All @@ -289,12 +350,17 @@ class UInt64Field(NumberField):
Args:
length: Field size (in bytes).
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; fixed as :data:`False` here, so a
contradicting :data:`True` is rejected rather than ignored.
byteorder: Field byte order.
bit_length: Field bit length.
callback: Callback function to be called upon
:meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.

Raises:
FieldValueError: If ``signed`` is given as :data:`True`, contradicting
the sign this class fixes.

"""

__length__ = 8
Expand All @@ -308,12 +374,17 @@ class Int8Field(NumberField):
Args:
length: Field size (in bytes).
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; fixed as :data:`True` here, so a
contradicting :data:`False` is rejected rather than ignored.
byteorder: Field byte order.
bit_length: Field bit length.
callback: Callback function to be called upon
:meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.

Raises:
FieldValueError: If ``signed`` is given as :data:`False`, contradicting
the sign this class fixes.

"""

__length__ = 1
Expand All @@ -327,12 +398,17 @@ class UInt8Field(NumberField):
Args:
length: Field size (in bytes).
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; fixed as :data:`False` here, so a
contradicting :data:`True` is rejected rather than ignored.
byteorder: Field byte order.
bit_length: Field bit length.
callback: Callback function to be called upon
:meth:`self.__call__ <pcapkit.corekit.fields.field.FieldBase.__call__>`.

Raises:
FieldValueError: If ``signed`` is given as :data:`True`, contradicting
the sign this class fixes.

"""

__length__ = 1
Expand All @@ -347,7 +423,9 @@ class EnumField(NumberField[Union[enum.IntEnum, aenum.IntEnum]]):
length: Field size (in bytes); if a callable is given, it should return
an integer value and accept the current packet as its only argument.
default: Field default value, if any.
signed: Whether the field is signed.
signed: Whether the field is signed; :data:`None` defers to the
class-level ``__signed__``, which this class leaves unset and so
means unsigned.
byteorder: Field byte order.
bit_length: Field bit length.
namespace: Field namespace (a :class:`enum.IntEnum` class).
Expand All @@ -357,7 +435,8 @@ class EnumField(NumberField[Union[enum.IntEnum, aenum.IntEnum]]):
"""

def __init__(self, length: 'int | Callable[[dict[str, Any]], int]',
default: 'StdlibEnum | AenumEnum | NoValueType' = NoValue, signed: 'bool' = False,
default: 'StdlibEnum | AenumEnum | NoValueType' = NoValue,
signed: 'Optional[bool]' = None,
byteorder: 'Literal["little", "big"]' = 'big',
bit_length: 'Optional[int]' = None,
namespace: 'Optional[Type[StdlibEnum] | Type[AenumEnum]]' = None,
Expand Down
Loading
Loading