SEZ9 commented on PR #11557: URL: https://github.com/apache/seatunnel/pull/11557#issuecomment-6183897993
Following up on F1 (unsafe deserialization in `ManagedSourceRegisterOperation`) against the current head `5dc655e6a91`. What I can accept so far: - The two test names are confirmed: `testReaderLocationPayloadRoundTrips` and `testMismatchedReaderLocationTypeIsRejected` are the only test methods in `ManagedSourceRegisterOperationSerializationTest`, so the "2 tests, 0 failures" result from the JDK 8 Ubuntu job (run `37762848399`, job `113270515885`) on `b00c3e0cdad` covers both the mismatched same-factory/different-class rejection path and the valid round trip. - The stated delta from `b00c3e0cdad` to `5dc655e6a91` does not touch `ManagedSourceRegisterOperation` or its serialization test, so I agree the F1 code under review is the same. As you noted, that is a code-level argument rather than executed evidence for the new SHA. Remaining asks before I close F1: 1. Once Build run `38352742729` finishes on `5dc655e6a91`, please post the executed result for `ManagedSourceRegisterOperationSerializationTest` (test count and pass/fail), ideally with a link to the job, so the finding is closed on the head that will actually merge. 2. Please link the legacy follow-up issue you filed (covering `AssignSplitOperation#readInternal` and `TaskOperation#readInternal`) in the PR description, so the scope boundary of this fix is visible without reading the comment history. Once the executed result on `5dc655e6a91` is in, I'll mark F1 resolved. <!-- streview-comment:1775 --> -- 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]
