xiangfu0 commented on PR #19285: URL: https://github.com/apache/pinot/pull/19285#issuecomment-5335951832
Two notes from review: 1. `SnappyCodecDefinition.decode` allocates a direct buffer from the untrusted header-declared decompressed size with only a `< 0` check — LZ4/GZIP/ZSTD all apply a sanity cap. Latent today (the path is package-private and unused in production), but the V7 reader PR later in this stack makes it reachable from segment data. I'll include the cap plus a standalone Snappy corrupt-input test in that PR unless you'd rather fix it here. 2. Cross-PR invariant for the rest of the stack: `ForwardIndexConfig` stores the structural normalization (`toDslString`), which can differ from the executor's `getCanonicalSpec()` (e.g. `ZSTANDARD` → `ZSTD(3)`). The V7 header write and the reload spec comparison must canonicalize via the executor on both sides — never persist or compare the config-stored string. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
