SEZ9 commented on PR #11559: URL: https://github.com/apache/seatunnel/pull/11559#issuecomment-6051268429
Thanks for the follow-up on `PR11559-F1`. The approach in `8909a67fce2` looks right to me: - `productionDeserializeSplitShouldRejectSamePackageSubclassOutsideAllowlist` is the negative case this finding needed. Using `KafkaSourceSplitState` is a good choice: it defeats a package-prefix check, a class-name-prefix check and an `instanceof KafkaSourceSplit` guard, so only exact-name matching in `KafkaGateObjectInputStream.isAllowedClass` makes it pass. Keeping the `java.util.HashMap` case alongside it and asserting the exact rejection message (not a substring) is also what I wanted. - Making `KafkaSourceReader.deserializeSplit` package-private for tests and dropping the reflective helper is fine. Good that the only production delta is the visibility of that one method and the allowlist logic itself is untouched. The Javadoc warning not to swap in the test's local plain-`ObjectInputStream` helper is useful, since that was exactly the gap in the earlier round-trip tests. Remaining asks before I can close `PR11559-F1`: 1. A CI run on `8909a67fce2` where the connector-kafka unit tests actually execute. As you reported, run 37351604289 never got there (`Dead links` failed on HTTP 429 from deepwiki and the remaining jobs were cancelled/skipped), so neither new test has run yet. Once the dead-links fix you referenced lands, please re-run this head and post the per-test result for `productionDeserializeSplitShouldRejectSamePackageSubclassOutsideAllowlist` and the `HashMap` case here. 2. If you are able to run `KafkaSourceReaderGateTest` locally in the meantime, a quick note that both rejection tests pass (and that the positive round-trip through the production `deserializeSplit` still passes) would let me verify the behaviour ahead of CI. Not a blocker, just a shortcut. Nothing else outstanding on this finding from my side; it stays open only pending the executed test results. <!-- streview-comment:1595 --> -- 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]
