Fix integer overflows when decoding large payloads #192
Conversation
|
What's your use case? The block format is not a good match for such large payloads, since it requires everything to be in memory (compressed and decompressed data). The frame format is a better choice for large payloads. |
We use LZ4 blocks as a low-level primitive within a larger protocol that already manages its own framing. Under normal operation, these blocks are around ~1 MiB in size, but in certain rare edge cases outside our control, they can grow significantly larger. We’re fine with the extra memory cost in those situations, as long as the rest of the system remains stable. |
|
Thanks for the PR! About the |
|
Thanks for merging this 🙏 |
|
Released with |
|
Thank you! |
This is a follow up to #11525. It fixes the remaining half of #11516. We don't want to merge it as is yet. Let's see if we can get the fix merged upstream first, so we don't have to depend on yet another fork: * PSeitz/lz4_flex#192 --- * Fixes #11516 --- Checks: * [x] `rerun` with `--nb_scans 3000` now works * [x] `rerun rrd stats` with `--nb_scans 3000` now works
This is a follow up to #11525. It fixes the remaining half of #11516. We don't want to merge it as is yet. Let's see if we can get the fix merged upstream first, so we don't have to depend on yet another fork: * PSeitz/lz4_flex#192 --- * Fixes #11516 --- Checks: * [x] `rerun` with `--nb_scans 3000` now works * [x] `rerun rrd stats` with `--nb_scans 3000` now works
This fixes an issue on 64bit platforms where encoding large payloads works as expected but decoding them subsequently fails.
The root cause is that
write_integerexpects host-sized integers (i.e.usize) whileread_integeron the other hand always assumes 32bit integers.This PR:
usizeeverywhere. We could also go all the way and expectu64everywhere instead, but I think keeping things host-sized by default is fine (yes, that means a 32bit host might still fail when decoding large enough data from a 64bit client).write_integer. It is incompatible with values >32bits, and I don't think it is worth its weight in complexity in any case given how optimizer-friendly the safe version already is. Happy to revert that part if needed.