SEZ9 commented on PR #11559: URL: https://github.com/apache/seatunnel/pull/11559#issuecomment-6008681012
Thanks for the follow-up on `PR11559-F1` — this is the shape I was hoping the coverage would end up in. On the `8909a67fce2` changes: 1. **Same-package subclass negative case.** Using `KafkaSourceSplitState` is a good choice for the negative test: as you say, it defeats a package-prefix check, a class-name-prefix check and an `instanceof KafkaSourceSplit` guard, so only exact-name matching rejects it. Combined with the kept `java.util.HashMap` case and the exact-message assertions, this pins the allowlist semantics tightly enough that a future widening would fail loudly rather than silently reopening the deserialization surface. 2. **Package-private instead of reflection.** Agreed that this is the better trade-off. Reflective access to a private static method is brittle and tends to rot; a package-private method with an explicit "exposed for tests" Javadoc is the conventional pattern here. Good that the test Javadoc also warns against swapping the call back to the local plain-`ObjectInputStream` helper, since that was exactly the gap in the original round-trip tests. Also appreciated that the only production delta in `KafkaSourceReader.java` is the visibility of that one method and that `KafkaGateObjectInputStream.isAllowedClass` itself is untouched — that keeps this round easy to reason about. Remaining asks before I mark this finding resolved: - Please report back once the CI run on `8909a67fce2` finishes. I have no result to go on yet, so I'm not treating the tests as passing until you confirm the outcome here. - If CI is green, a one-line confirmation that both new negative tests (`HashMap` and `KafkaSourceSplitState`) actually executed and failed on the rejection path as intended (rather than being skipped or passing for an unrelated reason) would be enough to close `PR11559-F1`. Nothing else outstanding from my side on this item. <!-- streview-comment:1549 --> -- 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]
