From 54e6da96629b9cfaaad7b9c4bff35c44801674bc Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 22 Sep 2026 00:10:31 -0400 Subject: [PATCH] test(tcp): retarget the last assertion pinning the old padding side (#604) * `TCPUDPUnitTests.test_a_truncated_option_still_parses_its_declared_length` expected a truncated TCP option's `data` as the synthesised zero octets followed by the real ones, which is what `rjust()` produced. #621 made the padding `ljust()` everywhere but could not retarget this file, because #612 owned it at the time; it has been red on `main` since #621 merged. * The real octets now come first for both parametrised widths, and the docstring above the assertion says tail-padding rather than left-padding. * Test-only: no library code changes. The sibling case in `tests/protocols/internet/test_ipv4_unit.py` was already retargeted in #621. Measured against `main` at 2221c2d8f: two subtest failures before, none after. `tests/protocols/transport/` and `tests/corekit/test_fields_field.py` together give 177 passed, 232 subtests passed. --- CHANGELOG.md | 1 + docs/source/changelog/1.5.0.rst | 13 +++++++++++++ tests/protocols/transport/test_tcp_udp_unit.py | 8 +++++--- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd1fc8786..5ecc9da2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ This is the resolution of #548, which reported `TransType.L2TP` (115) as registe - **Fixed** -- documentation. `mptcp_dss_ack_selector`'s note said a corrected field-width lambda "would not have worked" and that fixing it belonged to `pcapkit.corekit.fields.numbers`, which is exactly where #598 then fixed it; the same paragraph sat in `test_tcp_mptcp_length_arithmetic_unit.py`'s module docstring, whose other stale claim was that MP_JOIN "cannot be built through the public `TCP()` constructor at all", true only until #587. A callable-length `NumberField` packs and unpacks both DSS widths now, and wire *absence* was never the obstacle either: `MPTCPDSS.ssn`, `dl_len` and `checksum` have always been `ConditionalField` on the sibling `M` flag, so the class already relied on that wrapper to keep a field off the wire. The `SwitchField` form is kept for the narrower reason the note now gives -- `ConditionalField`'s `length` forwards to the wrapped field without consulting the condition, so it is safe here only because `Schema.pack` and `Schema.unpack` special-case that wrapper by name, whereas a `SwitchField` always resolves to a concrete field. Replacing it would be a behaviour change and is not made (#603). - **Changed** -- the README is a landing page now, and Markdown rather than reStructuredText. `README.rst` (424 lines) became `README.md` (103), keeping what a reader arriving from PyPI or a search result actually needs -- what the library is, why it exists rather than Scapy or DPKT, how to install it, a worked example, and where the documentation lives -- and dropping the technical detail the documentation already carried. **Module Structure**, **Engine Comparison**, **Engine support by Python version**, **Test Environment**, **Test Results** and **Installation Notes** were each already duplicated in `docs/source/index.rst`, in a fuller form, so they are linked rather than restated. Two blocks existed nowhere else and moved rather than going: **Testing** is now `docs/source/testing.rst`, registered in the index toctree, and the `pipenv` and `make setup` local development block joined the Installation section of `docs/source/index.rst`. Requested by the project owner, and a deliberate exception to the convention that documentation here is reStructuredText -- for the README only, since it is the one documentation file whose renderers are GitHub and PyPI rather than Sphinx. Accordingly `setup.py` reads `README.md` and declares its content type as `text/markdown`, and the `include README.md` line in `MANIFEST.in` is now the only thing that puts the README in a source distribution, because `global-include *.rst` no longer matches it -- which matters, since `setup.py` reads the file unguarded and an sdist without it cannot be installed. Verified with `twine check --strict` against a built sdist and wheel, both of which pass. The rename's own references moved with it, since a change that renames a file owns the references to it: `examples/benchmark/Dockerfile` copies `README.md` -- a literal `COPY` of the old name would have failed the layer outright and taken `make bench`, `make bench-quick` and `run.sh` with it -- and the benchmark harness prose that named the root README as the destination of its generated tables now names `docs/source/index.rst`, which is where those tables went. That covers the `Makefile` comment, `report.py`, `test_harness.py`, `run.sh` and the suite's own README, including the one place whose stated reason had inverted: the emitted markup is kept parseable by plain docutils, which is now a conservative choice rather than a hard requirement, because the page it lands on is rendered by Sphinx. `examples/benchmark/benchmark.py` still says `README.rst` and is left alone, because it means the benchmark suite's own README in the same directory, not the project's. - **Changed** -- `CODE_OF_CONDUCT.md` moves from Contributor Covenant 1.4 to Contributor Covenant 3.0, at the maintainer's request. The text is the canonical 3.0 Markdown fetched from https://www.contributor-covenant.org/version/3/0/code_of_conduct/code_of_conduct.md rather than a transcription, so the pledge, the encouraged and restricted behaviours and the scope are unaltered. Three things needed deciding rather than copying. 3.0 ships two `[NOTE` placeholders an adopter must fill: the reporting channel, which now names `jarryshaw@icloud.com` -- the same contact 1.4 carried and the one `SECURITY.md` already points at as its email fallback -- plus GitHub's report-abuse form for the case a single-maintainer project cannot otherwise cover, a report about the maintainer; and the enforcement section, whose placeholder is an instruction to the adopter and is removed. 3.0 then assigns enforcement throughout to plural "Community Moderators" (and once, inconsistently, to "Community Managers"), which this repository does not have, so all eight occurrences become the singular maintainer. The four-rung ladder -- Warning, Temporarily Limited Activities, Temporary Suspension, Permanent Ban -- is offered as a suggestion and is **kept**, because each rung maps onto a lever one person actually holds on GitHub: a private message, a locked thread, an interaction limit or block, a permanent block. Finally, 3.0 is licensed CC BY-SA 4.0 where 1.4's attribution paragraph carried no licence notice at all, so the attribution now names version 3.0, links the permanent `version/3/0/` URL, carries the CC BY-SA 4.0 notice and link, indicates that changes were made as BY requires, and says explicitly that the share-alike term covers this document only -- the code remains BSD-3-Clause and `LICENSE` is untouched. Rendering was checked against GitHub's own Markdown API rather than assumed: the ladder comes back as four list items each nesting three, which is what #613 had to repair in the 1.4 file when a stray list marker collapsed the whole document into one nested item (#624). +- **Fixed** -- the one assertion #604 left pinning the old padding side, which had been red on `mainline` since #621 merged. `TCPUDPUnitTests.test_a_truncated_option_still_parses_its_declared_length` expected a truncated TCP option's `data` as the synthesised zero octets *followed by* the real ones, which is what `rjust()` produced; #621 made the padding `ljust()` everywhere but could not retarget this file, since another change (#612) owned it at the time and editing it concurrently risked discarding work that has since landed. The real octets now come first for both parametrised widths, and the docstring above the assertion says tail-padding rather than left-padding. Test-only: no library code changes, and the sibling case in `tests/protocols/internet/test_ipv4_unit.py` was already retargeted in #621. Measured against `main` at `2221c2d8f`: two subtest failures before, none after (#604). Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) and `1.5.0b3` (2026-09-19), all published as prereleases and so resolved only by `pip install --pre`. `1.5.0b1` half-shipped: the tag, the GitHub release and the Conda deployments landed, but PyPI rejected the wheel because `twine check` found a Sphinx-only `:mod:` role in `README.rst`, which `pyproject.toml` declares as the dynamic long description. `1.5.0b2` is what reshipped it -- the release workflow is version-driven, so an existing version cannot republish -- and `1.5.0b3` followed the CI change that stops a TestPyPI outage from costing a release its wheels (#497, #498). diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index d827348f0..df3aec47d 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -1004,6 +1004,19 @@ pull requests between #326 and #509. own Markdown API rather than assumed: the ladder comes back as four list items each nesting three, which is what #613 had to repair in the 1.4 file when a stray list marker collapsed the whole document into one nested item (#624). +* **Fixed** -- the one assertion #604 left pinning the old padding side, which had + been red on ``mainline`` since #621 merged. + ``TCPUDPUnitTests.test_a_truncated_option_still_parses_its_declared_length`` + expected a truncated TCP option's ``data`` as the synthesised zero octets + *followed by* the real ones, which is what ``rjust()`` produced; #621 made the + padding ``ljust()`` everywhere but could not retarget this file, since another + change (#612) owned it at the time and editing it concurrently risked discarding + work that has since landed. The real octets now come first for both parametrised + widths, and the docstring above the assertion says tail-padding rather than + left-padding. Test-only: no library code changes, and + the sibling case in ``tests/protocols/internet/test_ipv4_unit.py`` was already + retargeted in #621. Measured against ``main`` at ``2221c2d8f``: two subtest + failures before, none after (#604). Preceded by ``1.5.0a1`` (2026-09-15), ``1.5.0b1`` and ``1.5.0b2`` (both 2026-09-18) and ``1.5.0b3`` (2026-09-19), all published as prereleases and so diff --git a/tests/protocols/transport/test_tcp_udp_unit.py b/tests/protocols/transport/test_tcp_udp_unit.py index ad5229d64..7034d5475 100644 --- a/tests/protocols/transport/test_tcp_udp_unit.py +++ b/tests/protocols/transport/test_tcp_udp_unit.py @@ -1371,9 +1371,11 @@ def test_a_truncated_option_still_parses_its_declared_length(self) -> None: ``data`` field (``BytesField(length=lambda pkt: pkt['length'] - 2)``, 10 octets here) for more than the 6 octets actually behind it. :meth:`FieldBase.unpack ` - left-pads the short read with zero octets rather than raising, so the + tail-pads the short read with zero octets rather than raising, so the option parses with its declared ``length`` intact and a ``data`` value - of four zero octets followed by the six real ones. That is reachable + of the six real octets followed by four zero ones -- the zeros go where + the octets that were never read would have been, which before #604 was + the other way round. That is reachable here because :meth:`~pcapkit.protocols.transport.tcp.TCP._read_tcp_options` sizes each parsed option by ``len(schema)`` -- what it actually consumed (8 octets) -- rather than by its self-reported ``length``, so @@ -1412,7 +1414,7 @@ def segment(data_offset: 'int', options: 'bytes') -> 'bytes': ) unassigned = next(opt for code, opt in proto.info.options.items(multi=True) if code == custom) - self.assertEqual(unassigned.data, b'\x00' * zeroes + trailing) + self.assertEqual(unassigned.data, trailing + b'\x00' * zeroes) self.assertEqual(bytes(proto.__header__), raw) def test_unregistered_option_kind_does_not_mutate_the_class_registry(self) -> None: