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
14 changes: 5 additions & 9 deletions tests/protocols/internet/test_ah_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ def test_docstring_cites_the_extension_header_registry(self) -> None:
"being *in* that registry is what makes something an extension
header" ruling.
Rephrasing the placement sentence as "places it among the IPv6
extension headers" overstated the RFC; that was caught in review
on the second version of this fix.
extension headers" overstates the RFC.

The section's separate IPv4-context sentence ("placed after the
IP header ... before the next layer protocol") is legitimate
Expand All @@ -152,13 +151,10 @@ def test_docstring_cites_the_extension_header_registry(self) -> None:
that fact *qualifies* an already-standalone header for a base
besides :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`;
it does not *make* the header standalone, and the RFC has no
say in which base is named. An earlier revision of this
docstring's ``ESP`` counterpart claimed the RFC "makes it a
standalone protocol" and "names ``IPsec`` as the second base",
the latter positionally false and unsupported by the cited
section; both were caught in review on the third version of
this fix. So this test targets the exact wrong phrasing from
the earlier rounds rather than banning "IPv4" outright, which
say in which base is named, so a claim that the RFC "makes it a
standalone protocol" or "names ``IPsec`` as the second base" is
unsupported by the cited section. So this test targets the wrong
phrasings themselves rather than banning "IPv4" outright, which
would forbid the legitimate use kept above.
"""
import re
Expand Down
24 changes: 11 additions & 13 deletions tests/protocols/internet/test_esp_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,9 @@ def test_security_association_rejects_a_bool_destination(self) -> None:
"""``destination`` calls :func:`ipaddress.ip_address` directly, with
no version check at all -- unlike the ``Field`` classes in
:mod:`pcapkit.corekit.fields.ipaddress`, ESP does not route through
that abstraction. Before this fix,
that abstraction. This guards against
``SecurityAssociation(spi=1, destination=True).destination`` silently
became ``IPv4Address('0.0.0.1')``, with no exception and no warning.
becoming ``IPv4Address('0.0.0.1')``, with no exception and no warning.
See #491.
"""
from pcapkit.protocols.internet.esp import SecurityAssociation
Expand Down Expand Up @@ -1059,10 +1059,9 @@ def test_docstring_cites_the_extension_header_registry(self) -> None:
"being *in* that registry is what makes something an extension
header" ruling.
Rephrasing the placement sentence as "places it among the IPv6
extension headers" overstated the RFC and contradicted this same
extension headers" overstates the RFC and contradicts this same
docstring's ``Note:`` (which says RFC 8200 declines ESP
extension-header status); that was caught in review on the
second version of this fix.
extension-header status).

The section's separate IPv4-context sentence ("placed after the
IP header ... before the next layer protocol") is legitimate
Expand All @@ -1077,14 +1076,13 @@ def test_docstring_cites_the_extension_header_registry(self) -> None:
besides :class:`~pcapkit.protocols.internet.ipv6_ext.IPv6_Ext`;
it does not *make* the header standalone, and the RFC has no
say in which base is named (``IPsec`` versus ``Internet``) --
an earlier revision claimed the RFC "makes it a standalone
protocol" and "names ``IPsec`` as the second base", the latter
positionally false (``ESP.__bases__`` puts ``IPsec`` first, not
second) and unsupported by the cited section; both were caught
in review on the third version of this fix. So this test
targets the exact wrong phrasing from the earlier rounds rather
than banning "IPv4" outright, which would forbid the legitimate
use kept above.
so a claim that the RFC "makes it a standalone protocol" or
"names ``IPsec`` as the second base" is unsupported by the cited
section, and the latter is positionally false too
(``ESP.__bases__`` puts ``IPsec`` first, not second). So this
test targets the wrong phrasings themselves rather than banning
"IPv4" outright, which would forbid the legitimate use kept
above.
"""
import re

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
one that reached the limit warned of it, twice for IPv6 and up to 33 times for
IPv4, and went up to 32 headers past where the parse had stopped.

Now a rebuilt payload takes its place in its chain from the layer it is the
A rebuilt payload takes its place in its chain from the layer it is the
payload of, as #1610 has it take its place in the frame, and only the layer
being rebuilt warns, once its own parse gets to the limit.

Expand Down Expand Up @@ -182,7 +182,8 @@ def test_where_both_limits_fall_on_one_layer_rebuilding_warns_once(self) -> None

def test_where_the_frame_limit_falls_first_rebuilding_warns_once(self) -> None:
# Guard: 40 tunnels put the 65th layer at the chain's 24th header, short
# of the chain limit; the rebuild warned once before #1619 too.
# of the chain limit; the rebuild warns once with or without the #1619
# fix.
from pcapkit.protocols.link.ethernet import Ethernet

for carrier in ETHERTYPE:
Expand Down
4 changes: 2 additions & 2 deletions tests/protocols/internet/test_hip_host_id_di_width_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def test_made_host_id_packs_the_rfc_layout(self) -> None:
hi_pub_key=PUB_KEY, di=DI, di_type=DITypes.FQDN)

self.assertEqual(schema.len, 14)
# 26 octets before #1118: two zero octets after the DI word.
# Without the #1118 fix, 26 octets: two zero octets after the DI word.
self.assertEqual(schema.pack().hex(), rfc_host_id().hex())

def test_rfc_host_id_parses_algorithm_and_di(self) -> None:
Expand All @@ -83,7 +83,7 @@ def test_rfc_host_id_parses_algorithm_and_di(self) -> None:
raw = rfc_host_id()
schema = HostIDParameter.unpack(raw, len(raw)) # type: ignore[arg-type]

# Read as 1 -- the curve -- before #1118.
# Without the #1118 fix, read as 1 -- the curve.
self.assertEqual(schema.algorithm, HIAlgorithm.ECDSA)
self.assertEqual(int(schema.algorithm), 7)
self.assertEqual(schema.di_data['type'], DITypes.FQDN)
Expand Down
2 changes: 1 addition & 1 deletion tests/protocols/internet/test_hip_locator_flags_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def _parse(self, frame: bytes): # type: ignore[no-untyped-def]

with warnings.catch_warnings():
# Nested ``Locator`` schemas warn about their placeholder length;
# that is pre-existing noise unrelated to the flags octet.
# that is noise unrelated to the flags octet.
warnings.simplefilter('ignore')
return HIP(frame, len(frame), extension=True)

Expand Down
24 changes: 11 additions & 13 deletions tests/protocols/internet/test_hip_locator_set_length_unit.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
# -*- coding: utf-8 -*-
"""HIP ``LOCATOR_SET``, whose ``Length`` unit and padding source were both wrong.
"""HIP ``LOCATOR_SET`` counts ``Length`` in bytes and pads from its own length.

GitHub issue #679. Two pre-existing defects, neither fixable alone:
GitHub issue #679. Two defects, neither fixable alone:

1. ``LocatorSetParameter.padding`` read the **nested** ``Locator.len`` rather than
the parameter's. A :class:`~pcapkit.corekit.fields.collections.ListField` packs
each nested schema into the enclosing packet context --
:meth:`Schema.pack <pcapkit.protocols.schema.schema.Schema.pack>` opens with
``packet.update(self.__dict__)`` -- and :class:`Locator` declares a ``len`` of
its own, so by the time ``padding`` was evaluated ``pkt['len']`` was the last
locator's. That is 4 for any IPv6 locator whatever the count, so the old
locator's. That is 4 for any IPv6 locator whatever the count, so that
expression appended exactly four octets to every ``LOCATOR_SET``.
2. The parameter's ``len`` was ``sum(Locator.len)``, in the 4-octet units
:rfc:`8046#section-4` gives ``Locator Length``, where :rfc:`7401`
Expand All @@ -30,7 +30,7 @@
**The cancellation was never general, and that is the whole reason this module
sweeps shapes rather than counts.** It needs every locator to be 24 octets and
there to be at least one of them. Measured on ``f0999858e``, the shapes where it
does not hold were non-conformant before this fix:
does not hold were non-conformant without the fix:

