Jens-G commented on PR #3414: URL: https://github.com/apache/thrift/pull/3414#issuecomment-4392729218
### Code review Found 2 issues: 1. `total_field_count` increments for unknown fields, reintroducing the multi-field false error that THRIFT-5926 fixed — a union with 1 known + 1 unknown field sets `total_field_count = 2` and returns `InvalidData "received multiple fields"` instead of succeeding https://github.com/apache/thrift/blob/6e56bd55e11357608f01e51ba52e75f2a953f406/compiler/cpp/src/thrift/generate/t_rs_generator.cc#L1859-L1879 2. Wire stream corruption when a union field's value type is itself a union with an unknown variant — the inner `read_from_in_protocol` returns `Err(UnknownUnionVariant)` via `?` before the outer union consumes its Stop byte; `suppress_unknown` catches the error and continues, but the orphaned Stop byte causes the enclosing struct's next `read_field_begin` to see `0x00` and terminate the struct early, silently dropping all subsequent fields https://github.com/apache/thrift/blob/6e56bd55e11357608f01e51ba52e75f2a953f406/compiler/cpp/src/thrift/generate/t_rs_generator.cc#L1717-L1727 Regression tests for both issues have been added as a separate commit: 24af312f5. 🤖 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]
