DanielLeens commented on PR #11512: URL: https://github.com/apache/seatunnel/pull/11512#issuecomment-6143686171
Thanks, @SEZ9. My previous comment was not cut off on my side: it ended with the sentence "Happy to defer to your final pass once you have re-checked the above." Since the tail may not have rendered for you, here is the remaining item in full. **Outstanding item (Medium, non-blocking, carryover from my earlier reviews; it is not part of your F1-F8 list):** the wire codec bounds entry and field counts but not the length of individual strings or the total frame size. - Location: `seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/task/operation/CdcProgressReportSerializer.java`, the count caps at lines 46-47 (`MAX_BATCH_ENTRIES = 100_000`, `MAX_POSITION_FIELDS = 1_024`), and the unbounded `writeString`/`readString` calls for position keys and values (around lines 88-96 and 173-191). - Why it matters: the progress SPI is connector-implementable, so a connector can place an arbitrarily long string into a position field. With 1,024 fields per position and up to 100,000 batch entries, the count caps alone do not bound memory on the coordinator side. This only concerns optional diagnostics, so it does not affect source or checkpoint state. - Suggested fix: add a per-string cap (and, if convenient, a total payload cap) with the same fail-with-`IOException` behavior the count caps already use. I am fine with this as a fast-follow PR. - The code in this file is unchanged between the head I reviewed earlier and `9cf8e059b8`, so my assessment is unchanged. Nothing else is open from my side. For F1-F8, the pointers in my previous comment are against the same head `9cf8e059b8`, so you can re-check them there at your convenience. Thanks for the patience! -- 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]