=================== ============== ====== =========================
shape declared ``len`` packed RFC total for the contents
Expand All @@ -46,7 +46,7 @@
=================== ============== ====== =========================

Four of the seven were not even multiples of eight. A sweep over n = 1..5 of
plain locators passed by cancellation before the fix and would pass a half-fix
plain locators passes by cancellation without the fix and would pass a half-fix
too, so it discriminates nothing; the empty set, the SPI-bearing variant and a
mixed set in both orders are the shapes that do.

Expand Down Expand Up @@ -97,13 +97,12 @@
#: :func:`~pcapkit.protocols.schema.internet.hip.locator_value_selector` accepts
#: only ``type == 0, len == 4`` or ``type == 1, len == 5`` -- so ``spi=`` without
#: an explicit ``type=1`` raises ``FieldValueError: invalid locator type or
#: length``. That is a separate rough edge in the maker, noted rather than fixed
#: here, and it is why an earlier attempt to measure these shapes could not reach
#: the SPI path at all.
#: length``. That is a separate rough edge in the maker, and without ``type=1``
#: these shapes could not reach the SPI path at all.
SPI = {'ip': '2001:db8::2', 'spi': 0xdeadbeef, 'type': 1}

#: Every shape, with the octets each locator record occupies. Ordered so that the
#: three the old cancellation covered come first and the four it did not follow.
#: shapes the cancellation covers come first and the ones it does not follow.
SHAPES = (
('empty', [], 0),
('plain n=1', [PLAIN], 24),
Expand Down Expand Up @@ -293,8 +292,7 @@ def test_every_shape_parses_back_to_the_locators_it_was_built_from(self) -> None
of them, one per field, because ``SchemaField`` seeds a nested schema's
``__length__`` at the ``-1`` placeholder
:meth:`Schema.pack <pcapkit.protocols.schema.schema.Schema.pack>`
documents; that is pre-existing, unrelated to ``Length`` units, and
unchanged by this fix -- what changed is that it now fires once per
documents; that is unrelated to ``Length`` units, and it fires once per
locator actually parsed rather than once per truncated first locator.
"""
from pcapkit.const.hip.parameter import Parameter
Expand Down Expand Up @@ -349,8 +347,8 @@ def test_a_lone_locator_set_parameter_survives_a_hip_packet(self) -> None:

with warnings.catch_warnings():
# The per-locator ``__length__`` warnings described above are
# pre-existing noise on a now-correct parse; the assertions
# below are what this case is about.
# noise unrelated to ``Length`` units; the assertions below
# are what this case is about.
warnings.simplefilter('ignore')
parsed = HIP(octets, len(octets), extension=True)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
field in 8-octet units excluding the first 8, so the two 16-octet HITs alone
make 4 the minimum. The header's own re-parse then computed a parameter length
of ``-32`` and raised ``ProtocolError``. :meth:`HIP.read
<pcapkit.protocols.internet.hip.HIP.read>` now rejects a wire Header Length
<pcapkit.protocols.internet.hip.HIP.read>` rejects a wire Header Length
below 4 up front.

Every case builds its own octets in memory and reads no capture.
Expand Down
28 changes: 14 additions & 14 deletions tests/protocols/internet/test_hip_r1_counter_width_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
this one's tail, and a real peer's 16-octet ``R1_COUNTER`` leaves its last four
octets to be read as the following parameter's ``Type``.

Why the existing suite could not see it
---------------------------------------
Why the suite could not see it
------------------------------

Three things hid it at once, and the third is the reason this module asserts
against the RFC's arithmetic rather than against a round trip.
Expand All @@ -30,10 +30,10 @@
each parameter in a frame, and two four-octet shortfalls sum to eight, so the
record area stays 8-aligned and ``HIP.make``'s ``total_length // 8 + 4``
comes out exact.
2. Before #651 the padding aligned the parameter *contents* rather than the
record, which at ``Length = 12`` appended exactly the four surplus octets this
parameter was missing. Correcting the padding did not break ``R1_COUNTER``; it
stopped compensating for it.
2. A padding rule that aligns the parameter *contents* rather than the record
(the #651 defect) appends, at ``Length = 12``, exactly the four surplus octets
this parameter was missing. So correcting the padding does not break
``R1_COUNTER``; it stops compensating for it.
3. The round trip cannot see it at all, because pcapkit's reader consumes exactly
the twelve octets its writer wrote. Only a comparison against the RFC's own
stride -- or a real peer -- can tell the difference, which is what
Expand Down Expand Up @@ -106,7 +106,7 @@ def test_r1_counter_record_is_the_rfc_total_at_both_codes(self) -> None:
The exact octets are pinned, not just the length, because the length
alone would pass for a record padded to 16 with a four-octet counter as
readily as for one carrying the eight-octet counter the RFC specifies --
and the padding is what the pre-#651 rule appended. So the assertion
that padding is what a contents-aligning rule (#651) appends. So the assertion
names where the eight octets sit: four of ``Reserved`` after the
type-and-length header, then the counter, then no padding at all, since
``rfc_total(12) - 4 - 12`` is zero.
Expand All @@ -128,7 +128,7 @@ def test_r1_counter_record_is_the_rfc_total_at_both_codes(self) -> None:
self.assertEqual(len(raw), rfc_total(12))
self.assertEqual(len(raw), 16)
self.assertEqual(len(raw) % 8, 0)
# Twelve octets before #672, four of them the counter:
# Without the #672 fix, twelve octets, four of them the counter:
# ``00 8x 00 0c 00 00 00 00 00 00 00 01``.
self.assertEqual(
raw.hex(), prefix + '000c' + '00000000' + '0000000000000001')
Expand Down Expand Up @@ -175,12 +175,12 @@ def test_a_lone_r1_counter_parameter_survives_a_hip_packet(self) -> None:
both codes; the two-copy pairing in
:mod:`examples.generators.options` is what concealed that.

Code 128 is deliberately not exercised here. #690 registered
``R1CounterParameter`` for 128 as well as 129, so it now reaches this
same class rather than parsing as an ``UnassignedParameter`` --
that coverage lives with the fix, in
Code 128 is deliberately not exercised here. ``R1CounterParameter`` is
registered for 128 as well as 129 (#690), so 128 reaches this same
class rather than parsing as an ``UnassignedParameter`` -- that
coverage lives in
:meth:`HIPUnitTests.test_hip_r1_counter_code_128_resolves_to_its_own_schema`,
not here; this module stays scoped to the width #672 fixed.
not here; this module stays scoped to the width (#672).
"""
from pcapkit.const.hip.parameter import Parameter
from pcapkit.protocols.internet.hip import HIP
Expand Down Expand Up @@ -213,7 +213,7 @@ def test_the_generator_gives_r1_counter_a_non_zero_counter(self) -> None:
walk from 1 violation to 2, the new one being ``frame 101: type 0 padding
not zeroed: aabbccdd``.

The width is fixed now, so the walk finds nothing here either way. This
With the width right, the walk finds nothing here either way. This
pins the override anyway, because a fixture that cannot discriminate a
wrong width from a right one is what let this defect survive as long as
it did -- the same trap #608 fell into with the ``SOLUTION`` field.
Expand Down
Loading
Loading