SEZ9 commented on PR #11559: URL: https://github.com/apache/seatunnel/pull/11559#issuecomment-6393670258
Following up on the one open item from my earlier review: the unsafe Java deserialization pattern in `KafkaSourceReader`. As I read the thread at head `cc3de569cca3601912dddc5d6c0113e774e98a96`: commit `8909a67f` widens `KafkaSourceReader.deserializeSplit` to package-private so the real allowlist is tested directly, and the method is still routed through the exact-name `KafkaGateObjectInputStream`. That is the right shape, and I am happy with the direction. I am not closing the finding yet because the thread so far describes the hardening in detail only for `DynamicLookupStateObjectInputStream` (exact names, recursive array checks, proxy rejection), not for the Kafka gate stream. Remaining asks before I close it: 1. Confirm `KafkaGateObjectInputStream` applies the same three rules as the dynamic-lookup stream: exact class-name matching only (no prefix/package allow), recursive checks of array component types, and rejection of proxy classes. If any of these is missing, please add it. 2. Point me to the negative test that drives an unlisted class through `deserializeSplit` and asserts rejection, plus one that covers an array of an unlisted type. If `8909a67f` already includes these, the test method names are enough. 3. Confirm there is no other `ObjectInputStream` construction on the Kafka gate path that bypasses `KafkaGateObjectInputStream`. Once those are confirmed in the PR, I will mark the finding closed. Everything else from my previous review stands as summarized in the latest round. <!-- streview-comment:2027 --> -- 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]
