LuciferYang commented on PR #68148: URL: https://github.com/apache/doris/pull/68148#issuecomment-5748869991
@HappenLee the bot's re-review at db0b0e958b7 accepted the four original fixes and raised five new inline findings. They open a scope question I'd like your read on before I push more. All five are pre-existing hazards in the v1 `parse_min_max_value` decode path, not regressions from this change; the up-front width gate I added is what put them in view. Two are memory-safety reachable from a crafted external file: an oversized BYTE_ARRAY decimal statistic overruns the fixed native-decimal buffer in `StringToDecimal` (r4055906694), and an `INT64_MAX` timestamp under a fixed-offset session overflows the signed `epoch_seconds + offset_seconds` inside conversion before the unit check can reject it (r4055906698). The other three are wrong bounds at extreme boundaries: a BOOLEAN statistic with padding bits reinterpreted as C++ `bool` can publish `true` for a `0x02` byte (r4055906685), and INT32 DATE / INT64 TIMESTAMP values near civil year 0 decode to an inverted `min > max` range even though the raw bytes are ordered (r4055906690, r4055906696). Two ways to take it. Fix all five here, which brings v1 min/max decode up to the conservative validation v2 already does: a BOOLEAN bit-mask, a decimal length bound, a supported-range check before conversion, and a decoded-inversion backstop. Or keep this PR scoped to the timestamp rollback and INT96 fence plus the two memory-safety fixes, and track the rare year-0 decode-inversion cases as a separate v1-decode-hardening issue. I will fix the two memory-safety ones here regardless. I lean toward doing all five in one pass since they cluster and it closes the door on the decode path, but it widens this PR well past #68118, so I would rather have your read on where to draw the line. -- 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]
