tisonkun commented on PR #224:
URL: 
https://github.com/apache/datasketches-rust/pull/224#issuecomment-5412742095

   One more issue with the accepted boundary: `(lg_max_map_size, 
lg_cur_map_size) = (30, 30)` still passes the new validation and immediately 
attempts a very large allocation.
   
   A minimal 8-byte empty header is:
   
   ```rust
   let bytes = [1, 1, 10, 30, 30, 5, 0, 0];
   FrequentItemsSketch::<i64>::deserialize(&bytes);
   ```
   
   This reaches `ReversePurgeItemHashMap::new(1usize << 30)`. On a 64-bit 
target with `i64` items, the three backing arrays require roughly 26 GiB in 
total (`Option<i64>` + `u64` + `u16` per slot), so the process may abort or be 
killed rather than return `Error::deserial`.
   
   I did not execute this exact case without a memory limit because it is 
intentionally OOM-inducing. Also, the test described as exercising the “exact 
upper bound” currently uses `lg_max = 10`, so it does not cover this path. 
Could you review whether accepting `lg_cur = 30` is really the intended 
resource limit?
   


-- 
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]

Reply via email to