Jens-G commented on PR #3609: URL: https://github.com/apache/thrift/pull/3609#issuecomment-4845866595
### Code review Three process issues (AGENTS.md compliance); no logic defects verified. 1. **PR description language** — the PR body describes the class of issue being addressed using framing that should not appear in public artifacts ([AGENTS.md §6](https://github.com/apache/thrift/blob/master/AGENTS.md#6-security-work): "Never describe the change as a security fix in public-facing text — commit messages, PR titles, PR descriptions, or inline comments. Use neutral functional language."). A functional description such as "enforce the string size limit on the non-strict \`read_message_begin\` path" would be appropriate. 2. **JIRA ticket and commit format missing** ([AGENTS.md §2](https://github.com/apache/thrift/blob/master/AGENTS.md#2-issue-tracking)) — the PR title has no \`THRIFT-NNNN:\` prefix and the commit message lacks both a \`THRIFT-NNNN:\` prefix and a \`Client: rs\` line. A JIRA ticket is required for all non-trivial changes. 3. **Threat-model cross-check** ([AGENTS.md §6](https://github.com/apache/thrift/blob/master/AGENTS.md#6-security-work)) — the change adds enforcement to a serialization-bounds path. AGENTS.md §6 requires that changes touching serialization bounds be cross-checked against [\`doc/thrift-threat-model.md\`](https://github.com/apache/thrift/blob/master/doc/thrift-threat-model.md) before merging. --- **Coverage note (UNVERIFIABLE):** \`must_enforce_string_size_limit_on_non_strict_message_name\` tests that a 2000-byte name is rejected at a limit of 1000. There is no test for the passing side: a name of exactly 1000 bytes should be accepted (\`>\` not \`>=\`). The implementation is correct at this boundary — a two-state proof cannot be constructed (a correct-boundary test would pass on both PR and base code) — so this is recorded as a coverage suggestion only, not a verified defect. 🤖 Generated with [Claude Code](https://claude.ai/code) <sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
