From aad2f0c766b4f743bd58ea1e97aaff01c97e25da Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 25 Oct 2026 10:28:20 -0400 Subject: [PATCH] docs(tests): state protocol, schema and misc tests' behaviour without change-relative narrative (#1103) Rewrite comments and docstrings in tests/protocols/ (top level), tests/protocols/schema/ and tests/protocols/misc/ so they state the behaviour under test and the defect it guards against, rather than "now", "used to", "before the fix" or "on the unfixed tree". Comments and docstrings only; the AST with docstrings stripped is unchanged. --- .../misc/pcap/test_frame_endian_runtime.py | 22 +-- .../misc/pcap/test_frame_from_data_runtime.py | 9 +- .../test_frame_incl_len_overrun_1541_unit.py | 5 +- .../misc/pcap/test_frame_length_runtime.py | 26 +-- .../protocols/misc/pcap/test_frame_runtime.py | 13 +- .../pcap/test_frame_snaplen_zero_1385_unit.py | 13 +- .../test_frame_trailing_bytes_1265_unit.py | 5 +- .../misc/pcap/test_header_frame_unit.py | 20 +- ...t_pcapng_captured_len_overrun_1405_unit.py | 8 +- ...st_pcapng_dsb_secrets_overrun_1980_unit.py | 10 +- .../test_pcapng_flags_undefined_1382_unit.py | 8 +- ..._pcapng_kept_as_captured_1413_1414_unit.py | 15 +- .../test_pcapng_nrb_empty_names_1383_unit.py | 10 +- .../misc/test_pcapng_option_area_kept_unit.py | 12 +- .../test_pcapng_padding_kept_1437_unit.py | 16 +- .../test_pcapng_registrar_identity_unit.py | 8 +- .../misc/test_pcapng_roundtrip_unit.py | 5 +- .../test_pcapng_short_shb_idb_1422_unit.py | 14 +- .../misc/test_pcapng_spb_snaplen_1384_unit.py | 12 +- ...t_pcapng_tzone_tsoffset_range_1340_unit.py | 10 +- tests/protocols/misc/test_pcapng_unit.py | 171 +++++++++--------- .../test_pcapng_zigbee_key_width_1986_unit.py | 10 +- .../test_raw_linktype_dispatch_1502_unit.py | 11 +- .../schema/test_enum_schema_registry_unit.py | 20 +- ...est_schema_child_finalisation_1498_unit.py | 18 +- .../test_schema_field_remainder_unit.py | 8 +- .../test_schema_metaclass_abc_cache_unit.py | 20 +- tests/protocols/schema/test_schema_unit.py | 43 ++--- .../test_capture_roundtrip_runtime.py | 4 +- .../test_construction_keyword_check_unit.py | 41 ++--- .../test_declared_length_overrun_unit.py | 4 +- .../test_declared_length_roundtrip_runtime.py | 8 +- .../protocols/test_dispatch_bindings_unit.py | 6 +- .../test_dispatch_default_resolution_unit.py | 8 +- .../test_frame_layer_limit_1610_unit.py | 23 ++- .../test_ipv4_l2tp_padding_guard_1437_unit.py | 6 +- .../test_ipv4_tcp_header_overrun_unit.py | 12 +- tests/protocols/test_make_index_str_unit.py | 6 +- .../protocols/test_make_raw_item_read_unit.py | 13 +- ...t_next_layer_hit_not_memoised_1559_unit.py | 32 ++-- .../test_next_layer_short_header_unit.py | 5 +- .../test_option_generator_1337_unit.py | 22 +-- ...tion_generator_deadline_scale_1428_unit.py | 22 +-- ...est_option_generator_httpv2_stream_unit.py | 12 +- ...st_option_generator_jumbogram_1687_unit.py | 12 +- .../test_option_generator_tcp_base_unit.py | 32 ++-- ...est_pack_keeps_instance_state_1769_unit.py | 7 +- .../test_padding_kept_as_captured_unit.py | 9 +- .../test_parser_registrar_identity_unit.py | 10 +- tests/protocols/test_pcapng_regression.py | 12 +- tests/protocols/test_protocol_base_unit.py | 24 +-- .../test_protocol_code_registration_unit.py | 11 +- tests/protocols/test_protocol_make_unit.py | 27 +-- .../test_protocol_module_exports_1770_unit.py | 6 +- .../test_register_class_guard_unit.py | 12 +- ...r_restores_shipped_descriptor_1504_unit.py | 9 +- .../test_registry_stored_descriptor_unit.py | 14 +- ...st_schema_payload_zero_length_1275_unit.py | 2 +- .../test_short_read_roundtrip_unit.py | 11 +- .../test_to_dict_compare_1484_runtime.py | 12 +- ...est_to_dict_payload_roundtrip_1447_unit.py | 7 +- .../test_trial_parse_warnings_1580_unit.py | 24 +-- 62 files changed, 507 insertions(+), 500 deletions(-) diff --git a/tests/protocols/misc/pcap/test_frame_endian_runtime.py b/tests/protocols/misc/pcap/test_frame_endian_runtime.py index f7a07c7b49..c2ce18143e 100644 --- a/tests/protocols/misc/pcap/test_frame_endian_runtime.py +++ b/tests/protocols/misc/pcap/test_frame_endian_runtime.py @@ -4,20 +4,18 @@ Classic PCAP declares its byte order in the magic number of the global header, and the four fields of every record header -- ``ts_sec``, ``ts_usec``, ``incl_len`` and ``orig_len`` -- are written in that order rather than in the -reading host's. Nothing in this suite read a big-endian ``.pcap`` before, because -no such capture existed here: every fixture was little-endian, so on a -little-endian runner a reader that ignored the file's declared order and used -:data:`sys.byteorder` produced exactly the right answer. +reading host's. Little-endian fixtures alone cannot show whether that order is +honoured: on a little-endian runner, a reader that ignores the file's declared +order and uses :data:`sys.byteorder` produces exactly the right answer. -That is what GitHub issue #605 was. +This guards against GitHub issue #605, where :meth:`Frame.unpack ` seeded the -order under the key ``bytesorder``, while ``byteorder_callback`` -- then in -:file:`pcapkit/protocols/schema/misc/pcap/frame.py`, now in -:file:`pcapkit/corekit/fields/byteorder.py` -- reads ``byteorder``, so the +order under the key ``bytesorder``, while ``byteorder_callback`` +(:file:`pcapkit/corekit/fields/byteorder.py`) reads ``byteorder``, so the ``.get()`` never found the key and always fell back to the host's order -- the sibling :meth:`Frame.pack ` -eleven lines earlier spelled it correctly, which is what marks it as a slip -rather than a second key. Measured on the unfixed tree against +spelled it correctly, which is what marks it as a slip rather than a second +key. Measured without the fix against ``big_endian.pcap``, whose first record really holds ``ts_sec=1500000000``, ``ts_usec=123456`` and ``incl_len=74``: frame 1 came back with ``ts_sec=3106905``, ``ts_usec=1088553216`` and ``incl_len=1241513984``, i.e. @@ -179,7 +177,7 @@ def assertRecords(self, name: 'str', magic: 'bytes', endian: 'str', def test_big_endian_record_headers_are_read_in_the_files_byte_order(self) -> None: """#605: ``a1 b2 c3 d4``, microsecond timestamps. - On the unfixed tree this raises ``ValueError: read length must be + Without the fix this raises ``ValueError: read length must be non-negative or -1`` before reaching any assertion: frame 1's swapped ``incl_len`` of 1241513984 consumes the whole file, and the read that follows is handed a negative payload length. @@ -203,7 +201,7 @@ def test_big_endian_nanosecond_record_headers_are_read_in_the_files_byte_order(s def test_little_endian_twin_is_unaffected(self) -> None: """The control: the same three records in the little-endian container. - This passes on the unfixed tree too, and that is what it is for. It + This passes without the fix too, and that is what it is for. It pins the expectations of the big-endian tests to a file whose byte order was never in question, so a failure there cannot be blamed on the records themselves being wrong. diff --git a/tests/protocols/misc/pcap/test_frame_from_data_runtime.py b/tests/protocols/misc/pcap/test_frame_from_data_runtime.py index e96e441907..2821e71d4e 100644 --- a/tests/protocols/misc/pcap/test_frame_from_data_runtime.py +++ b/tests/protocols/misc/pcap/test_frame_from_data_runtime.py @@ -1,11 +1,12 @@ # -*- coding: utf-8 -*- """``Frame.from_data`` rebuilds a captured frame. -GitHub issue #1126: :meth:`~pcapkit.protocols.protocol.ProtocolBase.from_data` -took nothing but the data, so the ``num`` and ``header`` that +This guards against GitHub issue #1126, where +:meth:`~pcapkit.protocols.protocol.ProtocolBase.from_data` took nothing but the +data, so the ``num`` and ``header`` that :class:`~pcapkit.protocols.misc.pcap.frame.Frame` requires could not reach it, -and every call raised :exc:`TypeError`. ``from_data`` now forwards extra -keywords to the construction, so the caller passes both alongside the data. +and every call raised :exc:`TypeError`. ``from_data`` forwards extra keywords to +the construction, so the caller passes both alongside the data. The frames are read from :file:`examples/captures/in.pcap`, so this belongs to the runtime tier. diff --git a/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py b/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py index 670d6d23f2..e04203cada 100644 --- a/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py +++ b/tests/protocols/misc/pcap/test_frame_incl_len_overrun_1541_unit.py @@ -1,8 +1,9 @@ # -*- coding: utf-8 -*- """A PCAP record claiming more octets than its file holds warns, and is kept as captured. -GitHub issue #1541. A record whose ``incl_len`` runs past the end of the file -parsed silently. It is now reported the way the PCAP-NG reader reports a +This guards against GitHub issue #1541, where a record whose ``incl_len`` runs +past the end of the file parsed silently. It is reported the way the PCAP-NG +reader reports a captured length running past its block (#1405), and is kept as captured in the same way: ``incl_len`` and ``cap_len`` stay as declared, the packet is the octets present, and the frame rebuilds as exactly the octets read, from ``info`` and diff --git a/tests/protocols/misc/pcap/test_frame_length_runtime.py b/tests/protocols/misc/pcap/test_frame_length_runtime.py index 6b0fe87270..700c4e5a3d 100644 --- a/tests/protocols/misc/pcap/test_frame_length_runtime.py +++ b/tests/protocols/misc/pcap/test_frame_length_runtime.py @@ -3,8 +3,8 @@ Both readers that build a :class:`~pcapkit.protocols.data.misc.pcap.frame.Frame` have to agree about which -of the two wire fields lands in which attribute, and until GitHub issue #618 they -did not: the classic-PCAP reader in +of the two wire fields lands in which attribute. GitHub issue #618 is the +defect where they did not: the classic-PCAP reader in :mod:`pcapkit.protocols.misc.pcap.frame` wrote ``len=incl_len, cap_len=orig_len`` while :func:`pcapkit.toolkit.pcapng.block2frame` wrote ``len=original_len, cap_len=captured_len``. Exactly inverted, so ``frame.len`` @@ -31,12 +31,12 @@ been truncated", which "SHOULD NOT be less than the Captured Packet Length". So neither format is the odd one out -- only the reader was. -Why it survived so long: **the two lengths are equal unless the snapshot length +Why fixtures can miss it: **the two lengths are equal unless the snapshot length cut the frame short**, so a fixture without a truncated frame compares a value -against itself and cannot tell the two assignments apart. There was no such -fixture here until #614 added one. This module therefore leans on the two that -exist, and asserts that they really are truncated before asserting anything -about them -- a fixture regenerated into something untruncated fails +against itself and cannot tell the two assignments apart. This module therefore +leans on the two truncated fixtures (#614), and asserts that they really are +truncated before asserting anything about them -- a fixture regenerated into +something untruncated fails :meth:`PcapFrameLengthRuntimeTests.test_the_fixture_really_holds_a_truncated_frame` loudly rather than making everything below vacuous: @@ -226,7 +226,7 @@ def test_the_fixture_really_holds_a_truncated_frame(self) -> None: def test_len_is_the_on_wire_length_and_cap_len_the_captured_one(self) -> None: """#618: the PCAP reader had these two the other way round. - On the unfixed tree the truncated frame comes back with ``len=96`` and + Without the fix the truncated frame comes back with ``len=96`` and ``cap_len=1200`` -- the captured length under the name that means on-wire, and vice versa -- so this fails on that frame in every fixture while passing on the two untruncated ones, which is exactly the shape of @@ -283,9 +283,9 @@ def test_cap_len_counts_the_octets_the_frame_actually_carries(self) -> None: class PcapngFrameLengthRuntimeTests(unittest.TestCase): """#618, on the PCAP-NG reader -- the one that was already right. - This is the control. It passes on the unfixed tree too, and that is what it - is for: it pins the convention the PCAP reader was brought into line with, so - a later change that "fixes" the disagreement from the other end fails here. + This is the control. It passes without the fix too, and that is what it is + for: it pins the convention the PCAP reader follows, so a change that + "fixes" the disagreement from the other end fails here. """ @@ -383,7 +383,7 @@ def setUp(self) -> None: def test_a_truncated_frame_reads_alike_whichever_container_it_came_from(self) -> None: """One caller, two capture formats, one meaning per attribute. - On the unfixed tree the PCAP side reports ``len < cap_len`` and the + Without the fix the PCAP side reports ``len < cap_len`` and the PCAP-NG side ``len > cap_len`` for the same kind of frame, and this fails on the first assertion of the pair. @@ -411,7 +411,7 @@ def test_a_truncated_frame_reads_alike_whichever_container_it_came_from(self) -> for frame in truncated: with self.subTest(cap_len=frame.cap_len): # the on-wire length is the larger of the two, in both formats. - # This is the assertion #618 breaks: before the fix the PCAP + # This is the assertion #618 breaks: without the fix the PCAP # frame reports 96 against 1200 and fails here, while the # PCAP-NG one reports 314 against 32 and passes self.assertGreater(frame.len, frame.cap_len) diff --git a/tests/protocols/misc/pcap/test_frame_runtime.py b/tests/protocols/misc/pcap/test_frame_runtime.py index 6a435451b3..0a9869c9f0 100644 --- a/tests/protocols/misc/pcap/test_frame_runtime.py +++ b/tests/protocols/misc/pcap/test_frame_runtime.py @@ -90,16 +90,15 @@ def test_extracted_frames_carry_their_own_record_bytes(self) -> None: self.assertEqual(bytes(frame), expected) self.assertEqual(frame.packet.header, expected[:16]) self.assertEqual(frame.packet.payload, expected[16:]) - # the nested layer was always right -- it is parsed from the - # schema's payload rather than from the frame's raw data -- so this - # pins the two to each other, which is what was broken. + # the nested layer is parsed from the schema's payload rather than + # from the frame's raw data, so this pins the two to each other. # # Sliced by ``cap_len`` rather than by ``len``: the octets in the - # file are the *captured* ones, and since #618 ``len`` is the on-wire - # length, which for a truncated frame runs past the end of the + # file are the *captured* ones, and ``len`` is the on-wire length + # (#618), which for a truncated frame runs past the end of the # record. ``arp.pcap`` is not truncated, so the two are equal here - # and this is a correctness fix rather than a behaviour change -- - # which is exactly why it needs stating. + # and the choice changes no result -- which is exactly why it needs + # stating. self.assertEqual(bytes(frame.payload), expected[16:16 + frame.info.cap_len]) diff --git a/tests/protocols/misc/pcap/test_frame_snaplen_zero_1385_unit.py b/tests/protocols/misc/pcap/test_frame_snaplen_zero_1385_unit.py index 8390b1659b..8c830b6be7 100644 --- a/tests/protocols/misc/pcap/test_frame_snaplen_zero_1385_unit.py +++ b/tests/protocols/misc/pcap/test_frame_snaplen_zero_1385_unit.py @@ -1,12 +1,13 @@ # -*- coding: utf-8 -*- """Building a PCAP frame under a zero snaplen keeps the whole packet. -GitHub issue #1385: :meth:`Frame.make ` -defaulted ``incl_len`` to ``min(len(packet), snaplen)``, so a global header with -snaplen 0 produced a 16-octet record holding no packet data. The PCAP draft says -snaplen "MUST NOT be zero" (draft-ietf-opsawg-pcap, section 4), and libpcap reads -a zero value as the maximum for the link type (``pcapint_adjust_snapshot``), so -a zero snaplen is now no limit, as it already is for PCAP-NG. +This guards against GitHub issue #1385, where +:meth:`Frame.make ` defaulted +``incl_len`` to ``min(len(packet), snaplen)``, so a global header with snaplen 0 +produced a 16-octet record holding no packet data. The PCAP draft says snaplen +"MUST NOT be zero" (draft-ietf-opsawg-pcap, section 4), and libpcap reads a zero +value as the maximum for the link type (``pcapint_adjust_snapshot``), so a zero +snaplen is no limit, as it is for PCAP-NG. Every case builds its own octets and reads no capture. Classes are imported inside each test, after :func:`~tests._support.reimport_once_per_class`. diff --git a/tests/protocols/misc/pcap/test_frame_trailing_bytes_1265_unit.py b/tests/protocols/misc/pcap/test_frame_trailing_bytes_1265_unit.py index c54a8fc8d7..dc079ab7de 100644 --- a/tests/protocols/misc/pcap/test_frame_trailing_bytes_1265_unit.py +++ b/tests/protocols/misc/pcap/test_frame_trailing_bytes_1265_unit.py @@ -1,10 +1,11 @@ # -*- coding: utf-8 -*- """A PCAP file ending in a partial record header stops cleanly at EOF. -GitHub issue #1265: with fewer than sixteen octets left after the last record, +This guards against GitHub issue #1265, where, with fewer than sixteen octets +left after the last record, :meth:`Frame.unpack ` parsed a record header out of zero padding, and the payload read got a negative length -(``ValueError: read length must be non-negative or -1``). A tail that short now +(``ValueError: read length must be non-negative or -1``). A tail that short raises :exc:`~pcapkit.utilities.exceptions.StreamEOFError`, which the extractor treats as the end of the file, as it does for a truncated PCAP-NG block (#678). diff --git a/tests/protocols/misc/pcap/test_header_frame_unit.py b/tests/protocols/misc/pcap/test_header_frame_unit.py index 451e5e986d..1adba6eba7 100644 --- a/tests/protocols/misc/pcap/test_header_frame_unit.py +++ b/tests/protocols/misc/pcap/test_header_frame_unit.py @@ -219,8 +219,8 @@ class Other(Raw): # ``register`` resolves a ``ModuleDescriptor`` before storing it, # so the entry above is already the plain ``Raw`` class, not the - # descriptor -- passing ``Raw`` again would now be the same-object - # no-op GitHub issue #718 added an identity guard for (see + # descriptor -- passing ``Raw`` again would be the same-object + # no-op covered by the identity guard for GitHub issue #718 (see # ``test_frame_register_stays_quiet_on_same_object_reregistration``). # A genuinely different class is needed to still warn here. with mock.patch('pcapkit.protocols.misc.pcap.frame.warn') as warn: @@ -297,7 +297,7 @@ class Other(Raw): bad_time._file = io.BytesIO(b'') # Faked through frame.py's own binding of # :func:`pcapkit.utilities.datetime.fromtimestamp`, which it calls - # instead of :meth:`datetime.datetime.fromtimestamp` since #2099. + # instead of :meth:`datetime.datetime.fromtimestamp` (#2099). with mock.patch('pcapkit.protocols.misc.pcap.frame.fromtimestamp', side_effect=ValueError), \ mock.patch.object(Frame, '_decode_next_layer', lambda self, info, proto=None, length=None, packet=None: info), \ @@ -326,9 +326,9 @@ class Other(Raw): def test_frame_register_stays_quiet_on_same_object_reregistration(self) -> None: """GitHub issue #718: replaying a call with the identical class is a no-op. - Before the fix the guard fired on presence alone, so the *second* of + Without the fix the guard fires on presence alone, so the *second* of two calls registering the exact same class under the exact same code - warned about an overwrite that never happened. Matches the identity + warns about an overwrite that never happens. Matches the identity guard :func:`register_protocol ` already applies. @@ -441,13 +441,11 @@ def test_frame_header_is_read_in_the_files_byte_order(self) -> None: :meth:`Frame.unpack ` seeded the file's byte order under the key ``bytesorder``, where - ``byteorder_callback`` -- then in - :file:`pcapkit/protocols/schema/misc/pcap/frame.py`, now in - :file:`pcapkit/corekit/fields/byteorder.py` -- reads ``byteorder``, - so the lookup always missed and always fell back to + ``byteorder_callback`` (:file:`pcapkit/corekit/fields/byteorder.py`) + reads ``byteorder``, so the lookup always missed and always fell back to :data:`sys.byteorder`. On a little-endian host reading a little-endian - capture that is the right answer by coincidence, which is why every - fixture here passed; on a big-endian capture all four record-header + capture that is the right answer by coincidence, so little-endian + fixtures cannot catch it; on a big-endian capture all four record-header fields came back byte-swapped. The capture is built here rather than read from diff --git a/tests/protocols/misc/test_pcapng_captured_len_overrun_1405_unit.py b/tests/protocols/misc/test_pcapng_captured_len_overrun_1405_unit.py index 7f50d5b32b..02fdf815bd 100644 --- a/tests/protocols/misc/test_pcapng_captured_len_overrun_1405_unit.py +++ b/tests/protocols/misc/test_pcapng_captured_len_overrun_1405_unit.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- """GitHub issue #1405: a captured length running past the block rebuilds byte-exactly. -An Enhanced Packet Block whose ``captured_len`` runs past the block read that -many octets as packet data: the trailing Block Total Length and whatever -followed went with them, so a 36-octet block rebuilt as 40 or 41. The packet -data is now bounded by the block, the padding and option area are sized from +Without the fix, an Enhanced Packet Block whose ``captured_len`` runs past the +block reads that many octets as packet data: the trailing Block Total Length and +whatever follows go with them, so a 36-octet block rebuilds as 40 or 41. The +packet data is bounded by the block, the padding and option area are sized from the octets read, and the declared ``captured_len`` is kept, so the block rebuilds as captured. The obsolete Packet Block has the same shape and the same fix. diff --git a/tests/protocols/misc/test_pcapng_dsb_secrets_overrun_1980_unit.py b/tests/protocols/misc/test_pcapng_dsb_secrets_overrun_1980_unit.py index 4d76c25a84..28c6763e1f 100644 --- a/tests/protocols/misc/test_pcapng_dsb_secrets_overrun_1980_unit.py +++ b/tests/protocols/misc/test_pcapng_dsb_secrets_overrun_1980_unit.py @@ -1,11 +1,11 @@ # -*- coding: utf-8 -*- """GitHub issue #1980: a Decryption Secrets Block's secrets are bounded by the block. -A Decryption Secrets Block whose ``secrets_length`` ran far past the block read -that many octets as secrets: the trailing Block Total Length and the blocks after -it went with them, and past the zero-pad ceiling the read raised -:exc:`~pcapkit.utilities.exceptions.FieldValueError` from inside the block's -schema, which ended the whole extraction. The secrets are bounded by the +Without the fix, a Decryption Secrets Block whose ``secrets_length`` runs far +past the block reads that many octets as secrets: the trailing Block Total Length +and the blocks after it go with them, and past the zero-pad ceiling the read +raises :exc:`~pcapkit.utilities.exceptions.FieldValueError` from inside the +block's schema, which ends the whole extraction. The secrets are bounded by the block, as an Enhanced Packet Block's packet data is (#1405): the declared ``secrets_length`` is kept, a :class:`~pcapkit.utilities.warnings.ProtocolWarning` reports the overrun, and extraction carries on to the next block. diff --git a/tests/protocols/misc/test_pcapng_flags_undefined_1382_unit.py b/tests/protocols/misc/test_pcapng_flags_undefined_1382_unit.py index b0da80cdf1..f543c45b1c 100644 --- a/tests/protocols/misc/test_pcapng_flags_undefined_1382_unit.py +++ b/tests/protocols/misc/test_pcapng_flags_undefined_1382_unit.py @@ -1,9 +1,9 @@ # -*- coding: utf-8 -*- """GitHub issue #1382: undefined ``epb_flags`` sub-field values round-trip. -A direction of ``0b11`` or a reception type of ``0b101`` to ``0b111`` raised -:exc:`~pcapkit.utilities.exceptions.ProtocolError`, so a capture carrying one -could be neither parsed nor rebuilt. Both now resolve to an ``Unassigned`` +Without the fix, a direction of ``0b11`` or a reception type of ``0b101`` to +``0b111`` raises :exc:`~pcapkit.utilities.exceptions.ProtocolError`, so a capture +carrying one can be neither parsed nor rebuilt. Both resolve to an ``Unassigned`` member that keeps the wire value. Bits 9 to 11, which pcapng-06 Table 5 defines (checksum not ready, checksum valid, TCP segmentation offloaded), are named instead of sitting in ``reserved``. @@ -26,7 +26,7 @@ def _pb_with_flags(word: 'bytes') -> 'bytes': class TestPCAPNGFlagsUndefined(PCAPNGTestCase): - """Pin how undefined and newly named flags bits parse and rebuild.""" + """Pin how undefined flags bits, and the named bits 9 to 11, parse and rebuild.""" def _flags(self, word: 'str') -> 'tuple[bytes, object]': from pcapkit.const.pcapng.option_type import OptionType diff --git a/tests/protocols/misc/test_pcapng_kept_as_captured_1413_1414_unit.py b/tests/protocols/misc/test_pcapng_kept_as_captured_1413_1414_unit.py index e844def35a..89354358f9 100644 --- a/tests/protocols/misc/test_pcapng_kept_as_captured_1413_1414_unit.py +++ b/tests/protocols/misc/test_pcapng_kept_as_captured_1413_1414_unit.py @@ -1,13 +1,14 @@ # -*- coding: utf-8 -*- """GitHub issues #1413 and #1414: malformed PCAP-NG blocks rebuild byte-exactly. -* #1413 -- a Journal Export binary field filling its entry with no closing - newline gained one on rebuild (28 octets in, 32 out), and a bare ``KEY`` with - no ``=`` and no newline was dropped (16 in, 12 out). Under the #1406 ruling the - whole entry is now kept as captured, as ``entry_raw``. -* #1414 -- a Simple Packet Block holding more data than its ``original_len`` - lost the excess (24 in, 20 out), and an Enhanced Packet Block or Packet Block - below its 32-octet minimum rebuilt at 32. Both now keep the octets captured. +* #1413 -- guards against a Journal Export binary field filling its entry with + no closing newline gaining one on rebuild (28 octets in, 32 out), and a bare + ``KEY`` with no ``=`` and no newline being dropped (16 in, 12 out). Under the + #1406 ruling the whole entry is kept as captured, as ``entry_raw``. +* #1414 -- guards against a Simple Packet Block holding more data than its + ``original_len`` losing the excess (24 in, 20 out), and an Enhanced Packet + Block or Packet Block below its 32-octet minimum rebuilding at 32. Both keep + the octets captured. :mod:`pcapkit` is imported inside each test, after :func:`~tests._support.reimport_once_per_class`. diff --git a/tests/protocols/misc/test_pcapng_nrb_empty_names_1383_unit.py b/tests/protocols/misc/test_pcapng_nrb_empty_names_1383_unit.py index f0736ca764..9ae1e9be70 100644 --- a/tests/protocols/misc/test_pcapng_nrb_empty_names_1383_unit.py +++ b/tests/protocols/misc/test_pcapng_nrb_empty_names_1383_unit.py @@ -1,11 +1,11 @@ # -*- coding: utf-8 -*- """GitHub issue #1383: NRB record names round-trip byte-exactly. -The parse stripped every trailing NUL before splitting the name resolution -data, so ``a\\0\\0`` came back as one name and rebuilt as ``a\\0``. Names are now -split one per terminator, so an empty name is kept as ``''``. Data that is -empty or not zero-terminated cannot be expressed as a list of names; it is -malformed, and the record area is kept as the octets captured (#1325). +Without the fix, the parse strips every trailing NUL before splitting the name +resolution data, so ``a\\0\\0`` comes back as one name and rebuilds as ``a\\0``. +Names are split one per terminator, so an empty name is kept as ``''``. Data +that is empty or not zero-terminated cannot be expressed as a list of names; it +is malformed, and the record area is kept as the octets captured (#1325). :mod:`pcapkit` is imported inside each test, after :func:`~tests._support.reimport_once_per_class`. diff --git a/tests/protocols/misc/test_pcapng_option_area_kept_unit.py b/tests/protocols/misc/test_pcapng_option_area_kept_unit.py index 782a0cc5df..83a80d1f78 100644 --- a/tests/protocols/misc/test_pcapng_option_area_kept_unit.py +++ b/tests/protocols/misc/test_pcapng_option_area_kept_unit.py @@ -1,12 +1,12 @@ # -*- coding: utf-8 -*- """A PCAP-NG option area holding a malformed option is kept as captured. -GitHub issue #1325: an option or record whose declared length runs past its -area used to be clamped to the octets the area had left, so the block parsed -but rebuilt a different length or padding than it was read with. Now the whole -area is kept as the octets captured: the block's ``options`` (or ``records``) -are empty, ``options_raw`` (or ``records_raw``) holds the area, the block -rebuilds byte for byte, and a capture holding such a block still extracts. +This guards against GitHub issue #1325, where an option or record whose declared +length runs past its area was clamped to the octets the area had left, so the +block parsed but rebuilt a different length or padding than it was read with. +The whole area is kept as the octets captured: the block's ``options`` (or +``records``) are empty, ``options_raw`` (or ``records_raw``) holds the area, the +block rebuilds byte for byte, and a capture holding such a block still extracts. Every case builds its own octets in memory. :mod:`pcapkit` is imported inside each test, after :func:`~tests._support.reimport_once_per_class`. diff --git a/tests/protocols/misc/test_pcapng_padding_kept_1437_unit.py b/tests/protocols/misc/test_pcapng_padding_kept_1437_unit.py index c665def715..b785c25399 100644 --- a/tests/protocols/misc/test_pcapng_padding_kept_1437_unit.py +++ b/tests/protocols/misc/test_pcapng_padding_kept_1437_unit.py @@ -1,14 +1,14 @@ # -*- coding: utf-8 -*- """GitHub issue #1437 (part B): PCAP-NG padding rebuilds as captured. -#1436 made :class:`~pcapkit.corekit.fields.strings.PaddingField` pack the octets -it is given, but the PCAP-NG reader never handed them on, so non-zero padding -parsed silently and came back zeroed: the packet data padding of an SPB, EPB and -Packet Block, the secrets padding of a DSB, the IDB reserved octets, and the -padding of every option and NRB record. Octets after ``opt_endofopt`` were -dropped outright, shrinking the block. Each is now kept on the data model -- -padding only when it is not all zeros, the octets after ``opt_endofopt`` -whenever there are any -- and written back by ``make``. +:class:`~pcapkit.corekit.fields.strings.PaddingField` packs the octets it is +given (#1436). Without the fix, the PCAP-NG reader never hands them on, so +non-zero padding parses silently and comes back zeroed: the packet data padding +of an SPB, EPB and Packet Block, the secrets padding of a DSB, the IDB reserved +octets, and the padding of every option and NRB record. Octets after +``opt_endofopt`` are dropped outright, shrinking the block. Each is kept on the +data model -- padding only when it is not all zeros, the octets after +``opt_endofopt`` whenever there are any -- and written back by ``make``. Every case builds its own octets in memory and reads no capture. diff --git a/tests/protocols/misc/test_pcapng_registrar_identity_unit.py b/tests/protocols/misc/test_pcapng_registrar_identity_unit.py index 7c97232b45..0564bd8dcd 100644 --- a/tests/protocols/misc/test_pcapng_registrar_identity_unit.py +++ b/tests/protocols/misc/test_pcapng_registrar_identity_unit.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- """GitHub issue #1401: the PCAP-NG parser registrars warn only on a real displacement. -``register_block``, ``register_option``, ``register_record`` and -``register_secrets`` tested only whether ``code`` was present, so handing back -the entry already stored warned that it was being overwritten. They now apply -the #1400 guard: an equal entry is a silent no-op, and the entry a code shipped +Without the fix, ``register_block``, ``register_option``, ``register_record`` +and ``register_secrets`` test only whether ``code`` is present, so handing back +the entry already stored warns that it is being overwritten. They apply the +#1400 guard: an equal entry is a silent no-op, and the entry a code shipped with is accepted back after an override, which undoes it. Every registry is process-wide, so each case runs under diff --git a/tests/protocols/misc/test_pcapng_roundtrip_unit.py b/tests/protocols/misc/test_pcapng_roundtrip_unit.py index ea97b17eaa..6c9588918b 100644 --- a/tests/protocols/misc/test_pcapng_roundtrip_unit.py +++ b/tests/protocols/misc/test_pcapng_roundtrip_unit.py @@ -50,8 +50,9 @@ '0200 0800 00000000 00000000 0000 0000 28000000') #: A TLS key log whose labels interleave and which carries comments -- the -#: order and the comments are what #1271's data model used to drop. Padded to a -#: multiple of four octets, so that the Decryption Secrets Block needs no pad. +#: order and the comments are what #1271 guards the data model against +#: dropping. Padded to a multiple of four octets, so that the Decryption Secrets +#: Block needs no pad. TLS_LOG = ('# a comment\n' 'CLIENT_RANDOM 00 11\n' 'SERVER_HANDSHAKE_TRAFFIC_SECRET 22 33\n' diff --git a/tests/protocols/misc/test_pcapng_short_shb_idb_1422_unit.py b/tests/protocols/misc/test_pcapng_short_shb_idb_1422_unit.py index 966c273dc1..a0a93f0ae2 100644 --- a/tests/protocols/misc/test_pcapng_short_shb_idb_1422_unit.py +++ b/tests/protocols/misc/test_pcapng_short_shb_idb_1422_unit.py @@ -1,12 +1,12 @@ # -*- coding: utf-8 -*- """GitHub issue #1422: a PCAP-NG SHB or IDB shorter than its fixed fields. -A Section Header Block below 28 octets, or an Interface Description Block -below 20, read its fixed fields past the Block Total Length and rebuilt at the -full fixed size (SHB 16 or 20 to 28, IDB 12 or 16 to 20). The block body is -now kept as ``block_raw`` and written back at the declared length, as #1414 -does for EPB/PB. A 12-octet SHB has no Byte-Order Magic and takes the byte -order in which its Block Total Length reads as 12. +Without the fix, a Section Header Block below 28 octets, or an Interface +Description Block below 20, reads its fixed fields past the Block Total Length +and rebuilds at the full fixed size (SHB 16 or 20 to 28, IDB 12 or 16 to 20). +The block body is kept as ``block_raw`` and written back at the declared length, +as #1414 does for EPB/PB. A 12-octet SHB has no Byte-Order Magic and takes the +byte order in which its Block Total Length reads as 12. :mod:`pcapkit` is imported inside each test, after :func:`~tests._support.reimport_once_per_class`. @@ -25,7 +25,7 @@ #: Section and interface blocks below their fixed size, by issue length. SHORT = ('shb-12', 'shb-16', 'shb-20', 'shb-24', 'idb-12', 'idb-16') -#: The same blocks at their full fixed size, already byte-exact before #1422. +#: The same blocks at their full fixed size, byte-exact with or without the #1422 fix. FULL = ('shb-28', 'idb-20') diff --git a/tests/protocols/misc/test_pcapng_spb_snaplen_1384_unit.py b/tests/protocols/misc/test_pcapng_spb_snaplen_1384_unit.py index 8f34a2e33f..7ccd31a2f8 100644 --- a/tests/protocols/misc/test_pcapng_spb_snaplen_1384_unit.py +++ b/tests/protocols/misc/test_pcapng_spb_snaplen_1384_unit.py @@ -1,12 +1,12 @@ # -*- coding: utf-8 -*- """GitHub issue #1384: a Simple Packet Block longer than the snaplen keeps its excess. -An SPB holding 16 data octets under an interface snaplen of 8 parsed as 8 -octets: the other 8 were dropped, the trailing Block Total Length was read from -the packet data, and the rebuild was 8 octets shorter. The parse now keeps every -octet the block holds past the snaplen and its padding, warns that the data -exceeds the snaplen, and rebuilds byte-exactly. A block sized for the snaplen -still reads ``min(original_len, snaplen)`` octets. +Without the fix, an SPB holding 16 data octets under an interface snaplen of 8 +parses as 8 octets: the other 8 are dropped, the trailing Block Total Length is +read from the packet data, and the rebuild is 8 octets shorter. The parse keeps +every octet the block holds past the snaplen and its padding, warns that the +data exceeds the snaplen, and rebuilds byte-exactly. A block sized for the +snaplen still reads ``min(original_len, snaplen)`` octets. :mod:`pcapkit` is imported inside each test, after :func:`~tests._support.reimport_once_per_class`. diff --git a/tests/protocols/misc/test_pcapng_tzone_tsoffset_range_1340_unit.py b/tests/protocols/misc/test_pcapng_tzone_tsoffset_range_1340_unit.py index a52e869760..1925482e20 100644 --- a/tests/protocols/misc/test_pcapng_tzone_tsoffset_range_1340_unit.py +++ b/tests/protocols/misc/test_pcapng_tzone_tsoffset_range_1340_unit.py @@ -3,15 +3,15 @@ ``if_tzone`` is a signed 32-bit count of seconds, but a :class:`~datetime.timezone` holds strictly less than 24 hours, so an offset of -a day or more raised a bare :exc:`ValueError` out of the IDB reader. It is now -kept as captured, as a :class:`~datetime.timedelta`, with a +a day or more raises a bare :exc:`ValueError` out of the IDB reader without the +fix. It is kept as captured, as a :class:`~datetime.timedelta`, with a :class:`~pcapkit.utilities.warnings.ProtocolWarning`, and the block's timestamps render in UTC. ``if_tsoffset`` is a signed 64-bit count of seconds; an epoch past what -``datetime`` or the platform ``time_t`` holds raised :exc:`OverflowError` or -:exc:`OSError` from :meth:`datetime.datetime.fromtimestamp`, where only -:exc:`ValueError` fell back to the epoch with a warning. All three do now. +``datetime`` or the platform ``time_t`` holds raises :exc:`OverflowError` or +:exc:`OSError` from :meth:`datetime.datetime.fromtimestamp`, not only +:exc:`ValueError`. All three fall back to the epoch with a warning. Every block rebuilds byte-exactly from its data model. diff --git a/tests/protocols/misc/test_pcapng_unit.py b/tests/protocols/misc/test_pcapng_unit.py index 012428bd8d..6eb26bf2b5 100644 --- a/tests/protocols/misc/test_pcapng_unit.py +++ b/tests/protocols/misc/test_pcapng_unit.py @@ -130,8 +130,8 @@ def test_pcapng_registry_overwrites_warn(self) -> None: secrets_map[SecretsType.TLS_Key_Log] = originals[3] def test_pcapng_record_registry_warning_names_the_name_resolution_record(self) -> None: - # #1038: the warning used to call an ``nrb_record_*`` entry a - # "systemd(1) journal export record", with a literal reST role. + # #1038: guards against the warning calling an ``nrb_record_*`` entry + # a "systemd(1) journal export record", with a literal reST role. from pcapkit.const.pcapng.record_type import RecordType from pcapkit.protocols.misc.pcapng import PCAPNG @@ -150,9 +150,9 @@ def test_pcapng_record_registry_warning_names_the_name_resolution_record(self) - def test_pcapng_name_resolution_option_guards_name_the_name_resolution_block(self) -> None: # #1038: the ``ns_dns*`` scope guards check ``Name_Resolution_Block`` - # but used to tell the caller the option belongs in the systemd(1) - # Journal Export Block, and two make-side sites spelled the option - # ``ns_dnsip4addr``/``ns_dnsip6addr``. + # and name that block to the caller, not the systemd(1) Journal Export + # Block, and the make side spells the option + # ``ns_dnsIP4addr``/``ns_dnsIP6addr``, not ``ns_dnsip4addr``/``ns_dnsip6addr``. from pcapkit.const.pcapng.block_type import BlockType from pcapkit.const.pcapng.option_type import OptionType from pcapkit.corekit.multidict import OrderedMultiDict @@ -193,7 +193,7 @@ def test_pcapng_name_resolution_option_guards_name_the_name_resolution_block(sel self.assertNotIn('Journal Export', message) self.assertNotIn(':manpage:', message) - # The make-side duplicate guards share the corrected spelling. + # The make-side duplicate guards share the ``ns_dnsIP*addr`` spelling. pcapng._type = BlockType.Name_Resolution_Block for option, type_, ip in (('ns_dnsIP4addr', OptionType.ns_dnsIP4addr, '8.8.8.8'), ('ns_dnsIP6addr', OptionType.ns_dnsIP6addr, '2001:4860::8888')): @@ -437,12 +437,12 @@ def test_pcapng_context_timestamp_and_address_helpers(self) -> None: self.assertEqual(pcapng._make_timestamp(decimal.Decimal(12)), (0, 2000)) # #361: the epoch is 2000 units at if_tsresol=1000, i.e. 2s, plus the - # 10s if_tsoffset -- 12s, and nothing else. It used to come back as 3612, - # the same instant with this interface's if_tzone (+01:00) *added* to it, - # which is 3600s of pure error: a PCAP-NG timestamp is an offset from the - # UNIX epoch and so carries no timezone. The datetime is the one place - # ``tz`` still shows, as the zone the instant is rendered in, and it - # names the very same instant -- which the two returns did not before. + # 10s if_tsoffset -- 12s, and nothing else. Without the fix it comes back + # as 3612, the same instant with this interface's if_tzone (+01:00) + # *added* to it, which is 3600s of pure error: a PCAP-NG timestamp is an + # offset from the UNIX epoch and so carries no timezone. The datetime is + # the one place ``tz`` shows, as the zone the instant is rendered in, and + # it names the very same instant as the epoch. ts_datetime, ts_decimal = pcapng._read_timestamp(0, 2000) self.assertEqual(ts_datetime, datetime.datetime.fromtimestamp(12, tz)) self.assertEqual(ts_decimal, decimal.Decimal(12)) @@ -560,15 +560,15 @@ def now(cls, tz): pcapng._type = BlockType.Enhanced_Packet_Block # Faked through pcapng.py's own binding of # :func:`pcapkit.utilities.datetime.fromtimestamp`, which it calls - # instead of :meth:`datetime.datetime.fromtimestamp` since #2099. + # instead of :meth:`datetime.datetime.fromtimestamp` (#2099). with mock.patch('pcapkit.protocols.misc.pcapng.fromtimestamp', side_effect=ValueError), \ mock.patch('pcapkit.protocols.misc.pcapng.warn') as warn: timestamp, epoch = pcapng._read_timestamp(0, 1, interface_id=0) self.assertEqual(timestamp, datetime.datetime.fromtimestamp(0, datetime.timezone.utc)) - # #361: 1 unit at if_tsresol=1e9 plus the 2s if_tsoffset. The leading - # ``Decimal(7200)`` this used to carry was this interface's if_tzone - # (+02:00) leaking into the epoch; the fallback datetime above is aware - # UTC either way, so the two used to name different instants. + # #361: 1 unit at if_tsresol=1e9 plus the 2s if_tsoffset. A leading + # ``Decimal(7200)`` would be this interface's if_tzone (+02:00) leaking + # into the epoch; the fallback datetime above is aware UTC either way, + # so the two would name different instants. self.assertEqual(epoch, decimal.Decimal(2) + decimal.Decimal('0.000000001')) warn.assert_called_once() @@ -623,8 +623,8 @@ class Other(Raw): # ``register`` resolves a ``ModuleDescriptor`` before storing it, # so the entry above is already the plain ``Raw`` class, not the - # descriptor -- passing ``Raw`` again would now be the same-object - # no-op GitHub issue #718 added an identity guard for (see + # descriptor -- passing ``Raw`` again would be the same-object + # no-op covered by the identity guard for GitHub issue #718 (see # ``test_pcapng_register_stays_quiet_on_same_object_reregistration``). # A genuinely different class is needed to still warn here. with mock.patch('pcapkit.protocols.misc.pcapng.warn') as warn: @@ -756,9 +756,9 @@ class Other(Raw): def test_pcapng_register_stays_quiet_on_same_object_reregistration(self) -> None: """GitHub issue #718: replaying a call with the identical class is a no-op. - Before the fix the guard fired on presence alone, so the *second* of + Without the fix the guard fires on presence alone, so the *second* of two calls registering the exact same class under the exact same code - warned about an overwrite that never happened. Matches the identity + warns about an overwrite that never happens. Matches the identity guard :func:`register_protocol ` already applies. @@ -888,11 +888,11 @@ def test_pcapng_option_readers_cover_block_families_and_guards(self) -> None: # Per the mint/unmint ruling for #775 # (``docs/source/contributing/conventions/mint-criterion.rst``), # ``Unassigned`` names a procedure rather than a party, so ``FilterType``'s - # ``_missing_`` no longer mints a registered ``Unassigned_0`` member for + # ``_missing_`` does not mint a registered ``Unassigned_0`` member for # code 0 -- it returns an unregistered member bearing the bare label # ``Unassigned``. ``FilterType(0)`` is used below wherever the fixtures # need *some* valid ``FilterType`` value for code 0; this pins that the - # value obtained that way is the one the ruling now produces. + # value obtained that way is the one the ruling produces. self.assertEqual(FilterType(0).name, 'Unassigned') pcapng = object.__new__(PCAPNG) @@ -920,7 +920,7 @@ def with_length(schema: object, length: int) -> object: 'checksum_not_ready': 0, 'checksum_valid': 0, 'tcp_segmentation_offloaded': 0, - 'reserved': 0, # bits 12-23, kept since #1273 + 'reserved': 0, # bits 12-23, kept (#1273) 'crc_error': 1, 'too_long': 0, 'too_short': 1, @@ -2099,11 +2099,11 @@ def test_pcapng_remaining_constructor_branches_and_custom_dispatch(self) -> None # Per the mint/unmint ruling for #775 # (``docs/source/contributing/conventions/mint-criterion.rst``), # ``Unassigned`` names a procedure rather than a party, so ``FilterType``'s - # ``_missing_`` no longer mints a registered ``Unassigned_0`` member for + # ``_missing_`` does not mint a registered ``Unassigned_0`` member for # code 0 -- it returns an unregistered member bearing the bare label # ``Unassigned``. ``FilterType(0)`` is used below wherever the fixtures # need *some* valid ``FilterType`` value for code 0; this pins that the - # value obtained that way is the one the ruling now produces. + # value obtained that way is the one the ruling produces. self.assertEqual(FilterType(0).name, 'Unassigned') pcapng = object.__new__(PCAPNG) @@ -2311,7 +2311,7 @@ def assert_duplicate(method: str, code: OptionType, block: BlockType, **kwargs: checksum_not_ready=False, checksum_valid=False, tcp_segmentation_offloaded=False, - reserved=0, # bits 12-23, kept since #1273 + reserved=0, # bits 12-23, kept (#1273) crc_error=True, too_long=True, too_short=False, @@ -2402,7 +2402,7 @@ def assert_duplicate(method: str, code: OptionType, block: BlockType, **kwargs: checksum_not_ready=False, checksum_valid=False, tcp_segmentation_offloaded=False, - reserved=0, # bits 12-23, kept since #1273 + reserved=0, # bits 12-23, kept (#1273) crc_error=True, too_long=False, too_short=True, @@ -3445,8 +3445,8 @@ def test_sample_capture_timestamp_matches_its_own_raw_bytes(self) -> None: ``dhcp.pcapng`` is committed, and its interface carries ``if_tsresol=6`` with neither ``if_tsoffset`` nor ``if_tzone`` -- the shape #361 is - about, where the epoch used to be shifted by whatever zone the reading - machine sat in. + about, which guards against the epoch being shifted by whatever zone + the reading machine sits in. """ from pcapkit.interface import extract @@ -3515,15 +3515,15 @@ class PCAPNGOptionAreaBoundTests(unittest.TestCase): """An option may not synthesise more payload than its area declares. An option's length is a 16-bit wire field, so one four-octet option header - can declare 65,535 octets of payload. Nothing bounded that against the area - the option sits in, and every shortfall inside a 16-bit length is padded - unconditionally at the field layer. Repeating such an option across blocks - therefore amplified without limit. C.f. #594, and #593 for the 32-bit band + can declare 65,535 octets of payload. Every shortfall inside a 16-bit length + is padded unconditionally at the field layer, so without a bound against the + area the option sits in, repeating such an option across blocks amplifies + without limit. C.f. #594, and #593 for the 32-bit band the field layer's own budget already covers. An option running past its area is refused, and the block keeps the whole option area as the octets captured, as one :obj:`bytes` item that packs - back verbatim (#1325). It used to be clamped to what the area had left. + back verbatim (#1325), rather than clamped to what the area had left. """ @@ -3815,8 +3815,8 @@ def test_an_option_cut_inside_its_header_keeps_the_area(self) -> None: The first option takes eight octets, leaving two, so the loop runs again with two octets of area. The next option's type field takes both and its length field reads short, so it runs past the data and the area is kept - as captured. It used to parse as a second option of length zero, which - rebuilt two octets of length that were never there. + as captured. Parsing it as a second option of length zero would rebuild + two octets of length that were never there. """ options = (tlv(UNKNOWN_OPT, b'\xaa\xbb') @@ -3915,14 +3915,15 @@ def readinto(self, buffer) -> 'int': # type: ignore[no-untyped-def] class PCAPNGTruncatedFileTests(unittest.TestCase): """A capture cut short must still report the frames before the cut. - Block Total Length is cross-checked against its own trailing copy and never - against the file, so a last block running past the real end left the reader - seeked *past* that end -- which is legal and silent. Every block read after - it then measured a negative remainder, and the negative reached - :meth:`io.RawIOBase.read` through ``pcapng_block_selector`` as a bare - ``ValueError`` that no handler in the frame loop catches. The whole - extraction was lost, not the one truncated block, which inverts the #431 - accommodation exactly. Measured on ``dhcp.pcapng`` before the fix: of its + Without the fix, Block Total Length is cross-checked against its own + trailing copy and never against the file, so a last block running past the + real end leaves the reader seeked *past* that end -- which is legal and + silent. Every block read after it then measures a negative remainder, and + the negative reaches :meth:`io.RawIOBase.read` through + ``pcapng_block_selector`` as a bare ``ValueError`` that no handler in the + frame loop catches. The whole extraction is lost, not the one truncated + block, which inverts the #431 accommodation exactly. Measured on + ``dhcp.pcapng`` without the fix: of its 1,509 octet boundaries, **6** parsed and 1,489 raised something from outside :mod:`pcapkit.utilities.exceptions` -- 1,479 ``ValueError`` and 10 ``struct.error``. C.f. #678, #431, #571, and #676 for the option-area bound @@ -4017,7 +4018,7 @@ def test_almost_every_truncation_level_parses(self) -> None: Absence of a foreign exception would be satisfied by turning all 1,509 levels into a tidy in-library refusal, which is the failure this guards - against. Before the fix six levels parsed; the rest is what the fix is + against. Without the fix six levels parse; the rest is what the fix is for. """ @@ -4084,7 +4085,7 @@ def test_a_truncated_last_block_keeps_the_frames_before_it(self) -> None: self.assertEqual(len(boundary.frame), 3) def test_a_block_overrunning_the_file_is_reported_and_leaves_the_reader_at_the_end(self) -> None: - """The root cause, on its own: the seek that used to go past the end. + """The root cause, on its own: the seek must not go past the end. ``_read_fileng`` stops at the end of the file, so the octets it returned are the authority on where a truncated block really finishes -- and @@ -4151,8 +4152,8 @@ def test_the_floor_cannot_reach_a_stream_that_cannot_seek(self) -> None: called :meth:`~io.IOBase.tell` on the stream to record ``_seek_set`` before ``unpack`` runs, so an unseekable stream never reaches the floor. Pinned rather than assumed, because a ``seekable()`` guard on the floor - would have been unreachable code -- and because the behaviour it would - have been protecting is unchanged by this fix. + would be unreachable code -- and because what such a guard would protect + behaves the same with or without the floor. """ from pcapkit.protocols.misc.pcapng import PCAPNG @@ -4241,10 +4242,9 @@ def test_every_length_callback_in_the_module_is_floored_at_zero(self) -> None: Walks every schema in :mod:`pcapkit.protocols.schema.misc.pcapng` and drives every ``length`` - callback with the hostile packet above. Measured on the parent commit: 28 - of the 74 callbacks returned a negative, from ``-1`` on an ``epb_hash`` - declaring no payload to ``-16777248`` on an Enhanced Packet Block's - option area. + callback with the hostile packet above. Measured without the floor, the + callbacks return negatives from ``-1`` on an ``epb_hash`` declaring no + payload to ``-16777248`` on an Enhanced Packet Block's option area. """ from pcapkit.protocols.schema.misc import pcapng as module @@ -4409,14 +4409,14 @@ def _extract_journal(self, entry: bytes): return journals[0].data, caught def test_an_ordinary_journal_entry_is_not_broken_by_its_own_padding(self) -> None: - """A 32-bit-unaligned journal entry used to raise a bare ``struct.error``. + """A 32-bit-unaligned journal entry does not raise a bare ``struct.error``. The block body is padded to a 32-bit boundary with NULs, and - ``bytes.strip()`` takes only ASCII whitespace -- so the padding survived - it and was read as the *name* of a binary field, whose 64-bit length - prefix then had nothing behind it. ``MESSAGE=hello\\n`` is 14 octets, so - two NULs follow it, and that is as ordinary as this block gets: the - failure was not confined to malformed input. + ``bytes.strip()`` takes only ASCII whitespace -- so without the fix the + padding survives it and is read as the *name* of a binary field, whose + 64-bit length prefix then has nothing behind it. ``MESSAGE=hello\\n`` is + 14 octets, so two NULs follow it, and that is as ordinary as this block + gets: the failure is not confined to malformed input. """ entries, caught = self._extract_journal(b'MESSAGE=hello\n') @@ -4504,9 +4504,9 @@ def test_a_journal_binary_field_declaring_more_than_its_entry_is_kept_as_capture def test_a_bad_terminator_keeps_the_entry_as_captured(self) -> None: """A binary field's value followed by anything but its newline (#1413). - The walk used to end only that entry's field collection and carry on, - which dropped the bad octet from the rebuild. The whole entry is kept as - captured instead, with or without a separator behind the bad octet. + Ending only that entry's field collection and carrying on would drop + the bad octet from the rebuild. The whole entry is kept as captured + instead, with or without a separator behind the bad octet. """ for tail in (b'GOOD=1\n', b'\n\nGOOD=1\n'): @@ -4519,13 +4519,13 @@ def test_a_binary_field_ending_the_entry_without_its_terminator_is_kept(self) -> self._assert_journal_kept(b'BIN\n' + struct.pack(' None: - """#704: skipping a binary field's terminator used to skip everything. + """#704: skipping a binary field's terminator does not skip everything. - ``entry_data.read()`` with no argument reads to *end of file*, not past - the one newline octet the format puts there, so the next ``readline()`` - found nothing and ended the entry. A single binary field cannot show - this -- there is nothing behind it to lose -- so the entry needs a - *second* binary field, and a text field after that, to tell a + ``entry_data.read()`` with no argument reads to *end of file*, not past the + one newline octet the format puts there, so a skip written that way leaves + the next ``readline()`` nothing and ends the entry. A single binary field + cannot show this -- there is nothing behind it to lose -- so the entry needs + a *second* binary field, and a text field after that, to tell a length-bounded skip from an unbounded one. """ @@ -4547,11 +4547,11 @@ def test_a_binary_last_field_before_a_trailing_separator_is_not_warned(self) -> """A blank-line separator is found by reading, not by pre-slicing. ``draft-richardson-opsawg-pcapng-extras-01`` lets an entry carry a - trailing separator. The old parser sliced the buffer into segments - with ``self.entry.split(b'\\n\\n')`` before reading a single field, so - the separator's own bytes were gone by the time a binary last field's - terminator was checked, and the check that landed in #722 warned over - a newline that was in the capture all along. Walking the buffer whole + trailing separator. A parser that slices the buffer into segments + with ``self.entry.split(b'\\n\\n')`` before reading a single field has + lost the separator's own bytes by the time a binary last field's + terminator is checked, so the terminator check (#722) warns over a + newline that was in the capture all along. Walking the buffer whole never removes those bytes -- the blank line ends the current entry by being *read*, and whatever the buffer still holds after it starts the next one -- so there is nothing here for that check to trip over. @@ -4584,19 +4584,18 @@ def test_a_binary_field_ending_the_first_of_two_entries_is_not_warned(self) -> N if item.category.__name__ == 'SchemaWarning'], []) def test_a_trailing_separator_with_no_padding_still_starts_an_entry(self) -> None: - """A trailing separator landing exactly on the last octet was dropped. + """A trailing separator landing exactly on the last octet is not dropped. The two tests above see their trailing separator followed by the block's own NUL padding, which the walk also treats as ending an - entry -- so the extra empty entry that padding produces stood in for - the one the separator itself should have produced, and the gap went - unnoticed. Seven octets of field plus one of separator is eight, a - multiple of four, so :meth:`_journal_block` adds none: the blank line - is the last octet in the buffer, ``entry_data.tell()`` reaches - ``total`` in the same read that found it, and the walk used to stop - right there without recording that a separator had been seen at all. - Rebuilding from the result then wrote a :attr:`length` one octet - short of what was actually read. + entry -- so the extra empty entry that padding produces stands in for + the one the separator itself should produce, and can hide a gap. + Seven octets of field plus one of separator is eight, a multiple of + four, so :meth:`_journal_block` adds none: the blank line is the last + octet in the buffer, and ``entry_data.tell()`` reaches ``total`` in the + same read that found it. A walk that stops right there without + recording that a separator had been seen at all rebuilds a + :attr:`length` one octet short of what was actually read. """ entries, caught = self._extract_journal(b'ABC=12\n' + b'\n') @@ -4610,8 +4609,8 @@ def test_a_trailing_separator_with_no_padding_still_starts_an_entry(self) -> Non def test_a_journal_binary_value_containing_a_blank_line_is_not_shredded(self) -> None: """#723 defect A: a length-prefixed value needs no escaping for ``\\n\\n``. - ``self.entry.split(b'\\n\\n')`` used to cut a binary field's own value - in the middle whenever that value happened to contain the entry + Splitting with ``self.entry.split(b'\\n\\n')`` cuts a binary field's own + value in the middle whenever that value happens to contain the entry separator, turning the tail of the value into a bogus field in a fabricated second entry. The value here carries two such separators; a parser that only counts bytes out by the declared length, and never @@ -4658,7 +4657,7 @@ def test_a_journal_binary_length_prefix_beginning_with_a_blank_line_is_not_shred if item.category.__name__ == 'SchemaWarning'], []) def test_a_journal_field_that_is_not_utf8_is_reported_and_kept_as_captured(self) -> None: - """One bad octet in one field used to cost the whole extraction. + """One bad octet in one field does not cost the whole extraction. :exc:`UnicodeDecodeError` is a :exc:`ValueError`, so it is neither in :mod:`pcapkit.utilities.exceptions` nor an :exc:`EOFError`. The octet is diff --git a/tests/protocols/misc/test_pcapng_zigbee_key_width_1986_unit.py b/tests/protocols/misc/test_pcapng_zigbee_key_width_1986_unit.py index 0614a9da81..9e59e820a5 100644 --- a/tests/protocols/misc/test_pcapng_zigbee_key_width_1986_unit.py +++ b/tests/protocols/misc/test_pcapng_zigbee_key_width_1986_unit.py @@ -3,11 +3,11 @@ The ZigBee NWK and APS key schemas have a fixed width, but a block's ``secrets_length`` may declare fewer octets or more. A key shorter than the -width parsed, but its data model rebuilt it at full width: the key zero-padded, -``secrets_length`` rewritten and the block grown. Octets past the width were -dropped, by the parse itself. The data model keeps the Secrets Data octets -as the block holds them, the schema keeps those past the fixed fields, and both -rebuild byte for byte. +width parses, but without the fix its data model rebuilds it at full width: the +key zero-padded, ``secrets_length`` rewritten and the block grown, and octets +past the width are dropped, by the parse itself. The data model keeps the +Secrets Data octets as the block holds them, the schema keeps those past the +fixed fields, and both rebuild byte for byte. :mod:`pcapkit` is imported inside each test, after :func:`~tests._support.reimport_once_per_class`. diff --git a/tests/protocols/misc/test_raw_linktype_dispatch_1502_unit.py b/tests/protocols/misc/test_raw_linktype_dispatch_1502_unit.py index 5696970fcf..dc9175510e 100644 --- a/tests/protocols/misc/test_raw_linktype_dispatch_1502_unit.py +++ b/tests/protocols/misc/test_raw_linktype_dispatch_1502_unit.py @@ -1,12 +1,13 @@ # -*- coding: utf-8 -*- """GitHub issue #1502: ``LINKTYPE_RAW`` (101) is dissected as IPv4 or IPv6. -Neither :attr:`Frame.__proto__ ` -nor :attr:`PCAPNG.__proto__ ` -had an entry for ``LinkType.RAW``, so a raw-IP frame was kept as -:class:`~pcapkit.protocols.misc.raw.Raw` and never reached reassembly. It +This guards against +:attr:`Frame.__proto__ ` and +:attr:`PCAPNG.__proto__ ` lacking +an entry for ``LinkType.RAW``, which keeps a raw-IP frame as +:class:`~pcapkit.protocols.misc.raw.Raw` so it never reaches reassembly. It carries a bare IPv4 or IPv6 datagram that only the version nibble tells apart, -so both now dispatch it as ``IPV4`` or ``IPV6`` by that nibble, unless ``RAW`` +so both dispatch it as ``IPV4`` or ``IPV6`` by that nibble, unless ``RAW`` itself is registered. Every layer is rebuilt byte-exactly, from ``info`` and from ``info.to_dict()``. diff --git a/tests/protocols/schema/test_enum_schema_registry_unit.py b/tests/protocols/schema/test_enum_schema_registry_unit.py index e8e943fe30..2563b79262 100644 --- a/tests/protocols/schema/test_enum_schema_registry_unit.py +++ b/tests/protocols/schema/test_enum_schema_registry_unit.py @@ -2,10 +2,10 @@ :class:`~pcapkit.protocols.schema.schema.EnumSchema`'s :attr:`registry` is a :class:`collections.defaultdict`-backed mapping of enumeration codes to schema -classes. Reading it with a bare ``registry[code]`` for a code nobody registered -used to *insert* that code -- with whatever the default factory produced -- the -same defect fixed at the protocol layer's ``__proto__`` family by GitHub issue -#421 and its option, chunk and block registries by #425. These tests cover +classes. These tests guard against a bare ``registry[code]`` read for a code +nobody registered *inserting* that code -- with whatever the default factory +produced -- the same defect fixed at the protocol layer's ``__proto__`` family +by GitHub issue #421 and its option, chunk and block registries by #425. They cover both the auto-created :attr:`EnumSchema.__enum__` (the shape used by e.g. :class:`pcapkit.protocols.schema.transport.tcp.Option`) and a manually @@ -257,11 +257,11 @@ class BaseSchema(EnumSchema[Code]): return Code, BaseSchema def test_register_warns_when_a_code_is_already_taken(self) -> None: - """The schema half now reports what it displaced. + """The schema half reports what it displaced. - Before this change the body was a bare ``cls.__enum__[code] = schema``: - the replacement happened, the read-side fell back to the new class, and - nothing connected that to the registration which caused it. + Guards against a bare ``cls.__enum__[code] = schema`` body, where the + replacement happens, the read-side falls back to the new class, and + nothing connects that to the registration which caused it. """ from pcapkit.utilities.warnings import RegistryWarning @@ -326,10 +326,10 @@ class Replacement(BaseSchema): def test_register_stays_quiet_on_same_object_reregistration(self) -> None: """GitHub issue #718: replaying a call with the identical schema is a no-op. - Before the fix the guard fired on presence alone, so a caller + Without the fix the guard fires on presence alone, so a caller idempotently re-asserting a binding it already made -- registering the exact same schema object under the exact same code a second time - -- warned about an overwrite that never happened, exactly as + -- warns about an overwrite that never happens, exactly as unhelpfully as a genuine collision. Matches the identity guard :func:`register_protocol ` already diff --git a/tests/protocols/schema/test_schema_child_finalisation_1498_unit.py b/tests/protocols/schema/test_schema_child_finalisation_1498_unit.py index 8769f831f2..aaf2dfdf3f 100644 --- a/tests/protocols/schema/test_schema_child_finalisation_1498_unit.py +++ b/tests/protocols/schema/test_schema_child_finalisation_1498_unit.py @@ -2,14 +2,14 @@ """GitHub issue #1498: each non-final :class:`~pcapkit.protocols.schema.schema.Schema` subclass is finalised on its own state, whatever was built before it. -:meth:`Schema.__new__ ` used to -read ``__finalised__`` through inheritance, so once a non-final parent had been -built its non-final children read the parent's ``BASE`` and were never -finalised. They kept the ``__excluded__`` copied at declaration, and -``to_dict()`` then included ``__map__``, ``__map_reverse__``, ``__buffer__`` and -``__updated__``. The Quick-Start option schemas of HOPOPT, IPv6-Opts and IPv4 -are the shipped instances: each is a non-final subclass of a non-final -``Option``. +Without the fix, +:meth:`Schema.__new__ ` reads +``__finalised__`` through inheritance, so once a non-final parent has been built +its non-final children read the parent's ``BASE`` and are never finalised. They +keep the ``__excluded__`` copied at declaration, and ``to_dict()`` then includes +``__map__``, ``__map_reverse__``, ``__buffer__`` and ``__updated__``. The +Quick-Start option schemas of HOPOPT, IPv6-Opts and IPv4 are the shipped +instances: each is a non-final subclass of a non-final ``Option``. The defect depends on process-wide state -- which class was built first -- so every order runs in a fresh interpreter, started from the repository root so it @@ -117,7 +117,7 @@ def method(self) -> 'None': self.assertIn('method', Child.__builtin__) def test_a_final_schema_under_a_base_state_ancestor_is_refused(self) -> None: - """A bare ``@final`` no longer escapes the guard by reading ``BASE`` off its parent.""" + """A bare ``@final`` does not escape the guard by reading ``BASE`` off its parent.""" from pcapkit.corekit.fields.numbers import UInt8Field from pcapkit.corekit.infoclass import FinalisedState from pcapkit.protocols.schema.schema import Schema diff --git a/tests/protocols/schema/test_schema_field_remainder_unit.py b/tests/protocols/schema/test_schema_field_remainder_unit.py index edacb8bf58..4283c07ce5 100644 --- a/tests/protocols/schema/test_schema_field_remainder_unit.py +++ b/tests/protocols/schema/test_schema_field_remainder_unit.py @@ -3,11 +3,11 @@ GitHub issue #1380: a nested schema may read less than its :class:`~pcapkit.corekit.fields.misc.SchemaField`'s declared span. Parsing then -packing kept the octets it left unread (#1292), but a schema rebuilt through +packing keeps the octets it left unread (#1292), but a schema rebuilt through :meth:`~pcapkit.protocols.schema.schema.Schema.from_dict`, or a protocol rebuilt -through :meth:`~pcapkit.protocols.protocol.ProtocolBase.from_data`, had never -been unpacked and zero-filled them. They now travel with the nested value as its -``__remainder__``. +through :meth:`~pcapkit.protocols.protocol.ProtocolBase.from_data`, has never +been unpacked, and without the fix zero-fills them. They travel with the nested +value as its ``__remainder__``. No shipped schema declares a :class:`~pcapkit.corekit.fields.misc.SchemaField` directly, so the schemas and the protocol here are built in the test. Everything diff --git a/tests/protocols/schema/test_schema_metaclass_abc_cache_unit.py b/tests/protocols/schema/test_schema_metaclass_abc_cache_unit.py index eadac36299..0588e28c0a 100644 --- a/tests/protocols/schema/test_schema_metaclass_abc_cache_unit.py +++ b/tests/protocols/schema/test_schema_metaclass_abc_cache_unit.py @@ -26,15 +26,15 @@ class SchemaMetaAbcCacheTests(unittest.TestCase): """Regression tests for GitHub issue #439. - ``SchemaMeta.__new__`` (:mod:`pcapkit.protocols.schema.schema`) used to bypass - :meth:`abc.ABCMeta.__new__` on Python < 3.11, to dodge a ``namespace=`` class - keyword collision described in that module. Bypassing it also skipped + The issue: ``SchemaMeta.__new__`` (:mod:`pcapkit.protocols.schema.schema`) + bypassed :meth:`abc.ABCMeta.__new__` on Python < 3.11, to dodge a ``namespace=`` + class keyword collision described in that module. Bypassing it also skipped :meth:`abc.ABCMeta.__new__`'s call to ``abc._abc_init(cls)``, so no :class:`~pcapkit.protocols.schema.schema.Schema` subclass ever got its own ``_abc_impl``. Attribute lookup then fell through the MRO to :class:`collections.abc.Mapping`'s ``_abc_impl`` -- the one ABC in the chain - still built through an untouched ``ABCMeta.__new__`` -- so *every* schema - class was reading and writing the same ``isinstance`` cache that + still built through an untouched ``ABCMeta.__new__`` -- so *every* schema class + was reading and writing the same ``isinstance`` cache that ``collections.abc.Mapping`` itself uses. :class:`dict` is a registered virtual subclass of ``Mapping``, so asking @@ -166,7 +166,7 @@ def _make_shared_hierarchy() -> 'tuple[type, type]': Neither class is given its own ``_abc_impl``, so attribute lookup falls through the MRO to :class:`collections.abc.Mapping`'s -- exactly - the shape ``SchemaMeta.__new__`` used to produce on Python < 3.11, + the shape #439's bypass in ``SchemaMeta.__new__`` produced on Python < 3.11, confirmed below rather than assumed. Building this on plain ``abc.ABCMeta`` rather than ``SchemaMeta`` keeps the two tests below about ``abc.ABCMeta`` in general, not about this library's fix. @@ -255,10 +255,10 @@ def test_the_pcapng_1248_symptom_no_longer_reproduces(self) -> None: :meth:`NameResolutionBlock.post_process ` asks ``isinstance(record, (IPv4Record, IPv6Record))`` and then reads - ``record.names``. Before #439 was fixed, every schema class shared one - ``_abc_impl``, so that question answered ``True`` for the block's + ``record.names``. Without the #439 fix, every schema class shares one + ``_abc_impl``, so that question answers ``True`` for the block's *terminating* ``EndRecord`` -- which has no ``names`` -- provided - *anything at all* had already asked whether that same ``EndRecord`` + *anything at all* has already asked whether that same ``EndRecord`` instance was a :class:`~pcapkit.protocols.schema.schema.Schema`. That is a lower bar than it sounds: ``isinstance(record, Schema)`` is @@ -298,7 +298,7 @@ def test_the_pcapng_1248_symptom_no_longer_reproduces(self) -> None: @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class SchemaMetaReservedClassKwargsTests(unittest.TestCase): - """Regression tests for the reserved-class-keyword guard added with #439. + """Regression tests for the reserved-class-keyword guard from #439. Renaming ``namespace=`` to ``ns=`` (see :mod:`pcapkit.protocols.schema.misc. pcapng`) fixed the one collision that was actually in use, but the diff --git a/tests/protocols/schema/test_schema_unit.py b/tests/protocols/schema/test_schema_unit.py index 388338f0e1..3b2d9deb9c 100644 --- a/tests/protocols/schema/test_schema_unit.py +++ b/tests/protocols/schema/test_schema_unit.py @@ -110,9 +110,9 @@ def test_list_field_pack_accepts_a_tuple_like_it_accepts_a_list(self) -> None: See #476: several data models declare a ``ListField``-backed attribute as ``tuple[...]`` (HIP's ``group_id``, MH's ``prefixes``/``fid``/``bid``, and others), and ``_read_*`` hands one straight back to ``_make_*`` on a - parse-then-reconstruct cycle. Before the fix, :meth:`Schema.pack - ` accepted a :obj:`list` but - raised :exc:`ProtocolUnbound` on the tuple -- a case no unit test that + parse-then-reconstruct cycle. Without the fix, :meth:`Schema.pack + ` accepts a :obj:`list` but + raises :exc:`ProtocolUnbound` on the tuple -- a case no unit test that builds the schema directly with a list could ever see. """ @@ -130,11 +130,10 @@ def test_list_field_pack_accepts_a_tuple_like_it_accepts_a_list(self) -> None: self.assertEqual(bytes(as_tuple), bytes(as_list)) self.assertEqual(bytes(as_tuple), b'\x09\xab\x10\x11\x44\x00\x00body') - # The branch still rejects what it always rejected: a tuple is accepted - # because it is a sequence ``ListField.pack`` can iterate, not because - # the check grew permissive. A :obj:`str` is also a sequence but is not - # what any data model here declares, so it stays out, same as - # ``object()`` did before this fix. + # The branch stays strict: a tuple is accepted because it is a sequence + # ``ListField.pack`` can iterate, not because the check is permissive. A + # :obj:`str` is also a sequence but is not what any data model here + # declares, so it stays out, as ``object()`` does. with self.assertRaises(ProtocolUnbound): bytes(FeatureSchema( kind=1, repeated='xy', # type: ignore[arg-type] @@ -266,7 +265,7 @@ class ManySchema(BaseEnumSchema, code=[Code.two, Code.three]): self.assertIs(BaseEnumSchema.registry[Code.three], ManySchema) # ``Code.two`` is held by ``ManySchema``, so this is a genuine overwrite - # and now reports one. Captured and asserted rather than left to escape: + # and reports one. Captured and asserted rather than left to escape: # an unasserted warning is noise in every later run of the suite, and the # capture is what stops this line from quietly becoming a second, silent # copy of the behaviour ``EnumSchemaRegistryOverwriteTests`` pins. @@ -288,14 +287,10 @@ class ManySchema(BaseEnumSchema, code=[Code.two, Code.three]): def test_schema_final_generated_init(self) -> None: """``schema_final`` generates a typed ``__init__`` from the field table. - This used to also cover a ``sys.version_info``-mocked "legacy version" - branch in ``SchemaMeta.__new__``, which no longer exists: that branch - was GitHub issue #439's workaround for a class-keyword collision with - ``abc.ABCMeta.__new__`` on Python < 3.11, and the fix removed it (and - the ``sys`` import along with it) in favour of renaming the one - keyword that actually collided. ``SchemaMeta.__new__`` no longer - branches on the interpreter version at all, so there is nothing left - for a mocked ``sys.version_info`` to exercise here. + ``SchemaMeta.__new__`` does not branch on the interpreter version: the + class-keyword collision with ``abc.ABCMeta.__new__`` on Python < 3.11 is + avoided by renaming the one keyword that collided (GitHub issue #439), + so there is nothing for a mocked ``sys.version_info`` to exercise here. """ from pcapkit.corekit.fields.numbers import UInt8Field @@ -339,11 +334,11 @@ class HeaderSchema(Schema): # ``NO_VALUE`` the generated ``__init__`` seeded them with self.assertEqual(schema.to_dict(), {'kind': 9, 'size': 0x0102}) - # so the schema packs, where before the fix the fields left out reached + # so the schema packs, where without the fix the fields left out reach # the packing as the field objects themselves self.assertEqual(bytes(schema), b'\x09\x01\x02\x00') - # and the two construction paths now agree + # and the two construction paths agree self.assertEqual(bytes(HeaderSchema.from_dict({'kind': 9})), bytes(schema)) # a packet context is what makes packing at construction possible, so it @@ -605,13 +600,13 @@ class PreviewedItem(Schema): def test_forward_match_field_does_not_count_toward_length(self) -> None: """A ``ForwardMatchField`` reads octets but must not be billed for them. - Before the fix, :meth:`Schema.unpack - ` kept the octets a + Without the fix, :meth:`Schema.unpack + ` keeps the octets a :class:`~pcapkit.corekit.fields.misc.ForwardMatchField` read in ``__buffer__`` even though it rewinds the stream past them, so - ``len(schema)`` double-counted them: the same octet is read once by + ``len(schema)`` double-counts them: the same octet is read once by ``length_peek`` (kept in the buffer) and again for real by ``length`` - (also kept), so a 3-octet input reported length 4. See #446. + (also kept), so a 3-octet input reports length 4. See #446. """ PreviewedItem = self._make_previewed_item_schema() @@ -631,7 +626,7 @@ def test_schema_list_field_rejects_a_declared_area_that_a_forward_match_over_rep Two ``PreviewedItem``s take two octets each off the wire -- four in total -- and :class:`~pcapkit.corekit.fields.collections.ListField` - is given exactly that as its declared ``length``. Before the fix, each + is given exactly that as its declared ``length``. Without the fix, each item's over-reported ``len(data)`` (3, not 2) drains the budget one octet too fast: ``4 - 3 = 1`` after the first item, then ``1 - 3 = -2`` on the second, and :meth:`ListField.unpack diff --git a/tests/protocols/test_capture_roundtrip_runtime.py b/tests/protocols/test_capture_roundtrip_runtime.py index cae3444d7f..81b1ccd0b2 100644 --- a/tests/protocols/test_capture_roundtrip_runtime.py +++ b/tests/protocols/test_capture_roundtrip_runtime.py @@ -62,8 +62,8 @@ class Gap(NamedTuple): #: Every (capture, frame, layer path) whose object-form rebuild does not close. -#: Frame numbers are one-based, as in the dumps. Empty since #1449 fixed the -#: IPv6 extension-header rebuild (#1446). +#: Frame numbers are one-based, as in the dumps. Empty: the IPv6 +#: extension-header rebuild closes (#1446, #1449). LAYER_GAPS: 'dict[str, Gap]' = {} class DictGap(NamedTuple): diff --git a/tests/protocols/test_construction_keyword_check_unit.py b/tests/protocols/test_construction_keyword_check_unit.py index 4d3ca55f2d..db06673c43 100644 --- a/tests/protocols/test_construction_keyword_check_unit.py +++ b/tests/protocols/test_construction_keyword_check_unit.py @@ -2,15 +2,15 @@ """A construction keyword no signature declares is refused, not discarded. Every ``make`` in the tree ends its signature with ``**kwargs: 'Any'`` and reads -nothing out of it, so until issue #617 a keyword it did not declare was accepted, -dropped, and the field it named kept its default. That is the worst shape a defect -can take in a packet library: the octets are wrong, nothing says so, and the -mapping that produced them reads correctly. +nothing out of it, so without the check from issue #617 a keyword it does not +declare is accepted, dropped, and the field it names keeps its default. That is +the worst shape a defect can take in a packet library: the octets are wrong, +nothing says so, and the mapping that produced them reads correctly. The cost is not hypothetical. Issue #602 was :data:`examples.generators.options.TCP_BASE` asking for ``seq=1`` where :meth:`TCP.make ` spells the parameter -``seq_no``. Measured on the tree before this fix:: +``seq_no``. Measured without the fix:: declared seq = 1 built info.seq = 0 warnings captured: [] @@ -20,7 +20,7 @@ was that somebody read the mapping against the signature. Issues #541 and #556 were the same family of silence. -The schema layer has never been so permissive: :meth:`Schema.__update__ +The schema layer is not so permissive: :meth:`Schema.__update__ ` warns :exc:`~pcapkit.utilities.warnings.UnknownFieldWarning` for a field it does not know (:file:`pcapkit/protocols/schema/schema.py`). The asymmetry between the two @@ -187,9 +187,9 @@ def test_the_three_misspellings_of_the_defect_are_each_refused(self) -> None: """Each key of #602, put back one at a time, raises and is named. ``TCP_BASE`` is correct, so reproducing the report means - reintroducing the misspelling. Before the fix each of these built a - segment whose field kept its default -- ``seq=1`` giving ``info.seq == 0`` - -- and captured no warning at all. + reintroducing the misspelling. Without the fix each of these builds a + segment whose field keeps its default -- ``seq=1`` giving ``info.seq == 0`` + -- and captures no warning at all. """ from pcapkit.protocols.transport.tcp import TCP @@ -519,14 +519,14 @@ def test_from_data_survives_the_injected_packet_keyword(self) -> None: declare it. :class:`~pcapkit.protocols.misc.null.NoPayload` is the case that matters, because :meth:`ProtocolBase._make_payload ` reaches it for the - innermost layer of every packet: refusing ``packet`` broke ``from_data`` - for all of them. Measured before ``packet`` was made out-of-band:: + innermost layer of every packet: refusing ``packet`` breaks ``from_data`` + for all of them. Measured with ``packet`` not out-of-band:: UnsupportedCall: NoPayload: unexpected keyword(s): 'packet' What is asserted is that the rebuild happens at all. Whether it reproduces the original octets is a separate question about ``from_data``'s fidelity, - which this change neither improves nor worsens. + which this test does not pin. """ from pcapkit.protocols.internet.ipv4 import IPv4 @@ -546,8 +546,8 @@ def test_a_dispatcher_may_decline_the_check(self) -> None: only ``version`` and forwards every other keyword to :class:`HTTPv1 ` or :class:`HTTPv2 ` according to - that value, so no fixed set of names is correct for it. Measured before the - opt-out existed:: + that value, so no fixed set of names is correct for it. Measured without + the opt-out:: UnsupportedCall: HTTP: unexpected keyword(s): 'http_version', 'method', 'uri' @@ -558,9 +558,8 @@ def test_a_dispatcher_may_decline_the_check(self) -> None: ``make`` produces from these keywords -- is the proof that they reached the delegate rather than being refused on the way. - Until #1041 that packet failed to read back, as ``ProtocolError: HTTP: - invalid format``, because ``httpv1.HTTP`` refused a message with no header - fields; this test asserted that error as its proof of delegation. + The packet reads back as HTTP/1.1 (#1041), although it carries no + header fields. :meth:`HTTPUnitTests.test_http_construction_reaches_the_versioned_make_callee` pins the same outcome from the HTTP side. @@ -576,9 +575,9 @@ def test_a_dispatcher_may_decline_the_check(self) -> None: # The opt-out is not inherited. Both versioned classes declare their # keywords in full, and exempting them along with their base would forgo - # the check on the only two HTTP classes that can carry it -- which is what - # happened on the first attempt at this, measured before the ``klass is - # cls`` guard: every HTTP subclass came back unchecked. + # the check on the only two HTTP classes that can carry it -- measured + # without the ``klass is cls`` guard, every HTTP subclass comes back + # unchecked. for versioned, keyword in ((HTTPv1, 'http_version'), (HTTPv2, 'sid')): with self.subTest(protocol=versioned.__module__): accepted = _declared_keywords(versioned) @@ -635,7 +634,7 @@ def _make_data(cls, data: 'Any') -> 'dict[str, Any]': @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class AsymmetryTests(unittest.TestCase): - """The two layers, and what each now does with a name it does not know.""" + """The two layers, and what each does with a name it does not know.""" def test_the_schema_layer_still_warns_where_construction_now_raises(self) -> None: """One unknown name, two layers, both of them audible. diff --git a/tests/protocols/test_declared_length_overrun_unit.py b/tests/protocols/test_declared_length_overrun_unit.py index a2503f1c05..fd90186f27 100644 --- a/tests/protocols/test_declared_length_overrun_unit.py +++ b/tests/protocols/test_declared_length_overrun_unit.py @@ -1,14 +1,14 @@ # -*- coding: utf-8 -*- """An option declaring more octets than are present is rejected, not zero-filled. -GitHub issue #1325: :meth:`OptionField.unpack +This guards against GitHub issue #1325, where :meth:`OptionField.unpack ` let an option schema read past the end of its data, and every field reading off the end padded the missing octets with zeros. A HIP ``ACK`` declaring 256 octets with 4 present parsed as 64 update IDs and rebuilt to 304 octets; an MH ``PadN`` declaring 3 octets with none present rebuilt 24 octets from 16. -Such an option now raises :exc:`~pcapkit.utilities.exceptions.ProtocolError`. +Such an option raises :exc:`~pcapkit.utilities.exceptions.ProtocolError`. Inside a parent layer the octets are kept as captured -- raw extension data under IPv6, a :class:`~pcapkit.protocols.misc.raw.Raw` payload elsewhere -- and the parent rebuilds byte for byte. diff --git a/tests/protocols/test_declared_length_roundtrip_runtime.py b/tests/protocols/test_declared_length_roundtrip_runtime.py index fac0d56830..208e97273c 100644 --- a/tests/protocols/test_declared_length_roundtrip_runtime.py +++ b/tests/protocols/test_declared_length_roundtrip_runtime.py @@ -1,19 +1,21 @@ # -*- coding: utf-8 -*- """``from_data`` keeps the length a truncated capture declared. -GitHub issue #1155. :meth:`IPv4.make `, +This guards against GitHub issue #1155, where +:meth:`IPv4.make `, :meth:`UDP.make ` and :meth:`IPv6.make ` computed their length field from the payload they were handed. On a capture cut short by the snaplen that is the *recorded* length rather than the declared one, so a :meth:`~pcapkit.protocols.protocol.ProtocolBase.from_data` rebuild of :file:`big_endian.pcap` frame 3 rewrote the IPv4 total length from 1186 to 82 and -the UDP length from 0x048e to 0x003e. Each ``make`` now takes the length +the UDP length from 0x048e to 0x003e. Each ``make`` takes the length explicitly, and ``_make_data`` hands it the parsed one; a construction that gives no length still computes it. The round-trip cases read generated captures, so this module belongs to the -fixture-dependent tier. The in-memory cases pin that construction is unchanged. +fixture-dependent tier. The in-memory cases pin construction with and without +an explicit length. """ from __future__ import annotations diff --git a/tests/protocols/test_dispatch_bindings_unit.py b/tests/protocols/test_dispatch_bindings_unit.py index e8662f9171..6eb99dbceb 100644 --- a/tests/protocols/test_dispatch_bindings_unit.py +++ b/tests/protocols/test_dispatch_bindings_unit.py @@ -5,9 +5,9 @@ tier and reads none of the generated captures under :file:`examples/captures/`. What it pins is that a *registered* code actually reaches a working dissector. -The tables and the dissectors were previously able to disagree without anything -noticing -- :class:`~pcapkit.protocols.application.ospf.OSPF` was reachable from no -table at all and could not have parsed a packet if it had been -- so each case +The tables and the dissectors can disagree without anything else noticing -- a +protocol can be reachable from no table at all, and unable to parse a packet if +it were -- so each case asserts on the parsed protocol chain rather than on the table entry alone. """ diff --git a/tests/protocols/test_dispatch_default_resolution_unit.py b/tests/protocols/test_dispatch_default_resolution_unit.py index aff9428086..15171efb08 100644 --- a/tests/protocols/test_dispatch_default_resolution_unit.py +++ b/tests/protocols/test_dispatch_default_resolution_unit.py @@ -8,8 +8,8 @@ defect GitHub issue #421 reported and pull request #426 fixed at this layer, pull request #428 extended to the option, chunk and block registries, and pull request #560 fixed at the schema layer. The cost of not writing it back is that -every unrecognised frame resolves the same descriptor again. Since GitHub issue -#1559 a registered descriptor is not written back either, so an extraction of +every unrecognised frame resolves the same descriptor again. A registered +descriptor is not written back either (GitHub issue #1559), so an extraction of :file:`many_interfaces.pcapng` performs 296 resolutions, 48 of them fallbacks. Proposed by @Ts-Boom in GitHub pull request #563, which paid that cost with a @@ -24,8 +24,8 @@ gone, the other two say what it was not allowed to cost: :meth:`DefaultDescriptorResolutionTests.test_repeated_miss_does_not_re_enter_the_import_machinery` - The saving itself, as a call count rather than as a timing. Fails on the - unfixed tree with one :func:`~importlib.import_module` call per lookup. + The saving itself, as a call count rather than as a timing. Fails without + the fix with one :func:`~importlib.import_module` call per lookup. :meth:`DefaultDescriptorResolutionTests.test_a_missed_code_can_still_be_registered_without_a_warning` The half that any write-back-on-miss implementation breaks: a code that has diff --git a/tests/protocols/test_frame_layer_limit_1610_unit.py b/tests/protocols/test_frame_layer_limit_1610_unit.py index a19be4b73b..55b8bf82c7 100644 --- a/tests/protocols/test_frame_layer_limit_1610_unit.py +++ b/tests/protocols/test_frame_layer_limit_1610_unit.py @@ -1,14 +1,15 @@ # -*- coding: utf-8 -*- """A frame is dissected to 64 layers, and no further. -GitHub issue #1610: each layer dissects the next one from inside its own parse, -so deep nesting of any protocol ran into Python's recursion limit, and -:func:`~pcapkit.corekit.decorators.beholder` caught the ``RecursionError`` and -left the rest raw, silently. From a script on Python 3.14, TCP was lost behind -108 IPv4 headers in IPv4, or behind 200 stacked 802.1Q tags. #1604 bounded one -chain of extension headers, but its count restarts at every tunnel. - -Now every frame stops at +This guards against GitHub issue #1610: each layer dissects the next one from +inside its own parse, so without a limit deep nesting of any protocol runs into +Python's recursion limit, and :func:`~pcapkit.corekit.decorators.beholder` +catches the ``RecursionError`` and leaves the rest raw, silently. From a script +on Python 3.14, TCP was lost behind 108 IPv4 headers in IPv4, or behind 200 +stacked 802.1Q tags. #1604 bounds one chain of extension headers, but its count +restarts at every tunnel. + +Every frame stops at :data:`~pcapkit.utilities.config.FRAME_LAYER_LIMIT` layers, counted along its protocol chain from the first layer its record carries, through tunnels, tags and extension headers alike: what follows is kept as @@ -223,7 +224,8 @@ def test_the_limit_is_64(self) -> None: self.assertEqual(config.FRAME_LAYER_LIMIT, 64) def test_a_frame_of_64_layers_is_dissected_to_the_tcp(self) -> None: - # Guard: below the old recursion cut-off, so this held before #1610 too. + # Guard: below the recursion cut-off #1610 is about, so this holds + # without the fix too. for shape, build in SHAPES.items(): with self.subTest(shape=shape): parsed, messages = self.frame(ethernet(build(64))) @@ -283,7 +285,8 @@ def test_what_a_walked_header_dissects_counts_after_every_header_walked(self) -> def test_where_both_limits_fall_on_one_layer_it_warns_once(self) -> None: # Guard: the 33rd header in a row is also the 65th layer, and the - # extension header limit, which held before #1610 too, is the one named. + # extension header limit, which holds without the #1610 limit too, is the + # one named. for carrier in ('ipv4', 'ipv6'): with self.subTest(carrier=carrier): layers = ['ipv4'] * 30 + [carrier] + ['opts'] * 40 + ['tcp'] diff --git a/tests/protocols/test_ipv4_l2tp_padding_guard_1437_unit.py b/tests/protocols/test_ipv4_l2tp_padding_guard_1437_unit.py index 9c069e858f..1643757e03 100644 --- a/tests/protocols/test_ipv4_l2tp_padding_guard_1437_unit.py +++ b/tests/protocols/test_ipv4_l2tp_padding_guard_1437_unit.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- -"""GitHub issue #1437 (part B): IPv4 and L2TP padding already rebuild as captured. +"""GitHub issue #1437 (part B): IPv4 and L2TP padding rebuild as captured. The issue listed the IPv4 Quick-Start report "Not Used" octet, the IPv4 octets after the End of Option List, and the L2TPv2 offset pad as sites that might -still zero-fill on rebuild. Measured on ``main`` at ``16c87f901``, all of them -already round-trip byte for byte, so nothing changed; these cases guard that. +still zero-fill on rebuild. All of them round-trip byte for byte (measured at +``16c87f901``); these cases guard that. Every case builds its own octets in memory and reads no capture. Classes are imported inside each test, after :func:`~tests._support.reimport_once_per_class`. diff --git a/tests/protocols/test_ipv4_tcp_header_overrun_unit.py b/tests/protocols/test_ipv4_tcp_header_overrun_unit.py index c2d32a052c..f945e7f2e7 100644 --- a/tests/protocols/test_ipv4_tcp_header_overrun_unit.py +++ b/tests/protocols/test_ipv4_tcp_header_overrun_unit.py @@ -1,17 +1,19 @@ # -*- coding: utf-8 -*- """A header length declared past the data, and TS address keys given as strings. -GitHub issue #1404: an IPv4 ``ihl`` or a TCP Data Offset declaring more header +This guards against GitHub issue #1404, where an IPv4 ``ihl`` or a TCP Data +Offset declaring more header than the data holds read the missing octets as zeros, and ``from_data`` rebuilt them -- 34 octets in and 38 out for an Ethernet frame whose IPv4 ``ihl`` is 6 -with only 20 header octets present. Such a header now raises +with only 20 header octets present. Such a header raises :exc:`~pcapkit.utilities.exceptions.ProtocolError`, so its parent keeps the octets as a :class:`~pcapkit.protocols.misc.raw.Raw` payload and rebuilds them byte for byte. -GitHub issue #1402: ``IPv4.make()`` called :func:`int` on each key of a TS -option's ``timestamp`` mapping, so a dotted-quad string key raised a bare -:exc:`ValueError`. The keys are now converted as addresses, and a :obj:`bool` +It also guards against GitHub issue #1402, where ``IPv4.make()`` called +:func:`int` on each key of a TS option's ``timestamp`` mapping, so a dotted-quad +string key raised a bare :exc:`ValueError`. The keys are converted as +addresses, and a :obj:`bool` key is rejected with :exc:`~pcapkit.utilities.exceptions.FieldValueError`. Every case builds its own octets in memory and reads no capture. diff --git a/tests/protocols/test_make_index_str_unit.py b/tests/protocols/test_make_index_str_unit.py index ea8ccae827..b0e1c4c72f 100644 --- a/tests/protocols/test_make_index_str_unit.py +++ b/tests/protocols/test_make_index_str_unit.py @@ -2,10 +2,10 @@ """``make()`` resolves an enum member name given as :obj:`str`. GitHub issue #1210: the ``make()`` signatures declare enum arguments as -``Enum | StdlibEnum | AenumEnum | str | int``, but -:meth:`~pcapkit.protocols.protocol.ProtocolBase._make_index` raised +``Enum | StdlibEnum | AenumEnum | str | int``, but without the fix +:meth:`~pcapkit.protocols.protocol.ProtocolBase._make_index` raises :exc:`~pcapkit.utilities.exceptions.ProtocolNotImplemented` for a :obj:`str` -unless a ``*_namespace`` was passed. A member name now resolves against the +unless a ``*_namespace`` is passed. A member name resolves against the enumeration class of the argument's default value, and builds the same bytes as the member itself and as its integer value. diff --git a/tests/protocols/test_make_raw_item_read_unit.py b/tests/protocols/test_make_raw_item_read_unit.py index a7bbd0b0ab..399debb452 100644 --- a/tests/protocols/test_make_raw_item_read_unit.py +++ b/tests/protocols/test_make_raw_item_read_unit.py @@ -1,12 +1,13 @@ # -*- coding: utf-8 -*- """A raw ``make()`` item rejected by ``read`` raises the wrapped error. -GitHub issue #1394: :meth:`~pcapkit.protocols.protocol.ProtocolBase.unpack` -wrapped only the schema re-parse of a made header holding raw items. A raw -item the schema accepts but ``read`` rejects -- an option whose length runs -past the option area, or one cut short of its own fields -- escaped as the -reader's bare :exc:`~pcapkit.utilities.exceptions.ProtocolError`. The read is -now covered too: the error says ``malformed raw item given to make()``, +This guards against GitHub issue #1394, where +:meth:`~pcapkit.protocols.protocol.ProtocolBase.unpack` wrapped only the schema +re-parse of a made header holding raw items. A raw item the schema accepts but +``read`` rejects -- an option whose length runs past the option area, or one cut +short of its own fields -- escaped as the reader's bare +:exc:`~pcapkit.utilities.exceptions.ProtocolError`. The read is covered too: +the error says ``malformed raw item given to make()``, carries the reader's message, and chains it as ``__cause__``. An item whose fields run past the octets given is refused by the schema instead (#1325), and is wrapped the same way. diff --git a/tests/protocols/test_next_layer_hit_not_memoised_1559_unit.py b/tests/protocols/test_next_layer_hit_not_memoised_1559_unit.py index 3a36464cfb..c1518d04be 100644 --- a/tests/protocols/test_next_layer_hit_not_memoised_1559_unit.py +++ b/tests/protocols/test_next_layer_hit_not_memoised_1559_unit.py @@ -1,19 +1,19 @@ # -*- coding: utf-8 -*- """GitHub issue #1559: a next-layer hit is resolved, not memoised. -:meth:`ProtocolBase._lookup_next_layer -` used to write the -class a registered :class:`~pcapkit.corekit.module.ModuleDescriptor` resolved to -back over that descriptor. Three consequences, one test each: - -* after :func:`importlib.reload` of the protocol's module, dispatch kept reaching - the pre-reload class, so a parsed layer failed :func:`isinstance` against the - live one -- what the method's own docstring said a memo would do; -* parsing mutated registries shared by the whole process, so what a registry - held depended on what had been parsed before; -* so whether re-registering a shipped code warned depended on it too. - -Every lookup now resolves through :attr:`ModuleDescriptor.klass +This guards against :meth:`ProtocolBase._lookup_next_layer +` writing the class a +registered :class:`~pcapkit.corekit.module.ModuleDescriptor` resolved to back +over that descriptor. That has three consequences, one test each: + +* after :func:`importlib.reload` of the protocol's module, dispatch keeps + reaching the pre-reload class, so a parsed layer fails :func:`isinstance` + against the live one -- what the method's own docstring says a memo would do; +* parsing mutates registries shared by the whole process, so what a registry + holds depends on what has been parsed before; +* so whether re-registering a shipped code warns depends on it too. + +Every lookup resolves through :attr:`ModuleDescriptor.klass `, which reads :data:`sys.modules`. Each test reloads or parses on a private import, under :func:`~tests._support.isolate_modules`, so neither a reloaded module nor a @@ -122,12 +122,12 @@ def test_a_parse_leaves_the_shared_registries_as_declared(self) -> None: self.assertEqual(dict(registry), before[name]) def test_registering_a_dispatched_code_warns_alike_before_and_after_a_parse(self) -> None: - """Whether ``register`` warns no longer depends on what was parsed. + """Whether ``register`` warns does not depend on what was parsed. ``register`` warns when it displaces an incumbent other than the class it is handed, and a shipped descriptor counts as different from the - class it names. While a hit wrote the class back, re-registering UDP - warned before any parse and was silent after one had reached UDP. + class it names. If a hit wrote the class back, re-registering UDP + would warn before any parse and be silent after one had reached UDP. """ from pcapkit.const.reg.transtype import TransType diff --git a/tests/protocols/test_next_layer_short_header_unit.py b/tests/protocols/test_next_layer_short_header_unit.py index e2f0f3b20c..ade26efd9a 100644 --- a/tests/protocols/test_next_layer_short_header_unit.py +++ b/tests/protocols/test_next_layer_short_header_unit.py @@ -1,13 +1,14 @@ # -*- coding: utf-8 -*- """A payload captured short of the next layer's header is kept as raw octets. -GitHub issue #1170. Every ``_import_next_layer`` parsed the next layer from +This guards against GitHub issue #1170, where every ``_import_next_layer`` +parsed the next layer from whatever octets were captured, however few, and only picked :class:`~pcapkit.protocols.misc.null.NoPayload` for a declared length of zero. :meth:`Schema.unpack ` zero-fills the fields the stream ends inside, so :meth:`~pcapkit.protocols.protocol.ProtocolBase.from_data` rebuilt a header that -was never captured. A payload with no captured octets is now a ``NoPayload``, +was never captured. A payload with no captured octets is a ``NoPayload``, and one short of the next layer's header a :class:`~pcapkit.protocols.misc.raw.Raw`. diff --git a/tests/protocols/test_option_generator_1337_unit.py b/tests/protocols/test_option_generator_1337_unit.py index 6d870ac3b4..285ea4f2f7 100644 --- a/tests/protocols/test_option_generator_1337_unit.py +++ b/tests/protocols/test_option_generator_1337_unit.py @@ -1,16 +1,14 @@ # -*- coding: utf-8 -*- -"""What #1337 changed in :file:`examples/generators/options.py`. +"""Generator code in :file:`examples/generators/options.py` covered by #1337. -Two comments there justified code by limitations that no longer hold. - -* ``_httpv2_rebuild`` withheld ``flags``, on the grounds that ``HTTP.make`` - ignored it. Since #1341 ``make`` ORs ``flags`` into the bits the frame - constructor derives, and that constructor knows only the bits its frame type - defines. So a parsed frame with an undefined flag bit, or the reserved bit, - lost it on the generator's rebuild although ``HTTP.from_data`` keeps it. -* ``SKIP`` left out TCP ``EOOL`` and ``NOP`` as "dropped by - ``_make_tcp_options``". Since #1163 that maker emits them as given, so each is - a distinct header that round-trips, and both now have a case. +* ``_httpv2_rebuild`` passes ``flags`` and ``reserved`` on. ``HTTP.make`` ORs + ``flags`` into the bits the frame constructor derives (#1341), and that + constructor knows only the bits its frame type defines, so without them a + parsed frame with an undefined flag bit, or the reserved bit, loses it on the + generator's rebuild although ``HTTP.from_data`` keeps it. +* ``SKIP`` does not leave out TCP ``EOOL`` and ``NOP``. ``_make_tcp_options`` + emits them as given (#1163), so each is a distinct header that round-trips, + and both have a case. This module is unit tier: it builds everything in memory and reads no capture. @@ -79,7 +77,7 @@ def _load_generator() -> 'types.ModuleType': @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class OptionGenerator1337Tests(unittest.TestCase): - """The generator code the stale #1337 comments used to justify.""" + """The generator code #1337 covers.""" options = None # type: types.ModuleType diff --git a/tests/protocols/test_option_generator_deadline_scale_1428_unit.py b/tests/protocols/test_option_generator_deadline_scale_1428_unit.py index bd5c40973f..c4f1268ec3 100644 --- a/tests/protocols/test_option_generator_deadline_scale_1428_unit.py +++ b/tests/protocols/test_option_generator_deadline_scale_1428_unit.py @@ -3,24 +3,24 @@ :func:`roundtrip` in :file:`examples/generators/options.py` arms a :func:`signal.alarm` around each case and records ``'TIMEOUT'`` when it fires. -The budget was a flat 5 seconds of wall-clock time, so a stalled CI runner failed +A flat budget of 5 seconds of wall-clock time lets a stalled CI runner fail :mod:`tests.protocols.test_option_generator_httpv2_stream_unit` on a case that -takes about 1.3 ms. The default is now 30 seconds, the budget -:mod:`tests.protocols.test_option_roundtrip_unit` already gives the same cycle, +takes about 1.3 ms. The default is 30 seconds, the budget +:mod:`tests.protocols.test_option_roundtrip_unit` gives the same cycle, and ``PCAPKIT_TEST_TIMEOUT_SCALE`` stretches it exactly as it stretches :func:`tests._support.time_limit`. The spin that stands in for a hang is bounded (#1889). Unbounded, a deadline -whose raise was lost -- one raised inside a finaliser is reported and dropped -- -spun until pytest-timeout's backstop ended the whole xdist worker. Now it gives up -after :data:`SPIN_LIMIT` and the test fails by name, and the generator repeats -the raise until :func:`roundtrip` has it. +whose raise is lost -- one raised inside a finaliser is reported and dropped -- +spins until pytest-timeout's backstop ends the whole xdist worker. Bounded, it +gives up after :data:`SPIN_LIMIT` and the test fails by name, and the generator +repeats the raise until :func:`roundtrip` has it. The spin waits in a call made from inside the ``try`` that drops a raise, and arms the deadline there (#1897). Waiting inline, a raise handled at the jump -closing the loop went past the ``except`` on CPython 3.13; armed as -:func:`roundtrip` entered it, the deadline could come due in a stall before the -``try``. Either way the first raise was not dropped. +closing the loop goes past the ``except`` on CPython 3.13; armed as +:func:`roundtrip` enters it, the deadline can come due in a stall before the +``try``. Either way the first raise is not dropped. This module is unit tier: it loads the generator by path and builds nothing, so it imports no :mod:`pcapkit` and reads no capture. @@ -276,7 +276,7 @@ def assert_dropped_once(self, spin: '_Spin', outcome: 'Any') -> None: self.assertEqual(signal.alarm(0), 0) def test_the_default_is_thirty_seconds_unscaled(self) -> None: - """Unset, the default applies as written -- and it is no longer 5.""" + """Unset, the default applies as written -- 30, not a flat 5.""" self.assertEqual(self.options.DEADLINE, 30) self.assertEqual(self.options.TIMEOUT_SCALE_ENV, SCALE_ENV) self.assertIn(self.armed(self.options.DEADLINE), (29, 30)) diff --git a/tests/protocols/test_option_generator_httpv2_stream_unit.py b/tests/protocols/test_option_generator_httpv2_stream_unit.py index 212653a5a2..bccfa0f2b9 100644 --- a/tests/protocols/test_option_generator_httpv2_stream_unit.py +++ b/tests/protocols/test_option_generator_httpv2_stream_unit.py @@ -3,14 +3,14 @@ :file:`examples/captures/options-transport.pcap` carries one TCP segment to port 80 per HTTP/2 frame type, built by ``_httpv2_build`` in -:file:`examples/generators/options.py`. That function put every frame on stream -0. :rfc:`9113#section-6` makes ``DATA``, ``HEADERS``, ``PRIORITY``, -``RST_STREAM``, ``PUSH_PROMISE`` and ``CONTINUATION`` on stream 0 a connection -error, and since #1153 :meth:`HTTP._guess_version +:file:`examples/generators/options.py`. :rfc:`9113#section-6` makes ``DATA``, +``HEADERS``, ``PRIORITY``, ``RST_STREAM``, ``PUSH_PROMISE`` and ``CONTINUATION`` +on stream 0 a connection error, and :meth:`HTTP._guess_version ` declines a frame no -conforming sender writes, so six of the ten segments decoded as ``TCP:Raw``. +conforming sender writes (#1153), so a function putting every frame on stream 0 +decodes six of the ten segments as ``TCP:Raw``. :mod:`tests.protocols.test_option_coverage_runtime` checks only the -``Ethernet:IPv4`` prefix, so nothing noticed. +``Ethernet:IPv4`` prefix, so it cannot notice. :data:`EXPECTED_STREAM` spells each type's stream out rather than reading it from the generator, so a wrong generator constant fails here as ``0 != 1``. diff --git a/tests/protocols/test_option_generator_jumbogram_1687_unit.py b/tests/protocols/test_option_generator_jumbogram_1687_unit.py index d4b7907c21..e13f8d4139 100644 --- a/tests/protocols/test_option_generator_jumbogram_1687_unit.py +++ b/tests/protocols/test_option_generator_jumbogram_1687_unit.py @@ -2,12 +2,12 @@ """The generator's Jumbo Payload frame is a jumbogram RFC 2675 accepts. C.f. #1687. :file:`examples/generators/options.py` puts HOPOPT's Jumbo Payload case in an -IPv6 envelope in :file:`examples/captures/options-ipv6.pcap`. It used to carry -the option's default Jumbo Payload Length of 0 behind a non-zero Payload Length, -two of the errors :rfc:`2675#section-3` lists, and since #1687 the default -engine rejects such a packet: the frame stopped at IPv6 and its option was never -walked. The case now carries a legal length, and ``_frame`` builds a genuine -jumbogram around it, filled out by ``_jumbogram_datagram``. +IPv6 envelope in :file:`examples/captures/options-ipv6.pcap`. The option's +default Jumbo Payload Length of 0 behind a non-zero Payload Length is two of the +errors :rfc:`2675#section-3` lists, and the default engine rejects such a packet +(#1687): the frame stops at IPv6 and its option is never walked. So the case +carries a legal length, and ``_frame`` builds a genuine jumbogram around it, +filled out by ``_jumbogram_datagram``. :data:`JUMBO_LEN` and the datagram's fields are spelled out here rather than read from the generator, so a wrong generator constant fails as ``0 != 65536``. diff --git a/tests/protocols/test_option_generator_tcp_base_unit.py b/tests/protocols/test_option_generator_tcp_base_unit.py index feeb5f66ce..ab6281f90c 100644 --- a/tests/protocols/test_option_generator_tcp_base_unit.py +++ b/tests/protocols/test_option_generator_tcp_base_unit.py @@ -7,7 +7,8 @@ description of every one of those fixture frames, and a key it spells wrongly is a description that does not match the octets. -Nothing warned when one did. :meth:`TCP.make +Without a check upstream, nothing warns when the mapping spells a key wrongly. +:meth:`TCP.make ` ends its signature with ``**kwargs: 'Any'`` and never reads anything out of it, so a keyword it does not declare is accepted, discarded, and the parameter it was meant to set keeps its @@ -21,7 +22,7 @@ which is the acknowledgement *flag*, not the acknowledgement number ``ack_no``. The pair was the wrong way round, and neither half said so. -That silence is gone: issue #617 has :meth:`ProtocolBase.__init__ +Issue #617 has :meth:`ProtocolBase.__init__ ` check a construction keyword against the protocol's signatures and raise :exc:`~pcapkit.utilities.exceptions.UnsupportedCall` for one that no signature @@ -29,7 +30,7 @@ it -- the check is upstream of it -- so the account above remains the account of how the defect was possible, and :meth:`TCPBaseKeywordTests.test_an_undeclared_keyword_is_now_refused_rather_than_absorbed` -is where the new behaviour is pinned. +is where the refusal is pinned. Why the expected header is written out here ------------------------------------------- @@ -246,16 +247,13 @@ def test_tcp_base_is_the_expected_header(self) -> None: def test_an_undeclared_keyword_is_now_refused_rather_than_absorbed(self) -> None: """Constructing with an undeclared keyword raises. - This test used to assert the opposite, and the change is the point. - ``make`` still ends its signature with ``**kwargs`` and still reads - nothing out of it, so on its own it would still absorb a misspelling; what - changed in #617 is that :meth:`ProtocolBase.__init__ - ` now checks the - keywords against the signatures before ``make`` is reached, and refuses a - name none of them declares. That closes the asymmetry with - :class:`~pcapkit.protocols.schema.schema.Schema` construction, which has - always warned :exc:`~pcapkit.utilities.warnings.UnknownFieldWarning` for - an unknown field. + ``make`` ends its signature with ``**kwargs`` and reads nothing out of it, + so on its own it would absorb a misspelling; :meth:`ProtocolBase.__init__ + ` checks the keywords + against the signatures before ``make`` is reached, and refuses a name none + of them declares (#617). That closes the asymmetry with + :class:`~pcapkit.protocols.schema.schema.Schema` construction, which warns + :exc:`~pcapkit.utilities.warnings.UnknownFieldWarning` for an unknown field. The ``**kwargs`` assertion is kept because it says why the check has to live outside ``make``: were the signature closed, Python would raise on @@ -281,7 +279,7 @@ def test_an_undeclared_keyword_is_now_refused_rather_than_absorbed(self) -> None self.assertIn('no_such_tcp_field', str(context.exception)) # A warning would be the weaker fix the issue weighed and rejected; the - # silence here is now the silence of an exception having been raised. + # silence here is the silence of an exception having been raised. self.assertEqual([str(warning.message) for warning in caught], []) def test_the_three_keys_of_the_defect_are_each_refused(self) -> None: @@ -289,9 +287,9 @@ def test_the_three_keys_of_the_defect_are_each_refused(self) -> None: ``TCP_BASE`` is correct, so the mapping itself cannot demonstrate the defect. Putting the three keys back one at a time is what - keeps the original report reproducible: before the fix every one of them - built a segment whose field kept its default -- ``seq=1`` produced - ``info.seq == 0`` -- with an empty ``warnings`` list. + keeps the original report reproducible: without the fix every one of + them builds a segment whose field keeps its default -- ``seq=1`` + produces ``info.seq == 0`` -- with an empty ``warnings`` list. """ from pcapkit.protocols.transport.tcp import TCP diff --git a/tests/protocols/test_pack_keeps_instance_state_1769_unit.py b/tests/protocols/test_pack_keeps_instance_state_1769_unit.py index 4943d7672a..d95471b250 100644 --- a/tests/protocols/test_pack_keeps_instance_state_1769_unit.py +++ b/tests/protocols/test_pack_keeps_instance_state_1769_unit.py @@ -2,9 +2,10 @@ """Calling ``pack()`` leaves the instance it is called on as it was. C.f. #1769. :meth:`ProtocolBase.pack ` builds -a fresh header from its keywords alone. It used to assign that header to the -instance as well, so a bare ``UDP(srcport=1234, dstport=53).pack()`` left -``schema.srcport`` at ``0`` while ``info.srcport`` was still ``1234``, and +a fresh header from its keywords alone. This guards against it assigning that +header to the instance as well, where a bare +``UDP(srcport=1234, dstport=53).pack()`` left ``schema.srcport`` at ``0`` while +``info.srcport`` was still ``1234``, and :meth:`TCP.make ` overwrote :attr:`~pcapkit.protocols.transport.tcp.TCP.connection` on the way. The ``pack`` overrides of ``Frame`` and ``PCAPNG`` did the same, and HTTP/2's diff --git a/tests/protocols/test_padding_kept_as_captured_unit.py b/tests/protocols/test_padding_kept_as_captured_unit.py index dff56305c1..13d60bd232 100644 --- a/tests/protocols/test_padding_kept_as_captured_unit.py +++ b/tests/protocols/test_padding_kept_as_captured_unit.py @@ -1,10 +1,11 @@ # -*- coding: utf-8 -*- """Padding and reserved octets held by a ``PaddingField`` rebuild as captured. -GitHub issue #1223. :class:`~pcapkit.corekit.fields.strings.PaddingField` packed -zeros whatever it had read, so non-zero padding or reserved octets parsed -silently and came back zeroed. The field now packs the octets it was given, -and zeros when it has none, which is still what a freshly built packet writes. +This guards against GitHub issue #1223, where +:class:`~pcapkit.corekit.fields.strings.PaddingField` packed zeros whatever it +had read, so non-zero padding or reserved octets parsed silently and came back +zeroed. The field packs the octets it was given, and zeros when it has none, +which is what a freshly built packet writes. Each protocol below carries the octets through its reader, data model and ``make()``; the HIP and SCTP trailing alignment padding is kept on the data model only when it is not all zeros. diff --git a/tests/protocols/test_parser_registrar_identity_unit.py b/tests/protocols/test_parser_registrar_identity_unit.py index 5bcaeedbe3..8a34a0c406 100644 --- a/tests/protocols/test_parser_registrar_identity_unit.py +++ b/tests/protocols/test_parser_registrar_identity_unit.py @@ -1,11 +1,11 @@ # -*- coding: utf-8 -*- """GitHub issues #1362 and #1363: the parser registrars warn only on a real displacement. -The registrars that store a parser -- a method name or a ``(parser, -constructor)`` pair -- tested only whether ``code`` was present, so handing back -the entry already stored warned that it was being overwritten. They now compare -the incumbent with the replacement, as the class-storing registrars do since -#718 and #739: an equal entry is a silent no-op. Under the #1363 ruling, the +Without the fix, the registrars that store a parser -- a method name or a +``(parser, constructor)`` pair -- test only whether ``code`` is present, so +handing back the entry already stored warns that it is being overwritten. They +compare the incumbent with the replacement, as the class-storing registrars do +(#718 and #739): an equal entry is a silent no-op. Under the #1363 ruling, the entry a code shipped with is accepted back after an override, which undoes it. Every registry is process-wide, so each case runs under diff --git a/tests/protocols/test_pcapng_regression.py b/tests/protocols/test_pcapng_regression.py index aa3447fbc3..d099b617de 100644 --- a/tests/protocols/test_pcapng_regression.py +++ b/tests/protocols/test_pcapng_regression.py @@ -324,9 +324,9 @@ def test_a_snapped_block_carries_the_octets_that_are_present(self) -> None: def test_packet_header_and_payload_partition_the_block_octets(self) -> None: """``packet.header`` is the octets ahead of the payload, not the whole block. - The header used to be the entire block -- ``_read_packet(header=self.length)`` - with ``length`` being the Block Total Length -- which is the same bug seen - from the other side. + Guards against the header being the entire block -- + ``_read_packet(header=self.length)`` with ``length`` being the Block Total + Length -- which is the same bug seen from the other side. """ with tempfile.TemporaryDirectory() as temp: @@ -405,9 +405,9 @@ def test_dumped_pcap_carries_the_payload_octets(self) -> None: self.assertGreaterEqual(orig_len, incl_len) # 24 octets of global header, then 16 of record header and the payload per - # record, with nothing left over. Before #646 this file was 104 octets for - # these four blocks -- the global header and four record headers, and not one - # octet of payload -- which is both the wrong size and, worse, a size a + # record, with nothing left over. Without the #646 fix this file is 104 octets + # for these four blocks -- the global header and four record headers, and not + # one octet of payload -- which is both the wrong size and, worse, a size a # reader cannot tell is wrong until it has already lost frame sync. self.assertEqual(len(dumped), 24 + sum(16 + len(payload) for payload in expected)) self.assertNotEqual(len(dumped), 104) diff --git a/tests/protocols/test_protocol_base_unit.py b/tests/protocols/test_protocol_base_unit.py index 5db35cd3c1..257b9bfb07 100644 --- a/tests/protocols/test_protocol_base_unit.py +++ b/tests/protocols/test_protocol_base_unit.py @@ -54,11 +54,11 @@ def length(self) -> int: # NOTE: ``value`` and ``next_protocol`` are declared parameters rather # than names read out of ``**kwargs``, and ``make`` declares the - # un-prefixed parse limits, because since #617 a construction keyword - # no signature declares is refused rather than dropped. Both spellings - # are faithful to what the library does -- ``IPv4.make`` really does - # take a ``protocol`` -- and declaring them is the pattern #617 asks - # for, as against naming them in ``__keywords__``. + # un-prefixed parse limits, because a construction keyword no signature + # declares is refused rather than dropped (#617). Both spellings are + # faithful to what the library does -- ``IPv4.make`` really does take a + # ``protocol`` -- and declaring them is the pattern #617 asks for, as + # against naming them in ``__keywords__``. def read(self, length: int | None = None, *, value: int = 0, next_protocol: object = None, **kwargs: object) -> DummyData: data = DummyData(value=value) @@ -250,10 +250,11 @@ def test_register_analyze_and_next_layer_paths(self) -> None: class Other(Raw): pass - # GitHub issue #718 added an identity guard, so the second call needs - # a genuinely different class from the first to still warn -- see + # The identity guard for GitHub issue #718 keeps a same-object call + # silent, so the second call needs a genuinely different class from the + # first to warn -- see # ``test_register_stays_quiet_on_same_object_reregistration`` for the - # same-object case this used to (incorrectly) exercise. + # same-object case. with mock.patch('pcapkit.protocols.protocol.warn') as warn: DummyProtocol.register(1, Raw) DummyProtocol.register(1, Other) @@ -359,11 +360,10 @@ def __index__(cls) -> int: def test_register_stays_quiet_on_same_object_reregistration(self) -> None: """GitHub issue #718: replaying a call with the identical class is a no-op. - Before the fix the guard fired on presence alone, so the *second* of + Without the fix the guard fires on presence alone, so the *second* of two calls registering the exact same class under the exact same code - warned about an overwrite that never happened -- this is the case - ``test_register_analyze_and_next_layer_paths`` used to (incorrectly) - exercise. Matches the identity guard :func:`register_protocol + warns about an overwrite that never happens. Matches the identity + guard :func:`register_protocol ` already applies. diff --git a/tests/protocols/test_protocol_code_registration_unit.py b/tests/protocols/test_protocol_code_registration_unit.py index 23bb89836c..faa7ecf148 100644 --- a/tests/protocols/test_protocol_code_registration_unit.py +++ b/tests/protocols/test_protocol_code_registration_unit.py @@ -263,12 +263,11 @@ def test_iterable_mixes_inferred_and_explicit_targets(self) -> None: UDP port (explicit, since a bare port cannot say TCP or UDP, :rfc:`3931` ยง4.1.2) -- see #514's design thread. - Deliberately *not* ``L2TPv2``, which this docstring named until #548 was - settled: v2 answers on port 1701 only, and registering it at - ``TransType.L2TP`` points the :rfc:`2661` parser at a v3-over-IP header. - The mechanism under test is unchanged either way -- the class here is a - stand-in and nothing is really registered -- but the example should not - recommend a binding the library refuses. + Deliberately *not* ``L2TPv2`` (#548): v2 answers on port 1701 only, and + registering it at ``TransType.L2TP`` points the :rfc:`2661` parser at a + v3-over-IP header. The mechanism under test is unchanged either way -- the + class here is a stand-in and nothing is really registered -- but the example + should not recommend a binding the library refuses. """ from pcapkit.const.reg.transtype import TransType from pcapkit.foundation.registry.protocols import register_protocol_code diff --git a/tests/protocols/test_protocol_make_unit.py b/tests/protocols/test_protocol_make_unit.py index 86fb44b045..74f8b2fe33 100644 --- a/tests/protocols/test_protocol_make_unit.py +++ b/tests/protocols/test_protocol_make_unit.py @@ -1,19 +1,20 @@ # -*- coding: utf-8 -*- """Construction through ``make()`` in :class:`~pcapkit.protocols.protocol.Protocol`. -GitHub issue #1312: :meth:`~pcapkit.protocols.protocol.ProtocolBase.unpack` -reused the schema :meth:`~pcapkit.protocols.protocol.ProtocolBase.pack` had -made instead of parsing the packed octets, so ``read`` met the raw -:obj:`bytes` (or :class:`~pcapkit.protocols.schema.schema.Schema`) items -the option makers accept, and construction crashed with -:exc:`AttributeError`. A bare ``MH()`` crashed the same way on its default -``data=b'\\x00\\x00'``. A made schema holding raw octets where parsing its -packed form yields a schema is now replaced by the parsed one. - -GitHub issue #1308: a :obj:`str` member name given to two enum arguments -of one ``make()`` raised ``ambiguous member name`` when only one of the -enumerations had it. Each ``_make_index`` call now resolves against the -argument it was passed. +This guards against GitHub issue #1312, where +:meth:`~pcapkit.protocols.protocol.ProtocolBase.unpack` reused the schema +:meth:`~pcapkit.protocols.protocol.ProtocolBase.pack` had made instead of +parsing the packed octets, so ``read`` met the raw :obj:`bytes` (or +:class:`~pcapkit.protocols.schema.schema.Schema`) items the option makers +accept, and construction crashed with :exc:`AttributeError`. A bare ``MH()`` +crashed the same way on its default ``data=b'\\x00\\x00'``. A made schema +holding raw octets where parsing its packed form yields a schema is replaced by +the parsed one. + +It also guards against GitHub issue #1308, where a :obj:`str` member name given +to two enum arguments of one ``make()`` raised ``ambiguous member name`` when +only one of the enumerations had it. Each ``_make_index`` call resolves against +the argument it was passed. Every case builds its own packet in memory and reads no capture. Classes are imported inside each test, after diff --git a/tests/protocols/test_protocol_module_exports_1770_unit.py b/tests/protocols/test_protocol_module_exports_1770_unit.py index 5fbc05a867..b7a29d70f1 100644 --- a/tests/protocols/test_protocol_module_exports_1770_unit.py +++ b/tests/protocols/test_protocol_module_exports_1770_unit.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- """GitHub issue #1770: :mod:`pcapkit.protocols.protocol` exports ``Protocol``. -The module's :attr:`__all__` was ``['ProtocolBase']``, so ``from -pcapkit.protocols.protocol import *`` did not bind +This guards against the module's :attr:`__all__` being ``['ProtocolBase']``, +where ``from pcapkit.protocols.protocol import *`` does not bind :class:`~pcapkit.protocols.protocol.Protocol`, the class its docstring and -:file:`protocol.rst` present as the module's user-facing one. It is listed now. +:file:`protocol.rst` present as the module's user-facing one. """ diff --git a/tests/protocols/test_register_class_guard_unit.py b/tests/protocols/test_register_class_guard_unit.py index 6d85c2a0b9..10655ac3af 100644 --- a/tests/protocols/test_register_class_guard_unit.py +++ b/tests/protocols/test_register_class_guard_unit.py @@ -1,11 +1,11 @@ # -*- coding: utf-8 -*- """GitHub issue #1026: the ``register`` classmethods reject a non-class themselves. -Each ``register`` classmethod guarded with a bare ``issubclass(protocol, -ProtocolBase)``. A non-class argument is refused by ``issubclass`` itself -before the guard's own ``raise`` is reached, so the caller got ``TypeError: -issubclass() arg 1 must be a class`` leaked out of ``abc`` rather than -:class:`~pcapkit.utilities.exceptions.RegistryError`. Each now tests +A bare ``issubclass(protocol, ProtocolBase)`` guard is not enough: a non-class +argument is refused by ``issubclass`` itself before the guard's own ``raise`` is +reached, so the caller gets ``TypeError: issubclass() arg 1 must be a class`` +leaked out of ``abc`` rather than +:class:`~pcapkit.utilities.exceptions.RegistryError`. Each ``register`` tests ``isinstance(protocol, type)`` first, after the :class:`~pcapkit.corekit.module.ModuleDescriptor` is unwrapped, so a descriptor naming a non-class attribute is rejected too. @@ -58,7 +58,7 @@ def test_non_class_raises_registry_error_from_the_guard(self) -> None: klass.register(code, bad) # type: ignore[arg-type] self.assertIn('must be a class', str(caught.exception)) # ``RegistryError`` is a ``TypeError``, so callers catching - # the old exception keep working. + # ``TypeError`` keep working. self.assertIsInstance(caught.exception, TypeError) def test_wrong_class_still_raises_registry_error(self) -> None: diff --git a/tests/protocols/test_register_restores_shipped_descriptor_1504_unit.py b/tests/protocols/test_register_restores_shipped_descriptor_1504_unit.py index da89637f01..c804bdeb6c 100644 --- a/tests/protocols/test_register_restores_shipped_descriptor_1504_unit.py +++ b/tests/protocols/test_register_restores_shipped_descriptor_1504_unit.py @@ -1,15 +1,16 @@ # -*- coding: utf-8 -*- """GitHub issue #1504: restoring a shipped dispatch entry puts back its descriptor. +Without the fix, :meth:`ProtocolBase.register ` and its ``Link``, ``Internet``, ``Transport`` (``TCP``, ``UDP``), ``SCTP``, -``Frame`` and ``PCAPNG`` overrides resolved a +``Frame`` and ``PCAPNG`` overrides resolve a :class:`~pcapkit.corekit.module.ModuleDescriptor` argument to its class before storing it. The #1364 guard returns early only when the argument equals the -*incumbent*, so after an override, handing back the shipped descriptor stored -the class it names: the round trip did not close, and the restore imported the +*incumbent*, so after an override, handing back the shipped descriptor stores +the class it names: the round trip does not close, and the restore imports the module the entry was meant to defer. A descriptor equal to the one a code -shipped with is now matched before it is resolved, and the shipped object itself +shipped with is matched before it is resolved, and the shipped object itself is stored. Every registry is process-wide, so each case runs under diff --git a/tests/protocols/test_registry_stored_descriptor_unit.py b/tests/protocols/test_registry_stored_descriptor_unit.py index 8e8029cf41..a0afd868e6 100644 --- a/tests/protocols/test_registry_stored_descriptor_unit.py +++ b/tests/protocols/test_registry_stored_descriptor_unit.py @@ -1,15 +1,15 @@ # -*- coding: utf-8 -*- """GitHub issue #1364: re-registering a stored module descriptor keeps it. -``ProtocolBase.register`` and the ``Internet``, ``Link``, ``Transport``, -``SCTP`` (keyed by PPID), ``Frame`` and ``PCAPNG`` overrides resolved a -:class:`~pcapkit.corekit.module.ModuleDescriptor` argument to its class before -comparing it with the stored entry. Handing back the stored descriptor itself -therefore warned of an overwrite and replaced the deferred entry with the -class. The registrars now compare first: the stored entry, or a descriptor +Without the fix, ``ProtocolBase.register`` and the ``Internet``, ``Link``, +``Transport``, ``SCTP`` (keyed by PPID), ``Frame`` and ``PCAPNG`` overrides +resolve a :class:`~pcapkit.corekit.module.ModuleDescriptor` argument to its +class before comparing it with the stored entry, so handing back the stored +descriptor itself warns of an overwrite and replaces the deferred entry with +the class. The registrars compare first: the stored entry, or a descriptor equal to it, is a silent no-op that keeps the stored object, while a different entry still warns. ``Extractor.register_dumper`` is pinned alongside; #1376 -already keeps its shipped descriptors. +keeps its shipped descriptors. Every registry is process-wide, so each case runs under :func:`unittest.mock.patch.dict`. Classes are imported inside each test, after diff --git a/tests/protocols/test_schema_payload_zero_length_1275_unit.py b/tests/protocols/test_schema_payload_zero_length_1275_unit.py index b9121de443..c49b6882e3 100644 --- a/tests/protocols/test_schema_payload_zero_length_1275_unit.py +++ b/tests/protocols/test_schema_payload_zero_length_1275_unit.py @@ -71,7 +71,7 @@ def test_unset_length_stops_at_the_declared_length(self) -> None: The declared length is the authority even when the stream holds more: the octets past it belong to whatever encloses this schema, so reading them is an over-read that also leaves ``__length__`` mis-decremented -- - it used to come back *larger* than before the payload was read. + without the fix it comes back *larger* than before the payload was read. """ R = self._unset_schema() diff --git a/tests/protocols/test_short_read_roundtrip_unit.py b/tests/protocols/test_short_read_roundtrip_unit.py index b5af8cc7c8..161b716a13 100644 --- a/tests/protocols/test_short_read_roundtrip_unit.py +++ b/tests/protocols/test_short_read_roundtrip_unit.py @@ -1,12 +1,13 @@ # -*- coding: utf-8 -*- """A header the data ends inside rebuilds as captured, in every family. -GitHub issues #1458 and #1451: when the data ended inside a fixed-width field, -the parse read the missing octets as zeros and recorded nothing, so -``from_data(info)`` wrote the field -- and every field after it -- at full -width. ``Ethernet(bytes(12) + b'\\x08')`` rebuilt as 14 octets. +This guards against GitHub issues #1458 and #1451, where, when the data ended +inside a fixed-width field, the parse read the missing octets as zeros and +recorded nothing, so ``from_data(info)`` wrote the field -- and every field +after it -- at full width. ``Ethernet(bytes(12) + b'\\x08')`` rebuilt as 14 +octets. -:meth:`Schema.unpack ` now +:meth:`Schema.unpack ` records the field the data ends inside and how much of it was read, as ``__short_read__``; :meth:`Schema.pack ` writes back only those octets, and the layer base classes carry the record diff --git a/tests/protocols/test_to_dict_compare_1484_runtime.py b/tests/protocols/test_to_dict_compare_1484_runtime.py index 1af1a23d25..bb311028dc 100644 --- a/tests/protocols/test_to_dict_compare_1484_runtime.py +++ b/tests/protocols/test_to_dict_compare_1484_runtime.py @@ -1,11 +1,13 @@ # -*- coding: utf-8 -*- """Two exports of the same packet compare ``==`` and not ``!=``. -GitHub issue #1484. :meth:`Info.to_dict ` -returns an :class:`~pcapkit.corekit.multidict.OrderedMultiDict`, and Werkzeug's -class defines ``__eq__`` but no ``__ne__``: ``!=`` fell back to :obj:`dict`'s, -which compares the internal buckets, so ``info.to_dict() != info.to_dict()`` -held for every frame. The export now negates its ``__eq__``. +This guards against GitHub issue #1484. +:meth:`Info.to_dict ` returns an +:class:`~pcapkit.corekit.multidict.OrderedMultiDict`, and Werkzeug's class +defines ``__eq__`` but no ``__ne__``: without an override ``!=`` falls back to +:obj:`dict`'s, which compares the internal buckets, so +``info.to_dict() != info.to_dict()`` holds for every frame. The export negates +its ``__eq__``. For every frame of the option-heavy captures in :file:`examples/captures/`: diff --git a/tests/protocols/test_to_dict_payload_roundtrip_1447_unit.py b/tests/protocols/test_to_dict_payload_roundtrip_1447_unit.py index 54c2f88151..0be8ef6e6f 100644 --- a/tests/protocols/test_to_dict_payload_roundtrip_1447_unit.py +++ b/tests/protocols/test_to_dict_payload_roundtrip_1447_unit.py @@ -1,9 +1,10 @@ # -*- coding: utf-8 -*- """``from_data(info.to_dict())`` rebuilds the payload, byte for byte. -GitHub issue #1447: :meth:`Info.to_dict ` -leaves out ``__next_type__`` and ``__next_name__``, which -:meth:`~pcapkit.protocols.protocol.ProtocolBase._make_payload` dispatched on, so +This guards against GitHub issue #1447, where +:meth:`Info.to_dict ` left out +``__next_type__`` and ``__next_name__``, which +:meth:`~pcapkit.protocols.protocol.ProtocolBase._make_payload` dispatches on, so a layer rebuilt from its dict lost its payload: an Ethernet frame came back as its 14-octet header. IPv6 lost its extension headers the same way, with ``__exthdr__``, and HTTP raised ``ProtocolError('invalid HTTP data: Raw')``. diff --git a/tests/protocols/test_trial_parse_warnings_1580_unit.py b/tests/protocols/test_trial_parse_warnings_1580_unit.py index 4b95a67e7c..52f138a6e0 100644 --- a/tests/protocols/test_trial_parse_warnings_1580_unit.py +++ b/tests/protocols/test_trial_parse_warnings_1580_unit.py @@ -3,15 +3,15 @@ :meth:`~pcapkit.protocols.protocol.ProtocolBase._parse_next_layer` parses a next layer on trial and keeps it as :class:`~pcapkit.protocols.misc.raw.Raw` when -fewer octets were captured than its header needs (#1170). The trial's warnings -were reported anyway, for a layer the result does not have. A PCAP-NG Simple -Packet Block cut to 10 octets by its interface's snaplen warned ``packet length -< 0: -2`` and ``-4`` from the Ethernet header it then dropped, on parse, on both -``from_data`` rebuilds and on ``make``. Nothing in it is PCAP-NG's: an Enhanced -Packet Block, an obsolete Packet Block and a PCAP record cut the same way warned -the same, in either byte order. - -A next layer captured shorter than its length hint is now parsed under +fewer octets were captured than its header needs (#1170). Without the fix, the +trial's warnings are reported anyway, for a layer the result does not have: a +PCAP-NG Simple Packet Block cut to 10 octets by its interface's snaplen warns +``packet length < 0: -2`` and ``-4`` from the Ethernet header it then drops, on +parse, on both ``from_data`` rebuilds and on ``make``. Nothing in it is +PCAP-NG's: an Enhanced Packet Block, an obsolete Packet Block and a PCAP record +cut the same way warn the same, in either byte order. + +A next layer captured shorter than its length hint is parsed under :class:`~pcapkit.utilities.warnings.hold_warnings`, and its warnings are dropped with the layer. A layer that is kept reports them from the caller's line when its trial ends, inside the same guard -- so under an ``error`` filter every chain is @@ -56,8 +56,8 @@ #: An IPv4 header of total length 30, carrying TCP. IPV4_TCP = bytes.fromhex('4500001e00000000400600007f0000017f000001') -#: Messages for a 10-octet TCP header kept in the chain, measured unchanged against -#: the tree before the fix: the data offset is cut off, so its length reads as 0. +#: Messages for a 10-octet TCP header kept in the chain, measured the same with +#: and without the fix: the data offset is cut off, so its length reads as 0. TCP_CUT_MESSAGES = ['packet length < 0: -2', 'packet length < 0: -3', 'packet length < 0: -4', 'packet length < 0: -6', 'packet length < 0: -8', 'packet length < 0: -10'] @@ -78,7 +78,7 @@ class _HoldNothing: - """A stand-in for :class:`hold_warnings` that holds nothing, as before #1580.""" + """A stand-in for :class:`hold_warnings` that holds nothing, as without the #1580 fix.""" discard = False