Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 16 additions & 5 deletions pcapkit/protocols/internet/ipv4.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,10 @@ def _make_data(cls, data: 'Data_IPv4') -> 'dict[str, Any]': # type: ignore[over
Key-value pairs for protocol construction.

"""
# NOTE: ``make`` takes ``offset`` in on-wire 8-octet units, while
# ``data.offset`` is in octets (see :meth:`read`), so scale back down to
# keep the data-to-schema round trip exact, mirroring
# :meth:`pcapkit.protocols.internet.ipv6_frag.IPv6_Frag._make_data`.
return {
'tos_pre': data.tos.pre,
'tos_del': data.tos['del'],
Expand All @@ -526,13 +530,16 @@ def _make_data(cls, data: 'Data_IPv4') -> 'dict[str, Any]': # type: ignore[over
'id': data.id,
'df': data.flags.df,
'mf': data.flags.mf,
'offset': data.offset,
'offset': data.offset // 8,
'ttl': data.ttl,
'protocol': data.protocol,
'checksum': data.checksum,
'src': data.src,
'dst': data.dst,
'options': data.options,
# NOTE: ``options`` is only present on ``data`` when the packet's
# ``hdr_len`` exceeds the fixed 20-octet header (see :meth:`read`),
# so read it defensively rather than assuming it always exists.
'options': getattr(data, 'options', None),
'payload': cls._make_payload(data),
}

Expand Down Expand Up @@ -1359,7 +1366,11 @@ def _make_opt_sec(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_SECOpt
Args:
kind: option type code
option: option data
sec: security option
level: classification level
level_default: default value for classification level
level_namespace: namespace for classification level
level_reversed: whether classification level is reversed
authorities: list of protection authority flags
**kwargs: arbitrary keyword arguments

Returns:
Expand Down Expand Up @@ -1773,8 +1784,8 @@ def _make_opt_qs(self, kind: 'Enum_OptionNumber', option: 'Optional[Data_QuickSt
"""Make IPv4 Quick-Start (``QS``) option.

Args:
code: option type value
opt: option data
kind: option type code
option: option data
func: QS function type
func_default: default value for QS function type
func_namespace: namespace for QS function type
Expand Down
50 changes: 50 additions & 0 deletions tests/protocols/internet/test_ipv4_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

import datetime
import importlib.util
import io
from ipaddress import ip_address
import types
import unittest
from unittest import mock
import warnings

from tests._support import purge_modules

Expand Down Expand Up @@ -56,6 +58,54 @@ class DummyDict(dict):
self.assertEqual(values['dst'], '198.51.100.20')
self.assertIn('payload', values)

def test_ipv4_make_data_scales_offset_and_defaults_missing_options(self) -> None:
"""Regression test for #494.

Two defects live in :meth:`IPv4._make_data
<pcapkit.protocols.internet.ipv4.IPv4._make_data>`: the fragment
``offset`` was handed back in octets to a parameter that
:meth:`IPv4.make <pcapkit.protocols.internet.ipv4.IPv4.make>` takes in
on-wire 8-octet units (:rfc:`791`), and ``data.options`` was read
unconditionally even though :meth:`IPv4.read
<pcapkit.protocols.internet.ipv4.IPv4.read>` only sets it when
``hdr_len`` exceeds the fixed 20-octet header.

The two interact: the missing-``options`` ``AttributeError`` fires
before the unscaled ``offset`` can even be returned, so a fixture
that always supplies ``options`` -- like the ``DummyDict`` one above,
which also uses ``offset=0``, the one value for which the missing
``// 8`` is invisible -- cannot catch either. This test uses a
non-zero offset *and* a real packet with no options, so both
defects are exercised together, exactly as the issue's own repro
does.

"""
from pcapkit.protocols.internet.ipv4 import IPv4

proto = object.__new__(IPv4)

# Wire Fragment Offset of 5 (8-octet units), no options -> hdr_len
# stays at the fixed 20 octets and read() never sets ``.options``.
with warnings.catch_warnings():
warnings.simplefilter('ignore')
raw = proto.make(offset=5, protocol=6, payload=b'\xaa' * 8).pack()
data = IPv4(io.BytesIO(raw), len(raw)).info

self.assertFalse(hasattr(data, 'options'))
self.assertEqual(data.offset, 40) # read() scales wire units to octets

values = IPv4._make_data(data)
self.assertEqual(values['offset'], 5) # scaled back down to wire units
self.assertIsNone(values['options'])

# Full round trip: re-packing with the recovered offset must
# reproduce the exact original wire bytes.
with warnings.catch_warnings():
warnings.simplefilter('ignore')
raw2 = proto.make(offset=values['offset'], protocol=6,
payload=b'\xaa' * 8).pack()
self.assertEqual(raw, raw2)

def test_ipv4_properties_read_and_make_cover_packet_paths(self) -> None:
from pcapkit.const.ipv4.option_number import OptionNumber
from pcapkit.const.reg.transtype import TransType
Expand Down
Loading