Skip to content

Commit 95c7537

Browse files
authored
Merge branch 'main' into split-selection-modules
2 parents b3efe2d + b8b59fb commit 95c7537

3 files changed

Lines changed: 65 additions & 9 deletions

File tree

arrow-avro/src/reader/block.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ impl BlockDecoder {
8080
while !buf.is_empty() {
8181
match self.state {
8282
BlockDecoderState::Count => {
83-
if let Some(c) = self.vlq_decoder.long(&mut buf) {
83+
if let Some(c) = self.vlq_decoder.long(&mut buf)? {
8484
self.in_progress.count = c.try_into().map_err(|_| {
8585
AvroError::ParseError(format!(
8686
"Block count cannot be negative, got {c}"
@@ -91,7 +91,7 @@ impl BlockDecoder {
9191
}
9292
}
9393
BlockDecoderState::Size => {
94-
if let Some(c) = self.vlq_decoder.long(&mut buf) {
94+
if let Some(c) = self.vlq_decoder.long(&mut buf)? {
9595
self.bytes_remaining = c.try_into().map_err(|_| {
9696
AvroError::ParseError(format!("Block size cannot be negative, got {c}"))
9797
})?;

arrow-avro/src/reader/header.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ impl HeaderDecoder {
253253
}
254254
}
255255
HeaderDecoderState::BlockCount => {
256-
if let Some(block_count) = self.vlq_decoder.long(&mut buf) {
256+
if let Some(block_count) = self.vlq_decoder.long(&mut buf)? {
257257
match block_count.try_into() {
258258
Ok(0) => {
259259
self.state = HeaderDecoderState::Sync;
@@ -271,7 +271,7 @@ impl HeaderDecoder {
271271
}
272272
}
273273
HeaderDecoderState::BlockLen => {
274-
if self.vlq_decoder.long(&mut buf).is_some() {
274+
if self.vlq_decoder.long(&mut buf)?.is_some() {
275275
self.state = HeaderDecoderState::KeyLen
276276
}
277277
}
@@ -301,13 +301,13 @@ impl HeaderDecoder {
301301
}
302302
}
303303
HeaderDecoderState::KeyLen => {
304-
if let Some(len) = self.vlq_decoder.long(&mut buf) {
304+
if let Some(len) = self.vlq_decoder.long(&mut buf)? {
305305
self.bytes_remaining = len as _;
306306
self.state = HeaderDecoderState::Key;
307307
}
308308
}
309309
HeaderDecoderState::ValueLen => {
310-
if let Some(len) = self.vlq_decoder.long(&mut buf) {
310+
if let Some(len) = self.vlq_decoder.long(&mut buf)? {
311311
self.bytes_remaining = len as _;
312312
self.state = HeaderDecoderState::Value;
313313
}

arrow-avro/src/reader/vlq.rs

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18+
use crate::errors::AvroError;
19+
1820
/// Decoder for zig-zag encoded variable length (VLW) integers
1921
///
2022
/// See also:
@@ -29,19 +31,29 @@ pub struct VLQDecoder {
2931

3032
impl VLQDecoder {
3133
/// Decode a signed long from `buf`
32-
pub fn long(&mut self, buf: &mut &[u8]) -> Option<i64> {
34+
///
35+
/// Returns `Err` if more than 10 continuation bytes accumulate across calls without a
36+
/// terminator, which would otherwise overflow the accumulated `i64`.
37+
pub fn long(&mut self, buf: &mut &[u8]) -> Result<Option<i64>, AvroError> {
3338
while let Some(byte) = buf.first().copied() {
39+
if self.shift == 63 && byte >= 0x02 {
40+
self.in_progress = 0;
41+
self.shift = 0;
42+
return Err(AvroError::ParseError(
43+
"Malformed Avro varint: too many continuation bytes".to_string(),
44+
));
45+
}
3446
*buf = &buf[1..];
3547
self.in_progress |= ((byte & 0x7F) as u64) << self.shift;
3648
self.shift += 7;
3749
if byte & 0x80 == 0 {
3850
let val = self.in_progress;
3951
self.in_progress = 0;
4052
self.shift = 0;
41-
return Some((val >> 1) as i64 ^ -((val & 1) as i64));
53+
return Ok(Some((val >> 1) as i64 ^ -((val & 1) as i64)));
4254
}
4355
}
44-
None
56+
Ok(None)
4557
}
4658
}
4759

@@ -163,4 +175,48 @@ mod tests {
163175
varint_test(rand::random());
164176
}
165177
}
178+
179+
fn zigzag_encode(n: i64) -> u64 {
180+
((n << 1) ^ (n >> 63)) as u64
181+
}
182+
183+
fn long_test(n: i64) {
184+
let mut buf = [0_u8; 10];
185+
let len = encode_var(zigzag_encode(n), &mut buf);
186+
let mut decoder = VLQDecoder::default();
187+
let mut slice = &buf[..len];
188+
assert_eq!(decoder.long(&mut slice).unwrap(), Some(n));
189+
assert!(slice.is_empty());
190+
}
191+
192+
#[test]
193+
fn test_long_roundtrip() {
194+
long_test(0);
195+
long_test(1);
196+
long_test(-1);
197+
long_test(4395932);
198+
long_test(-4395932);
199+
long_test(i64::MAX);
200+
long_test(i64::MIN);
201+
202+
for _ in 0..1000 {
203+
long_test(rand::random());
204+
}
205+
}
206+
207+
#[test]
208+
fn test_long_overlong_varint_returns_error() {
209+
// The Avro file magic followed by 13 continuation bytes and a terminator, as
210+
// reported against #10290: previously drove `self.shift` past 63 and panicked
211+
// with "attempt to shift left with overflow" inside `<< self.shift`.
212+
let overlong = [0xFFu8; 13];
213+
let mut decoder = VLQDecoder::default();
214+
let mut buf = &overlong[..];
215+
assert!(decoder.long(&mut buf).is_err());
216+
217+
// The decoder's state must be reset after a malformed varint, so a subsequent
218+
// call decodes a fresh varint cleanly rather than staying stuck mid-decode.
219+
let mut fresh = &[0x02u8][..]; // zigzag-encoded 1
220+
assert_eq!(decoder.long(&mut fresh).unwrap(), Some(1));
221+
}
166222
}

0 commit comments

Comments
 (0)