Repository navigation
fix(protocols): makers name the rejected argument in their error messages - #2404
Qiu-LingYan wants to merge 1 commit into
Conversation
|
Review: NEEDS CHANGES at Verified good: every change is maker-side only (AST scan, reader-side Three things need fixing:
Non-blocking: the Pad1 warning and SCTP "must not be empty" message could use a |
|
Closing in favor of #2405, which fixes the same issue (#2296) and is already at good-to-go — see the findings above for why this one needed another pass. Thanks for picking up #2296, @Qiu-LingYan — genuinely appreciate the contribution, the timing just didn't line up. Happy to look at future PRs. |
…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/.
…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.
…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.
…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.
#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.
Fixes #2296.
Per the issue,
_make_*builders that reject a bad argument reported the reader's parse-error text (invalid format), which made it impossible to tell which argument was at fault. This PR aligns every maker-side guard with the convention already used elsewhere (e.g.MH._make_opt_mnp'sinvalid mobile network prefix: {prefix!r}):Reader-side messages (which legitimately keep
invalid format) are untouched.Changed sites
pcapkit/protocols/transport/sctp.pyinvalid flags: must be one octet, not {flags!r}(wasinvalid format)_make_chunk_data:invalid data: must not be empty(wasinvalid format)_make_param_hostname:invalid name: must be NUL-terminated, not {name!r}_make_cause_invalid_stream:invalid reserved: must be two octets, not {reserved!r}pcapkit/protocols/internet/mh.pymake()unsupported-type guard:invalid data: unsupported type {data.__class__.__name__}(also avoids thetypeparameter shadowing the builtin)_make_opt_padPad1 fallback warning:invalid length for Pad1: {length}; falling back to PadN_make_opt_bad:invalid data: length must be a multiple of 8, not {len(data)!r}_make_opt_auth:invalid data: length must make the option a multiple of 4, not {len(data)!r}_make_opt_badf:invalid data: must not be empty, not {data!r}pcapkit/protocols/transport/tcp.py_make_mptcp_capable: the prefix-ordering guard now names the first out-of-order field and the one it requires:invalid fields: {name!r} given without {prev!r}pcapkit/protocols/internet/hip.py_make_locator(via_make_param_locator_set):invalid locator: must be an IPv6Address or LocatorData, not {value!r}pcapkit/protocols/misc/pcapng.py_make_block_dsb:invalid secrets_data: unsupported type {type(secrets_data).__name__!r}Tests
New
tests/protocols/misc/test_maker_invalid_argument_message_2296_unit.py— 12 tests asserting the full message strings (no substring matching), exercising each guard through its public path (e.g.SCTP._make_chunk_*,proto._make_mptcp_capable(skey=None, rkey=...), the HIP parameter-set path with a bad inner locator value, andPCAPNG._make_block_dsbwith an unsupported type).tests/protocols/transport+internet+miscsuite: 1142 passed + 21433 subtests, 0 failures (7 sample-captureFileNotFoundErrors in one run were environmental — captures are generated, not committed — and those 7 tests pass afterpython examples/generators/make_samples.py)ruff checkon the touched production files: unchanged frommainbaseline