Skip to content

Commit fef87f3

Browse files
committed
fix(corekit): IPv4InterfaceField keeps non-netmask masks; OrderedMultiDict deepcopy keeps order
- IPv4InterfaceField.post_process read a hostmask-shaped mask (0.0.0.255) as a prefix length and packed a different netmask; a non-contiguous mask raised. A mask that is not a contiguous netmask now unpacks to an IPv4Interface subclass that keeps the four octets, and pre_process packs its "addr/a.b.c.d" string form octet for octet, so both the value and its str() repack unchanged. Equality and ordering use the mask (#1294). - OrderedMultiDict.deepcopy rebuilds from items(multi=True) instead of the inherited, key-grouping to_dict(flat=False); the werkzeug port of MultiDict is left verbatim (#1295). Closes #1294 Closes #1295
1 parent c9676c5 commit fef87f3

5 files changed

Lines changed: 342 additions & 29 deletions

File tree

‎pcapkit/corekit/fields/ipaddress.py‎

Lines changed: 133 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,106 @@ class _IPInterfaceField(_IPField[_IT]):
347347
"""
348348

349349

350+
class _RawMaskIPv4Interface(ipaddress.IPv4Interface):
351+
"""IPv4 interface whose mask is not a contiguous netmask.
352+
353+
Args:
354+
address: Dotted address and dotted mask, as ``'a.b.c.d/m.m.m.m'``.
355+
356+
:class:`ipaddress.IPv4Interface` can only hold a prefix length, so it
357+
reads a hostmask-shaped mask such as ``0.0.0.255`` as ``/24`` and rejects
358+
a mask such as ``255.0.255.0``. This class keeps the mask octets as given
359+
in :attr:`netmask`, so :meth:`IPv4InterfaceField.pre_process` writes them
360+
back unchanged. :func:`str`, :func:`repr` and :mod:`pickle` use the
361+
dotted ``address/mask`` form, which the field also packs octet for octet.
362+
363+
Equality and ordering use the mask too. An instance sorts as the stdlib
364+
sorts ``address/32``, then by mask, so the order stays total when mixed
365+
with :class:`ipaddress.IPv4Interface` values.
366+
367+
Such a mask has no prefix length, so :attr:`network`, :attr:`with_prefixlen`
368+
and the prefix length all describe the address alone (``/32``); only
369+
:attr:`netmask`, :attr:`hostmask`, :attr:`with_netmask` and
370+
:attr:`with_hostmask` carry the mask.
371+
372+
"""
373+
374+
def __init__(self, address: 'str') -> 'None':
375+
addr, _, mask = address.partition('/')
376+
super().__init__(addr)
377+
self.netmask = ipaddress.IPv4Address(mask)
378+
379+
@property
380+
def hostmask(self) -> 'IPv4Address':
381+
"""Bitwise inverse of :attr:`netmask`."""
382+
return ipaddress.IPv4Address(int(self.netmask) ^ 0xFFFFFFFF)
383+
384+
def __str__(self) -> 'str':
385+
return f'{self.ip}/{self.netmask}'
386+
387+
def __eq__(self, other: 'object') -> 'bool':
388+
if not isinstance(other, ipaddress.IPv4Interface):
389+
return NotImplemented
390+
return self.ip == other.ip and self.netmask == other.netmask
391+
392+
def __hash__(self) -> 'int':
393+
return hash((int(self.ip), int(self.netmask)))
394+
395+
def __lt__(self, other: 'Any') -> 'bool':
396+
if not isinstance(other, ipaddress.IPv4Interface):
397+
return ipaddress.IPv4Interface.__lt__(self, other)
398+
return _interface_order(self) < _interface_order(other)
399+
400+
def __le__(self, other: 'Any') -> 'bool':
401+
if not isinstance(other, ipaddress.IPv4Interface):
402+
return ipaddress.IPv4Interface.__le__(self, other)
403+
return _interface_order(self) <= _interface_order(other)
404+
405+
def __gt__(self, other: 'Any') -> 'bool':
406+
if not isinstance(other, ipaddress.IPv4Interface):
407+
return ipaddress.IPv4Interface.__gt__(self, other)
408+
return _interface_order(self) > _interface_order(other)
409+
410+
def __ge__(self, other: 'Any') -> 'bool':
411+
if not isinstance(other, ipaddress.IPv4Interface):
412+
return ipaddress.IPv4Interface.__ge__(self, other)
413+
return _interface_order(self) >= _interface_order(other)
414+
415+
416+
def _is_netmask(mask: 'IPv4Address') -> 'bool':
417+
"""Tell whether ``mask`` is a contiguous netmask (leading ones, then zeros).
418+
419+
Args:
420+
mask: Dotted mask.
421+
422+
Returns:
423+
:data:`True` for ``0.0.0.0`` through ``255.255.255.255`` with no gap
424+
in the ones, :data:`False` otherwise.
425+
426+
"""
427+
inverse = int(mask) ^ 0xFFFFFFFF
428+
return not inverse & (inverse + 1)
429+
430+
431+
def _interface_order(value: 'IPv4Interface') -> 'tuple[int, int, int, int]':
432+
"""Sort key for an IPv4 interface, consistent with ``==``.
433+
434+
Args:
435+
value: IPv4 interface, either from the stdlib or a
436+
:class:`_RawMaskIPv4Interface`.
437+
438+
Returns:
439+
The stdlib order (network address, netmask, address), with a third
440+
element that is ``0`` for a stdlib interface and ``1 + mask`` for a
441+
:class:`_RawMaskIPv4Interface`, whose stdlib network is
442+
``address/32``.
443+
444+
"""
445+
if isinstance(value, _RawMaskIPv4Interface):
446+
return int(value.ip), 0xFFFFFFFF, 1 + int(value.netmask), int(value.ip)
447+
return int(value.network.network_address), int(value.network.netmask), 0, int(value.ip)
448+
449+
350450
class IPv4InterfaceField(_IPInterfaceField[ipaddress.IPv4Interface]):
351451
"""IPv4 interface value for protocol fields.
352452
@@ -383,14 +483,30 @@ def pre_process(self, value: 'IPv4Interface | bytes | int | str', packet: 'dict[
383483
:func:`_reject_bool`), is not a valid IP interface, or is the
384484
wrong IP version for this field.
385485
486+
Notes:
487+
A string with a dotted mask that is not a contiguous netmask,
488+
such as ``'10.0.0.1/0.0.0.255'``, packs those mask octets as
489+
given -- the form :func:`str` gives a
490+
:class:`_RawMaskIPv4Interface` -- where :func:`ipaddress.ip_interface`
491+
would read a hostmask or reject the mask. Any other string goes
492+
through :func:`ipaddress.ip_interface`.
493+
386494
"""
387495
_reject_bool(value, 'invalid IP interface')
388496

389497
if isinstance(value, ipaddress.IPv4Interface):
390498
val = value
391499
else:
392-
with _reraise_as_field_value_error('invalid IP interface'):
393-
parsed = ipaddress.ip_interface(value)
500+
parsed = None # type: Optional[IPv4Interface | IPv6Interface]
501+
if isinstance(value, str) and '.' in value.partition('/')[2]:
502+
# a dotted mask that is not a netmask is kept as given, as
503+
# in :meth:`post_process`, rather than read as a hostmask
504+
with _reraise_as_field_value_error('invalid IP interface'):
505+
if not _is_netmask(ipaddress.IPv4Address(value.partition('/')[2])):
506+
parsed = _RawMaskIPv4Interface(value)
507+
if parsed is None:
508+
with _reraise_as_field_value_error('invalid IP interface'):
509+
parsed = ipaddress.ip_interface(value)
394510
if not isinstance(parsed, ipaddress.IPv4Interface):
395511
raise FieldValueError(f'IP version mismatch: {parsed.version} != {self.version}')
396512
val = parsed
@@ -410,26 +526,32 @@ def post_process(self, value: 'bytes', packet: 'dict[str, Any]') -> 'IPv4Interfa
410526
Processed field value.
411527
412528
Raises:
413-
FieldValueError: If the trailing four octets are not a valid
414-
dotted netmask, or if the resulting interface is the wrong IP
415-
version for this field. The leading four octets cannot
416-
actually fail here -- they are always exactly 4 octets, fixed
417-
by this field's length, and any such octet string is a valid
418-
address -- but the conversion is still wrapped for
419-
consistency with the rest of this module.
529+
FieldValueError: If the resulting interface is the wrong IP
530+
version for this field. The eight octets cannot actually fail
531+
to convert here -- they are always exactly 8 octets, fixed by
532+
this field's length, any four octets are a valid address, and
533+
a mask that is not a netmask is kept as given -- but the
534+
conversion is still wrapped for consistency with the rest of
535+
this module.
420536
421537
Notes:
422538
The trailing four octets are a dotted netmask, as written by
423539
:meth:`pre_process` -- not a prefix length as in
424-
:meth:`IPv6InterfaceField.post_process`.
540+
:meth:`IPv6InterfaceField.post_process`. When they are not a
541+
contiguous netmask, the value is a :class:`_RawMaskIPv4Interface`
542+
that keeps them as given, rather than an interface the stdlib
543+
would reinterpret (``0.0.0.255`` as the hostmask of ``/24``).
425544
426545
"""
427546
with _reraise_as_field_value_error('invalid IPv4 address'):
428547
ip = ipaddress.IPv4Address(value[:4])
429548
mask = ipaddress.IPv4Address(value[4:])
430549

431550
with _reraise_as_field_value_error('invalid IPv4 interface'):
432-
val = ipaddress.ip_interface(f'{ip}/{mask}')
551+
if _is_netmask(mask):
552+
val = ipaddress.ip_interface(f'{ip}/{mask}')
553+
else:
554+
val = _RawMaskIPv4Interface(f'{ip}/{mask}')
433555
if not isinstance(val, ipaddress.IPv4Interface):
434556
raise FieldValueError(f'IP version mismatch: {val.version} != {self.version}')
435557
return val

‎pcapkit/corekit/multidict.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,18 @@ def __eq__(self, other: 'Any') -> 'bool':
497497

498498
__hash__ = None # type: ignore[assignment]
499499

500+
def deepcopy(self, memo: 'Optional[dict]' = None) -> 'OrderedMultiDict[_KT, _VT]':
501+
"""Return a deep copy of this object.
502+
503+
Notes:
504+
:meth:`MultiDict.deepcopy` rebuilds from :meth:`to_dict` with
505+
``flat=False``, which groups the values by key. This rebuilds from
506+
``items(multi=True)`` instead, so the copy keeps the insertion
507+
order of interleaved keys. :meth:`__deepcopy__` reaches it too.
508+
509+
"""
510+
return self.__class__(copy.deepcopy(list(self.items(multi=True)), memo=memo))
511+
500512
def __reduce_ex__(self, protocol: 'SupportsIndex') -> 'tuple[type, tuple[list[tuple[_KT, _VT]]]]':
501513
return type(self), (list(self.items(multi=True)),)
502514

‎tests/corekit/test_fields_ipaddress.py‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -606,30 +606,24 @@ def test_switch_backed_address_makers_reject_a_bool(self) -> None:
606606
locator_set=[{'ip': int(True)}]).pack().hex(),
607607
'00c1001800000400000000000000000000000000000000000000000100000000')
608608

