This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 2ca3d609cc fix: incorrect assertion in `BitChunks::new` (#8620)
2ca3d609cc is described below
commit 2ca3d609cc99e0c153d1ab2e62af16a4b3f79a12
Author: Raz Luvaton <[email protected]>
AuthorDate: Fri Oct 17 19:18:21 2025 +0300
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
---
arrow-buffer/src/util/bit_chunk_iterator.rs | 56 ++++++++++++++++++++++++++++-
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/arrow-buffer/src/util/bit_chunk_iterator.rs
b/arrow-buffer/src/util/bit_chunk_iterator.rs
index ea8e8f472a..e11383f6f3 100644
--- a/arrow-buffer/src/util/bit_chunk_iterator.rs
+++ b/arrow-buffer/src/util/bit_chunk_iterator.rs
@@ -221,7 +221,10 @@ pub struct BitChunks<'a> {
impl<'a> BitChunks<'a> {
/// Create a new [`BitChunks`] from a byte array, and an offset and length
in bits
pub fn new(buffer: &'a [u8], offset: usize, len: usize) -> Self {
- assert!(ceil(offset + len, 8) <= buffer.len() * 8);
+ assert!(
+ ceil(offset + len, 8) <= buffer.len(),
+ "offset + len out of bounds"
+ );
let byte_offset = offset / 8;
let bit_offset = offset % 8;
@@ -476,6 +479,57 @@ mod tests {
assert_eq!(0x7F, bitchunks.remainder_bits());
}
+ #[test]
+ #[should_panic(expected = "offset + len out of bounds")]
+ fn test_out_of_bound_should_panic_length_is_more_than_buffer_length() {
+ const ALLOC_SIZE: usize = 4 * 1024;
+ let input = vec![0xFF_u8; ALLOC_SIZE];
+
+ let buffer: Buffer = Buffer::from_vec(input);
+
+ // We are reading more than exists in the buffer
+ buffer.bit_chunks(0, (ALLOC_SIZE + 1) * 8);
+ }
+
+ #[test]
+ #[should_panic(expected = "offset + len out of bounds")]
+ fn
test_out_of_bound_should_panic_length_is_more_than_buffer_length_but_not_when_not_using_ceil()
+ {
+ const ALLOC_SIZE: usize = 4 * 1024;
+ let input = vec![0xFF_u8; ALLOC_SIZE];
+
+ let buffer: Buffer = Buffer::from_vec(input);
+
+ // We are reading more than exists in the buffer
+ buffer.bit_chunks(0, (ALLOC_SIZE * 8) + 1);
+ }
+
+ #[test]
+ #[should_panic(expected = "offset + len out of bounds")]
+ fn
test_out_of_bound_should_panic_when_offset_is_not_zero_and_length_is_the_entire_buffer_length()
+ {
+ const ALLOC_SIZE: usize = 4 * 1024;
+ let input = vec![0xFF_u8; ALLOC_SIZE];
+
+ let buffer: Buffer = Buffer::from_vec(input);
+
+ // We are reading more than exists in the buffer
+ buffer.bit_chunks(8, ALLOC_SIZE * 8);
+ }
+
+ #[test]
+ #[should_panic(expected = "offset + len out of bounds")]
+ fn
test_out_of_bound_should_panic_when_offset_is_not_zero_and_length_is_the_entire_buffer_length_with_ceil()
+ {
+ const ALLOC_SIZE: usize = 4 * 1024;
+ let input = vec![0xFF_u8; ALLOC_SIZE];
+
+ let buffer: Buffer = Buffer::from_vec(input);
+
+ // We are reading more than exists in the buffer
+ buffer.bit_chunks(1, ALLOC_SIZE * 8);
+ }
+
#[test]
#[allow(clippy::assertions_on_constants)]
fn test_unaligned_bit_chunk_iterator() {
