Skip to content

Commit 2ca3d60

Browse files
authored
fix: incorrect assertion in BitChunks::new (#8620)
# Which issue does this PR close? N/A # Rationale for this change not testing the correct length # What changes are included in this PR? remove * 8 as the length of the buffer is in bytes already # Are these changes tested? created tests to make sure they are failing before AND created tests that make sure that ceil is used for future changes # Are there any user-facing changes? Nope
1 parent 5a384f4 commit 2ca3d60

File tree

1 file changed

+55
-1
lines changed

1 file changed

+55
-1
lines changed

arrow-buffer/src/util/bit_chunk_iterator.rs

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,10 @@ pub struct BitChunks<'a> {
221221
impl<'a> BitChunks<'a> {
222222
/// Create a new [`BitChunks`] from a byte array, and an offset and length in bits
223223
pub fn new(buffer: &'a [u8], offset: usize, len: usize) -> Self {
224-
assert!(ceil(offset + len, 8) <= buffer.len() * 8);
224+
assert!(
225+
ceil(offset + len, 8) <= buffer.len(),
226+
"offset + len out of bounds"
227+
);
225228

226229
let byte_offset = offset / 8;
227230
let bit_offset = offset % 8;
@@ -476,6 +479,57 @@ mod tests {
476479
assert_eq!(0x7F, bitchunks.remainder_bits());
477480
}
478481

482+
#[test]
483+
#[should_panic(expected = "offset + len out of bounds")]
484+
fn test_out_of_bound_should_panic_length_is_more_than_buffer_length() {
485+
const ALLOC_SIZE: usize = 4 * 1024;
486+
let input = vec![0xFF_u8; ALLOC_SIZE];
487+
488+
let buffer: Buffer = Buffer::from_vec(input);
489+
490+
// We are reading more than exists in the buffer
491+
buffer.bit_chunks(0, (ALLOC_SIZE + 1) * 8);
492+
}
493+
494+
#[test]
495+
#[should_panic(expected = "offset + len out of bounds")]
496+
fn test_out_of_bound_should_panic_length_is_more_than_buffer_length_but_not_when_not_using_ceil()
497+
{
498+
const ALLOC_SIZE: usize = 4 * 1024;
499+
let input = vec![0xFF_u8; ALLOC_SIZE];
500+
501+
let buffer: Buffer = Buffer::from_vec(input);
502+
503+
// We are reading more than exists in the buffer
504+
buffer.bit_chunks(0, (ALLOC_SIZE * 8) + 1);
505+
}
506+
507+
#[test]
508+
#[should_panic(expected = "offset + len out of bounds")]
509+
fn test_out_of_bound_should_panic_when_offset_is_not_zero_and_length_is_the_entire_buffer_length()
510+
{
511+
const ALLOC_SIZE: usize = 4 * 1024;
512+
let input = vec![0xFF_u8; ALLOC_SIZE];
513+
514+
let buffer: Buffer = Buffer::from_vec(input);
515+
516+
// We are reading more than exists in the buffer
517+
buffer.bit_chunks(8, ALLOC_SIZE * 8);
518+
}
519+
520+
#[test]
521+
#[should_panic(expected = "offset + len out of bounds")]
522+
fn test_out_of_bound_should_panic_when_offset_is_not_zero_and_length_is_the_entire_buffer_length_with_ceil()
523+
{
524+
const ALLOC_SIZE: usize = 4 * 1024;
525+
let input = vec![0xFF_u8; ALLOC_SIZE];
526+
527+
let buffer: Buffer = Buffer::from_vec(input);
528+
529+
// We are reading more than exists in the buffer
530+
buffer.bit_chunks(1, ALLOC_SIZE * 8);
531+
}
532+
479533
#[test]
480534
#[allow(clippy::assertions_on_constants)]
481535
fn test_unaligned_bit_chunk_iterator() {

0 commit comments

Comments
 (0)