609-
def test_ipv4_interface_post_process_rejects_a_non_contiguous_netmask(self) -> None:
610-
"""``IPv4InterfaceField.post_process`` builds
611-
``ipaddress.ip_interface(f'{ip}/{mask}')`` from wire bytes whose
612-
trailing four octets are meant to be a dotted netmask. Unlike the
613-
leading four octets (always exactly 4 octets, so always a valid
614-
address), those trailing octets are not guaranteed to form a
615-
*contiguous* netmask -- e.g. a capture with a malformed or corrupted
616-
IPv4 interface option can carry ``0.255.0.255``, which
617-
:func:`ipaddress.ip_interface` rejects with a bare
618-
:exc:`~ipaddress.NetmaskValueError`. This is reachable through
619-
:meth:`~pcapkit.corekit.fields.field.FieldBase.unpack` alone, with no
620-
malformed-length input required, and must raise :exc:`FieldValueError`
621-
instead.
609+
def test_ipv4_interface_post_process_keeps_a_non_contiguous_netmask(self) -> None:
610+
"""``IPv4InterfaceField.post_process`` reads wire bytes whose trailing
611+
four octets are meant to be a dotted netmask. Those octets are not
612+
guaranteed to form a *contiguous* netmask -- e.g. a capture with a
613+
malformed or corrupted IPv4 interface option can carry
614+
``0.255.0.255``, which :func:`ipaddress.ip_interface` rejects. The
615+
field keeps such a mask as given, so the value packs back to the same
616+
octets (c.f. :issue:`1294`).
622617
"""
623618
from pcapkit.corekit.fields.ipaddress import IPv4InterfaceField
624-
from pcapkit.utilities.exceptions import BaseError, FieldValueError
625619

626620
field = IPv4InterfaceField()
627621
raw = ipaddress.IPv4Address('1.2.3.4').packed + bytes([0, 255, 0, 255])
628622

629-
with self.assertRaises(FieldValueError) as context:
630-
field.unpack(raw, {})
631-
self.assertIsInstance(context.exception, BaseError)
632-
self.assertIn('0.255.0.255', str(context.exception))
623+
value = field.unpack(raw, {})
624+
self.assertIsInstance(value, ipaddress.IPv4Interface)
625+
self.assertEqual(value.netmask, ipaddress.IPv4Address('0.255.0.255'))
626+
self.assertEqual(field.pack(value, {}), raw)
633627

634628

635629
if __name__ == '__main__':
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
# -*- coding: utf-8 -*-
2+
"""IPv4 interface fields keep the mask octets they read.
3+
4+
GitHub issue #1294: :meth:`IPv4InterfaceField.post_process
5+
<pcapkit.corekit.fields.ipaddress.IPv4InterfaceField.post_process>` built the
6+
value with :func:`ipaddress.ip_interface`, which reads a hostmask-shaped mask
7+
such as ``0.0.0.255`` as ``/24``. The field then packed ``255.255.255.0``, so
8+
a pcapng ``if_IPv4addr`` option did not rebuild byte for byte.
9+
10+
Every case builds its own octets in memory and reads no capture. Classes are
11+
imported inside each test, after
12+
:func:`~tests._support.reimport_once_per_class`, for the reason given in
13+
:mod:`tests.protocols.link.test_ethernet_mac_roundtrip_unit`.
14+
15+
"""
16+
17+
import copy
18+
import ipaddress
19+
import pickle
20+
import unittest
21+
22+
from tests._support import reimport_once_per_class
23+
24+
#: Address ``10.0.0.1`` followed by four mask octets.
25+
MASKS = {
26+
'hostmask /24': '0a000001' '000000ff',
27+
'hostmask /1': '0a000001' '7fffffff',
28+
'non-contiguous': '0a000001' 'ff00ff00',
29+
'netmask /24': '0a000001' 'ffffff00',
30+
'netmask /0': '0a000001' '00000000',
31+
'netmask /32': '0a000001' 'ffffffff',
32+
}
33+
34+
35+
class TestIPv4InterfaceMask(unittest.TestCase):
36+
"""Pin the mask octets of :class:`IPv4InterfaceField` across a round trip."""
37+
38+
def setUp(self) -> None:
39+
reimport_once_per_class(self)
40+
41+
def test_unpack_then_pack_keeps_the_mask_octets(self) -> None:
42+
from pcapkit.corekit.fields.ipaddress import IPv4InterfaceField
43+
44+
field = IPv4InterfaceField()
45+
for name, hexstr in MASKS.items():
46+
with self.subTest(mask=name):
47+
raw = bytes.fromhex(hexstr)
48+
value = field.unpack(raw, {})
49+
self.assertIsInstance(value, ipaddress.IPv4Interface)
50+
self.assertEqual(value.ip, ipaddress.IPv4Address('10.0.0.1'))
51+
self.assertEqual(value.netmask.packed, raw[4:])
52+
self.assertEqual(field.pack(value, {}), raw)
53+
54+
def test_string_form_of_a_parsed_value_packs_to_the_same_octets(self) -> None:
55+
from pcapkit.corekit.fields.ipaddress import IPv4InterfaceField
56+
57+
field = IPv4InterfaceField()
58+
for name, hexstr in MASKS.items():
59+
with self.subTest(mask=name):
60+
raw = bytes.fromhex(hexstr)
61+
self.assertEqual(field.pack(str(field.unpack(raw, {})), {}), raw)
62+
63+
def test_prefix_and_netmask_strings_still_use_the_stdlib(self) -> None:
64+
from pcapkit.corekit.fields.ipaddress import IPv4InterfaceField
65+
66+
field = IPv4InterfaceField()
67+
for text in ('10.0.0.1/24', '10.0.0.1/255.255.255.0'):
68+
with self.subTest(text=text):
69+
self.assertEqual(field.pack(text, {}), bytes.fromhex(MASKS['netmask /24']))
70+
self.assertEqual(field.pack('10.0.0.1', {}), bytes.fromhex(MASKS['netmask /32']))
71+
72+
def test_ordering_agrees_with_equality(self) -> None:
73+
from pcapkit.corekit.fields.ipaddress import IPv4InterfaceField
74+
75+
field = IPv4InterfaceField()
76+
values = [field.unpack(bytes.fromhex(hexstr), {}) for hexstr in MASKS.values()]
77+
values += [field.unpack(bytes.fromhex('0a000002' '000000ff'), {}),
78+
ipaddress.IPv4Interface('10.0.0.5/8'), ipaddress.IPv4Interface('10.0.0.3/32')]
79+
for a in values:
80+
for b in values:
81+
with self.subTest(a=str(a), b=str(b)):
82+
self.assertEqual([a < b, a == b, a > b].count(True), 1)
83+
self.assertEqual(a < b, b > a)
84+
self.assertEqual(a <= b, b >= a)
85+
self.assertEqual(a <= b, a < b or a == b)
86+
self.assertEqual(a >= b, a > b or a == b)
87+
for c in values:
88+
if a < b < c:
89+
self.assertLess(a, c)
90+
91+
def test_contiguous_netmask_stays_a_plain_interface(self) -> None:
92+
from pcapkit.corekit.fields.ipaddress import IPv4InterfaceField
93+
94+
value = IPv4InterfaceField().unpack(bytes.fromhex(MASKS['netmask /24']), {})
95+
self.assertIs(type(value), ipaddress.IPv4Interface)
96+
self.assertEqual(value, ipaddress.IPv4Interface('10.0.0.1/24'))
97+
98+
def test_kept_mask_survives_copy_pickle_and_compares_by_mask(self) -> None:
99+
from pcapkit.corekit.fields.ipaddress import IPv4InterfaceField
100+
101+
field = IPv4InterfaceField()
102+
value = field.unpack(bytes.fromhex(MASKS['hostmask /24']), {})
103+
self.assertEqual(str(value), '10.0.0.1/0.0.0.255')
104+
self.assertEqual(value.hostmask, ipaddress.IPv4Address('255.255.255.0'))
105+
for clone in (copy.copy(value), copy.deepcopy(value), pickle.loads(pickle.dumps(value))):
106+
self.assertEqual(clone, value)
107+
self.assertEqual(hash(clone), hash(value))
108+
self.assertEqual(clone.netmask, value.netmask)
109+
# the stdlib reading of the same octets is a different value
110+
self.assertNotEqual(value, ipaddress.IPv4Interface('10.0.0.1/24'))
111+
self.assertNotEqual(value, field.unpack(bytes.fromhex(MASKS['hostmask /1']), {}))
112+
113+
def test_pcapng_if_ipv4addr_option_rebuilds_byte_for_byte(self) -> None:
114+
from pcapkit.protocols.schema.misc.pcapng import IF_IPv4AddrOption
115+
116+
option = bytes.fromhex('0400' '0800') + bytes.fromhex(MASKS['hostmask /24'])
117+
schema = IF_IPv4AddrOption.unpack(option)
118+
self.assertEqual(schema.interface.netmask, ipaddress.IPv4Address('0.0.0.255'))
119+
self.assertEqual(schema.pack(), option)
120+
121+
122+
if __name__ == '__main__':
123+
unittest.main()

0 commit comments

Comments
 (0)