Repository navigation
fix(protocols): four more maker guards name the rejected argument - #2407
Merged
Merged
Conversation
JarryShaw
added
bug
Owner
Author
|
Review: NEEDS CHANGES at Verified good: all four old messages were genuinely maker-side One thing to fix: the new test module's docstring states counts ("the four makers", "swept eighteen", etc.) — per the 2026-10-15 ruling, docstrings carry no counts of codebase items, and #2405's own test file (the one this follows) has none. Drop them. Non-blocking: the commit message's "1998 passed/51306 subtests" figure is stale (measured on a pre-#2405 tree); the real number is 2020/51388, "0 failed" is still accurate. |
JarryShaw
added
review: needs-changes
Contributor
|
Coverage: 91.72% (unit tier, Python 3.15,
Per-file detail: the |
Follow-up to #2405, which left these out of #2296's checklist. #2406. - hopopt.py / ipv6_opts.py: _make_opt_pad's Pad1-length warning, matching MH._make_opt_pad's text from #2405 (alias swapped). - httpv2.py: HTTP.make's unsupported-frame-type guard. - pcapng.py: PCAPNG.make's unsupported-block-type guard (the dispatcher, not _make_block_dsb, which #2405 already fixed). Added the missing Raises: docstring section on the two make() guards, matching #2405's MH.make/_make_block_dsb docstrings. Test: tests/protocols/test_maker_invalid_format_messages_2406_unit.py (new, 9 tests/18 subtests), plus a ReadersKeepInvalidFormat pin that _read_opt_pad on both protocols still says "invalid format" -- the exact twin of the maker guard changed here. Confirmed fail-without-fix: 12 failures against unpatched pcapkit, each a message-text mismatch. tests/protocols/ in full: 2020 passed/51388 subtests/0 failed. mypy and pylint findings on the 4 touched files are unchanged from main.
JarryShaw
force-pushed
the
fix-2406-four-more-maker-guards
branch
from
October 28, 2026 04:47
d0ebeb9 to
26f9bce
Compare
JarryShaw
added
review: running
Owner
Author
|
Review: GOOD TO GO at |
JarryShaw
added
review: good-to-go
JarryShaw
added a commit
that referenced
this pull request
Oct 28, 2026
…egistry and registration prose (#1103) #1103 whole-repo prose sweep over the maker-message wave (#2296, #2404, #2405, #2407) and the __proto_dest__ registry refactor (#2392, #2403). Docstrings corrected where the prose did not match the code: * ``Protocol`` -> ``ProtocolBase`` in the ``register``/``register_protocol`` cross-references (protocol.py, internet.py, link.py, transport.py, sctp.py, registry/protocols.py). * ``Raises:`` entries for the ``bool`` port guard (#2287) in TCP, UDP and SCTP ``make``, and for HTTP/1's request-without-``uri`` guard. * ``protochain`` documented on both sides of its narrowing to ``None`` (protocol.py, misc/null.py, #2317), and ``__init__`` names the ``__own_make_keywords__`` exception to its keyword check. * IP reassembly names the IPv6 ``M`` bit and which buffer an atomic fragment is isolated from (#2270); ``_check_code`` names ``register_protocol_code`` among its callers (#2186). * HIP's shared lifetime helper says where the octet appears instead of counting the makers that carry it, and ``_make_opt_pad`` warns and makes a ``PadN`` rather than raising as its reader does. Before/after narration replaced with present tense, the pattern #1103's round 6 banned: the ``__proto_dest__`` prose in the registration and vendor tests, and the #2296 maker-message module docstring. AST guard (``ast.dump`` via util/vendor_code_changed.py's ``code_of``, with docstrings and bare string literals stripped) confirms all 16 touched files are prose-only against e48062c. 135 tests and 334 subtests pass across the four touched test modules, tests/test_docstring_contract.py, and the convention and issue/PR-role guards in tests/project/.
4 tasks done
JarryShaw
added a commit
that referenced
this pull request
Oct 28, 2026
…egistry and registration prose (#1103) #1103 whole-repo prose sweep over the maker-message wave (#2296, #2404, #2405, #2407) and the __proto_dest__ registry refactor (#2392, #2403). Docstrings corrected where the prose did not match the code: * ``Protocol`` -> ``ProtocolBase`` in the ``register``/``register_protocol`` cross-references (protocol.py, internet.py, link.py, transport.py, sctp.py, registry/protocols.py). * ``Raises:`` entries for the ``bool`` port guard (#2287) in TCP, UDP and SCTP ``make``, and for HTTP/1's request-without-``uri`` guard. * ``protochain`` documented on both sides of its narrowing to ``None`` (protocol.py, misc/null.py, #2317), and ``__init__`` names the ``__own_make_keywords__`` exception to its keyword check. * IP reassembly names the IPv6 ``M`` bit and which buffer an atomic fragment is isolated from (#2270); ``_check_code`` names ``register_protocol_code`` among its callers (#2186). * HIP's shared lifetime helper says where the octet appears instead of counting the makers that carry it, and ``_make_opt_pad`` warns and makes a ``PadN`` rather than raising as its reader does. Before/after narration replaced with present tense, the pattern #1103's round 6 banned: the ``__proto_dest__`` prose in the registration and vendor tests, the ``register_protocol_code`` key-check narration, and the #2296 and #2406 maker-message module docstrings. AST guard (``ast.dump`` via util/vendor_code_changed.py's ``code_of``, with docstrings and bare string literals stripped) confirms all 16 touched files are prose-only against e48062c. 130 tests and 334 subtests pass across the four touched test modules, tests/test_docstring_contract.py, and tests/project/test_conventions_doc_claims.py, tests/project/test_docs_issue_pr_roles_1103_unit.py and tests/project/test_issue_refs_in_comments.py.
JarryShaw
added a commit
that referenced
this pull request
Oct 28, 2026
…egistry and registration prose (#1103) #1103 whole-repo prose sweep over the maker-message wave (#2296, #2404, #2405, #2407) and the __proto_dest__ registry refactor (#2392, #2403). Docstrings corrected where the prose did not match the code: * ``Protocol`` -> ``ProtocolBase`` in the ``register``/``register_protocol`` cross-references (protocol.py, internet.py, link.py, transport.py, sctp.py, registry/protocols.py). * ``Raises:`` entries for the ``bool`` port guard (#2287) in TCP, UDP and SCTP ``make``, and for HTTP/1's request-without-``uri`` guard. * ``protochain`` documented on both sides of its narrowing to ``None`` (protocol.py, misc/null.py, #2317), and ``__init__`` names the ``__own_make_keywords__`` exception to its keyword check. * IP reassembly names the IPv6 ``M`` bit and which buffer an atomic fragment is isolated from (#2270); ``_check_code`` names ``register_protocol_code`` among its callers (#2186). * HIP's shared lifetime helper says where the octet appears instead of counting the makers that carry it, and ``_make_opt_pad`` warns and makes a ``PadN`` rather than raising as its reader does. Before/after narration replaced with present tense, the pattern #1103's round 6 banned: * the ``__proto_dest__`` prose in the registration and vendor tests, and the ``register_protocol_code`` key-check narration; * the #2296 and #2406 maker-message module docstrings -- readers are exempt by design rather than "deliberately untouched", and the HTTP ``frame=None`` case is outside the guard rather than one the guard "has not widened to"; * the ``_NameLease`` rationale and the ``register_protocol`` name-collision note in registry/protocols.py, which described a fix and a redesign in the past tense; * TCP's ``_make_mptcp_unknown`` length comment, which contrasted the current expression with "the former" one. AST guard (``ast.dump`` via util/vendor_code_changed.py's ``code_of``, with docstrings and bare string literals stripped) confirms all 16 touched files are prose-only against e48062c. 129 tests pass (1 pre-existing skip) with 334 subtests across the four touched test modules, tests/test_docstring_contract.py, tests/project/test_conventions_doc_claims.py, tests/project/test_docs_issue_pr_roles_1103_unit.py and tests/project/test_issue_refs_in_comments.py.
JarryShaw
added a commit
that referenced
this pull request
Oct 28, 2026
Problem: of the 60 pull requests merged in the #2312-#2407 range, only #2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and #2398 and #2403 only reworded lines that were already there. Every other user-visible change in the range -- wire-semantics fixes, maker guards, reassembly behaviour, public-API and error-message changes -- went unrecorded, so the entry no longer describes the release. Fix: one entry per user-visible pull request, in each module section's existing style and ordered by the issue it closes. Two pairs share a bullet, since each is the direct follow-up of the other: #2393 with #2401, and #2405 with #2407. The diff on the entry file is purely additive. Internal-only work is left out deliberately: dependency and action bumps (#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379, #2380, #2402), the unreachable-branch removal (#2378), and the #: comment marker the HIP Group and Packet crawlers now write (#2381). That last one renders nothing either way: the marker lands inside the generated _missing_ body, conf.py's exclude-members drops _missing_ from autodoc, and a comment in a method body is not an attribute comment to begin with. #2392 and #2403 are recorded together, under Added rather than Fixed, for the declarations they introduce: __proto_key__ on the seven classes owning a __proto__ registry (#2392), __proto_dest__ on the four enum registries and Vendor.PROTO_DEST to generate it (#2403). Both PRs added the autodoc entries, so both are rendered public attributes. #2392's rejection-message change is deliberately not recorded as a fix: the bare "code must be ..." text it prefixed came from _check_code, which #2193 introduced and which is in no release tag -- absent at v1.5.0b14 -- so no released version ever emitted it. #2403 is no longer skipped as internal because it reworded an existing bullet to name __proto_dest__ while nothing credited the attribute's introduction. Four claims are corrected against the source rather than carried over: FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not 1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback read cmmd.value, so a .value-carrying object was accepted unchecked rather than raising; #2399 left the dpkt adapter and the adapters' ModuleNotFound raises alone, so it claims no universal prefix; and #2397's old tail did name the version it got, so the entry quotes it and gains the :issue:`2302` it was missing. #2370's "a Section Header Block ... still fails" sentence is dropped: #2369 settles the byte order first, and an SHB built from raw octets with ctx=None now rebuilds byte-exact in both byte orders. CHANGELOG.md is regenerated with util/changelog_md.py. Test: tests/project/ in full, 0 failures, including test_changelog_md.py's in-step and supported-subset checks; util/changelog_md.py --check reports the two files in step; the entry parses under docutils with no structural warning.
JarryShaw
added a commit
that referenced
this pull request
Oct 28, 2026
…egistry and registration prose (#1103) #1103 whole-repo prose sweep over the maker-message wave (#2296, #2404, #2405, #2407) and the __proto_dest__ registry refactor (#2392, #2403). Docstrings corrected where the prose did not match the code: * ``Protocol`` -> ``ProtocolBase`` in the ``register``/``register_protocol`` cross-references (protocol.py, internet.py, link.py, transport.py, sctp.py, registry/protocols.py). * ``Raises:`` entries for the ``bool`` port guard (#2287) in TCP, UDP and SCTP ``make``, and for HTTP/1's request-without-``uri`` guard. * ``protochain`` documented on both sides of its narrowing to ``None`` (protocol.py, misc/null.py, #2317), and ``__init__`` names the ``__own_make_keywords__`` exception to its keyword check. * IP reassembly names the IPv6 ``M`` bit and which buffer an atomic fragment is isolated from (#2270); ``_check_code`` names ``register_protocol_code`` among its callers (#2186). * HIP's shared lifetime helper says where the octet appears instead of counting the makers that carry it, and ``_make_opt_pad`` warns and makes a ``PadN`` rather than raising as its reader does. Before/after narration replaced with present tense, the pattern #1103's round 6 banned: * the ``__proto_dest__`` prose in the registration and vendor tests, and the ``register_protocol_code`` key-check narration; * the #2296 and #2406 maker-message module docstrings -- readers are exempt by design rather than "deliberately untouched", and the HTTP ``frame=None`` case is outside the guard rather than one the guard "has not widened to"; * the ``_NameLease`` rationale and the ``register_protocol`` name-collision note in registry/protocols.py, which described a fix and a redesign in the past tense; * TCP's ``_make_mptcp_unknown`` length comment, which contrasted the current expression with "the former" one. AST guard (``ast.dump`` via util/vendor_code_changed.py's ``code_of``, with docstrings and bare string literals stripped) confirms all 16 touched files are prose-only against e48062c. 129 tests pass (1 pre-existing skip) with 334 subtests across the four touched test modules, tests/test_docstring_contract.py, tests/project/test_conventions_doc_claims.py, tests/project/test_docs_issue_pr_roles_1103_unit.py and tests/project/test_issue_refs_in_comments.py.
JarryShaw
added a commit
that referenced
this pull request
Oct 28, 2026
Problem: of the 60 pull requests merged in the #2312-#2407 range, only #2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and #2398 and #2403 only reworded lines that were already there. Every other user-visible change in the range -- wire-semantics fixes, maker guards, reassembly behaviour, public-API and error-message changes -- went unrecorded, so the entry no longer describes the release. Fix: one entry per user-visible pull request, in each module section's existing style and ordered by the issue it closes. Three pairs share a bullet, since each is the direct follow-up or counterpart of the other: #2393 with #2401, #2405 with #2407, and #2392 with #2403. The diff on the entry file is purely additive. Internal-only work is left out deliberately: dependency and action bumps (#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379, #2380, #2402), the unreachable-branch removal (#2378), and the #: comment marker the HIP Group and Packet crawlers now write (#2381). That last one renders nothing either way: the marker lands inside the generated _missing_ body, conf.py's exclude-members drops _missing_ from autodoc, and a comment in a method body is not an attribute comment to begin with. #2392 and #2403 are recorded together, under Added rather than Fixed, for the declarations they introduce: __proto_key__ on each class owning a __proto__ registry (#2392), __proto_dest__ on the enum registries and Vendor.PROTO_DEST to generate it (#2403). The bullet names those classes and registries rather than counting them, per the no-counted-numbers rule for the unreleased entry. Both PRs added the autodoc entries, so both are rendered public attributes. #2392's rejection-message change is deliberately not recorded as a fix: the bare "code must be ..." text it prefixed came from _check_code, which #2193 introduced and which is in no release tag -- absent at v1.5.0b14 -- so no released version ever emitted it. #2403 is no longer skipped as internal because it reworded an existing bullet to name __proto_dest__ while nothing credited the attribute's introduction. Four claims are corrected against the source rather than carried over: FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not 1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback read cmmd.value, so a .value-carrying object was accepted unchecked rather than raising; #2399 left the dpkt adapter and the adapters' ModuleNotFound raises alone, so it claims no universal prefix; and #2397's old tail did name the version it got, so the entry quotes it and gains the :issue:`2302` it was missing. #2370's "a Section Header Block ... still fails" sentence is dropped: #2369 settles the byte order first, and an SHB built from raw octets with ctx=None now rebuilds byte-exact in both byte orders. CHANGELOG.md is regenerated with util/changelog_md.py. Test: tests/project/ in full, 0 failures, including test_changelog_md.py's in-step and supported-subset checks; util/changelog_md.py --check reports the two files in step; the entry parses under docutils with no structural warning.
JarryShaw
added a commit
that referenced
this pull request
Oct 28, 2026
#1103 whole-repo prose sweep over the maker-message wave (#2296, #2404, #2405, #2407) and the __proto_dest__ registry refactor (#2392, #2403). Docstrings corrected where the prose did not match the code: * ``Protocol`` -> ``ProtocolBase`` in the ``register``/``register_protocol`` cross-references (protocol.py, internet.py, link.py, transport.py, sctp.py, registry/protocols.py). * ``Raises:`` entries for the ``bool`` port guard (#2287) in TCP, UDP and SCTP ``make``, and for HTTP/1's request-without-``uri`` guard. * ``protochain`` documented on both sides of its narrowing to ``None`` (protocol.py, misc/null.py, #2317), and ``__init__`` names the ``__own_make_keywords__`` exception to its keyword check. * IP reassembly names the IPv6 ``M`` bit and which buffer an atomic fragment is isolated from (#2270); ``_check_code`` names ``register_protocol_code`` among its callers (#2186). * HIP's shared lifetime helper says where the octet appears instead of counting the makers that carry it, and ``_make_opt_pad`` warns and makes a ``PadN`` rather than raising as its reader does. Before/after narration replaced with present tense, the pattern #1103's round 6 banned: * the ``__proto_dest__`` prose in the registration and vendor tests, and the ``register_protocol_code`` key-check narration; * the #2296 and #2406 maker-message module docstrings -- readers are exempt by design rather than "deliberately untouched", and the HTTP ``frame=None`` case is outside the guard rather than one the guard "has not widened to"; * the ``_NameLease`` rationale and the ``register_protocol`` name-collision note in registry/protocols.py, which described a fix and a redesign in the past tense; * TCP's ``_make_mptcp_unknown`` length comment, which contrasted the current expression with "the former" one. AST guard (``ast.dump`` via util/vendor_code_changed.py's ``code_of``, with docstrings and bare string literals stripped) confirms all 16 touched files are prose-only against e48062c. 129 tests pass (1 pre-existing skip) with 334 subtests across the four touched test modules, tests/test_docstring_contract.py, tests/project/test_conventions_doc_claims.py, tests/project/test_docs_issue_pr_roles_1103_unit.py and tests/project/test_issue_refs_in_comments.py.
JarryShaw
added a commit
that referenced
this pull request
Oct 28, 2026
Problem: of the 60 pull requests merged in the #2312-#2407 range, only #2334, #2336, #2337, #2340 and #2348 wrote a 1.5.0 changelog entry, and #2398 and #2403 only reworded lines that were already there. Every other user-visible change in the range -- wire-semantics fixes, maker guards, reassembly behaviour, public-API and error-message changes -- went unrecorded, so the entry no longer describes the release. Fix: one entry per user-visible pull request, in each module section's existing style and ordered by the issue it closes. Three pairs share a bullet, since each is the direct follow-up or counterpart of the other: #2393 with #2401, #2405 with #2407, and #2392 with #2403. The diff on the entry file is purely additive. Internal-only work is left out deliberately: dependency and action bumps (#2387 to #2391), test-only and docs-prose passes (#2333, #2376, #2379, #2380, #2402), the unreachable-branch removal (#2378), and the #: comment marker the HIP Group and Packet crawlers now write (#2381). That last one renders nothing either way: the marker lands inside the generated _missing_ body, conf.py's exclude-members drops _missing_ from autodoc, and a comment in a method body is not an attribute comment to begin with. #2392 and #2403 are recorded together, under Added rather than Fixed, for the declarations they introduce: __proto_key__ on each class owning a __proto__ registry (#2392), __proto_dest__ on the enum registries and Vendor.PROTO_DEST to generate it (#2403). The bullet names those classes and registries rather than counting them, per the no-counted-numbers rule for the unreleased entry. Both PRs added the autodoc entries, so both are rendered public attributes. #2392's rejection-message change is deliberately not recorded as a fix: the bare "code must be ..." text it prefixed came from _check_code, which #2193 introduced and which is in no release tag -- absent at v1.5.0b14 -- so no released version ever emitted it. #2403 is no longer skipped as internal because it reworded an existing bullet to name __proto_dest__ while nothing credited the attribute's introduction. Four claims are corrected against the source rather than carried over: FTP_DATA shipped in pcapkit/protocols/application/ftp.py from 1.0.0, not 1.4.0 (class FTP_DATA(Raw) is there at v1.0.0); #2365's pre-fix fallback read cmmd.value, so a .value-carrying object was accepted unchecked rather than raising; #2399 left the dpkt adapter and the adapters' ModuleNotFound raises alone, so it claims no universal prefix; and #2397's old tail did name the version it got, so the entry quotes it and gains the :issue:`2302` it was missing. #2370's "a Section Header Block ... still fails" sentence is dropped: #2369 settles the byte order first, and an SHB built from raw octets with ctx=None now rebuilds byte-exact in both byte orders. CHANGELOG.md is regenerated with util/changelog_md.py. Test: tests/project/ in full, 0 failures, including test_changelog_md.py's in-step and supported-subset checks; util/changelog_md.py --check reports the two files in step; the entry parses under docutils with no structural warning.
JarryShaw
removed
the
review: good-to-go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
make testpasses, and a test case covers the changeWhat is the purpose of your pull request?
fix — bug fixDescription
Fixes #2406, the four sites #2405 left out of #2296's checklist:
HOPOPT._make_opt_pad/IPv6_Opts._make_opt_pad— the Pad1-length warning, matchingMH._make_opt_pad's text from fix(protocols): makers name the rejected argument and value in error messages #2405 with the alias swapped.HTTP.make— the unsupported-frame-type guard.PCAPNG.make(the dispatcher, not_make_block_dsb, which fix(protocols): makers name the rejected argument and value in error messages #2405 already fixed) — the unsupported-block-type guard.Added the missing
Raises:docstring section on the twomake()guards, matching #2405'sMH.make/_make_block_dsbdocstrings.Test:
tests/protocols/test_maker_invalid_format_messages_2406_unit.py(new, 9 tests/18 subtests), plus aReadersKeepInvalidFormatpin that_read_opt_padon both protocols still saysinvalid format— the exact reader-side twin of the maker guard changed here. Confirmed fail-without-fix: 12 failures against unpatchedpcapkit, each a message-text mismatch.tests/protocols/in full: 0 failures. mypy and pylint findings on the 4 touched files are unchanged frommain.