From 751e29d862023bed402e8044c9935b258f01c3de Mon Sep 17 00:00:00 2001 From: okdistribute <633012+okdistribute@users.noreply.github.com> Date: Fri, 8 Jan 2021 11:53:37 -1000 Subject: [PATCH 1/2] Change encode decode signatures to improve error handling --- Cargo.toml | 9 +++-- src/lib.rs | 95 ++++++++++++++++++++++++++------------------------- tests/test.rs | 31 +++++++++-------- 3 files changed, 71 insertions(+), 64 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 833e492..41a21b1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,9 +1,12 @@ [package] name = "varinteger" -version = "1.0.7-alpha.0" +version = "2.0.0" description = "Rust module for encoding/decoding varints that doesn't do any IO. Inspired by the Node.js varint module" -authors = ["Mathias Buus "] -repository = "https://github.com/mafintosh/varinteger-rs" +authors = ["James Halliday", "Karissa McKelvey ", "Mathias Buus "] +repository = "https://github.com/datrs/varinteger" keywords = ["protobuf", "varint", "variable", "integer"] readme = "README.md" license = "MIT" + +[dependencies] +failure = "0.1.8" diff --git a/src/lib.rs b/src/lib.rs index 39ff82d..5029b9c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,68 +1,73 @@ -#![cfg_attr(feature = "nightly", deny(missing_docs))] -#![cfg_attr(feature = "nightly", feature(external_doc))] -#![cfg_attr(feature = "nightly", doc(include = "../README.md"))] -#![cfg_attr(test, deny(warnings))] - -/// Returns how many bytes are needed to encode a value. -#[inline] -pub fn length(value: u64) -> usize { - let zero_len = 64 - value.leading_zeros(); - let offset = if zero_len == 0 { 7 } else { 6 }; - ((offset + zero_len) / 7) as usize -} +extern crate failure; +use failure::{bail, Error}; /// Encode a `u64` integer to the byte slice. Returns how many bytes were /// encoded. #[inline] -pub fn encode(value: u64, buf: &mut [u8]) -> usize { +pub fn encode(value: u64, buf: &mut [u8]) -> Result { encode_with_offset(value, buf, 0) } /// Encode a `u64` integer at a specific offset in the byte slice. Returns how /// many bytes were encoded. #[inline] -pub fn encode_with_offset(value: u64, buf: &mut [u8], offset: usize) -> usize { +pub fn encode_with_offset( + value: u64, + buf: &mut [u8], + offset: usize, +) -> Result { + let len = length(value); + if buf.len() < len { + bail!["buffer is too small to write varint"] + } + let mut v = value; let mut off = offset; - let mut val = value; - - while val > 127 { - buf[off] = (val as u8) | 128; + while v > 127 { + buf[off] = (v as u8) | 128; off += 1; - val >>= 7; + v >>= 7; } - buf[off] = val as u8; - - off + 1 - offset + buf[off] = v as u8; + Ok(len) } /// Decode a byte slice into a `u64` integer. Returns how many bytes were /// decoded. #[inline] -pub fn decode(buf: &[u8], value: &mut u64) -> usize { - decode_with_offset(buf, 0, value) +pub fn decode(buf: &[u8]) -> Result<(usize, u64), Error> { + decode_with_offset(buf, 0usize) } /// Decode a byte slice into a `u64` integer at a specific offset. Returns how /// many bytes were decoded. #[inline] -pub fn decode_with_offset(buf: &[u8], offset: usize, value: &mut u64) -> usize { - let mut val = 0 as u64; - let mut fac = 1 as u64; - let mut off = offset; - - loop { - let byte = buf[off]; - off += 1; - val += fac * u64::from(byte & 127); - fac <<= 7; +pub fn decode_with_offset( + buf: &[u8], + _offset: usize, +) -> Result<(usize, u64), Error> { + let mut value = 0u64; + let mut m = 1u64; + let mut offset = _offset; + for _i in 0..8 { + if offset >= buf.len() { + bail!["buffer supplied to varint decoding too small"] + } + let byte = buf[offset]; + offset += 1; + value += m * u64::from(byte & 127); + m *= 128; if byte & 128 == 0 { break; } } + Ok((offset, value)) +} - *value = val; - - off - offset +/// Returns how many bytes are needed to encode a value. +#[inline] +pub fn length(value: u64) -> usize { + let msb = (64 - value.leading_zeros()) as usize; + (msb.max(1) + 6) / 7 } /// Returns how many bytes are needed to encode a value. @@ -74,7 +79,7 @@ pub fn signed_length(value: i64) -> usize { /// Encode a `i64` (signed) integer at a specific offset in the byte slice. /// Returns how many bytes were encoded. #[inline] -pub fn signed_encode(value: i64, buf: &mut [u8]) -> usize { +pub fn signed_encode(value: i64, buf: &mut [u8]) -> Result { encode_with_offset(unsign(value), buf, 0) } @@ -85,15 +90,15 @@ pub fn signed_encode_with_offset( value: i64, buf: &mut [u8], offset: usize, -) -> usize { +) -> Result { encode_with_offset(unsign(value), buf, offset) } /// Decode a byte slice into a `i64` (signed) integer. Returns how many bytes /// were decoded. #[inline] -pub fn signed_decode(buf: &[u8], value: &mut i64) -> usize { - signed_decode_with_offset(buf, 0, value) +pub fn signed_decode(buf: &[u8]) -> Result<(usize, i64), Error> { + signed_decode_with_offset(buf, 0) } /// Decode a byte slice into a `i64` (signed) integer at a specific offset. @@ -102,12 +107,10 @@ pub fn signed_decode(buf: &[u8], value: &mut i64) -> usize { pub fn signed_decode_with_offset( buf: &[u8], offset: usize, - value: &mut i64, -) -> usize { +) -> Result<(usize, i64), Error> { let mut val = 0; - let off = decode_with_offset(buf, offset, &mut val); - *value = sign(val); - off + let (off, value) = decode_with_offset(buf, offset)?; + Ok((off, sign(value))) } /// Convert an `i64` into a `u64`. diff --git a/tests/test.rs b/tests/test.rs index 1ddebdb..38e3086 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1,4 +1,6 @@ +extern crate failure; extern crate varinteger; +use failure::Error; use varinteger::{ decode, encode, length, signed_decode, signed_encode, signed_length, @@ -7,22 +9,24 @@ use varinteger::{ #[test] fn test_encode() { let mut buf = [0; 512]; - assert_eq!(encode(100, &mut buf), 1); + assert_eq!(encode(100, &mut buf).unwrap(), 1); assert_eq!(buf[0], 100); - assert_eq!(encode(1000, &mut buf), 2); + assert_eq!(encode(1000, &mut buf).unwrap(), 2); assert_eq!(buf[0], 232); assert_eq!(buf[1], 7); } #[test] -fn test_decode() { - let mut value = 0 as u64; - assert_eq!(decode(&[100], &mut value), 1); - assert_eq!(value, 100); +fn test_encode_failure() { + let mut buf = [0; 1]; + assert!(encode(1000, &mut buf).is_err()); +} - assert_eq!(decode(&[232, 7], &mut value), 2); - assert_eq!(value, 1000); +#[test] +fn test_decode() { + assert_eq!(decode(&[100]).unwrap(), (1, 100)); + assert_eq!(decode(&[232, 7]).unwrap(), (2, 1000)); } #[test] @@ -42,23 +46,20 @@ fn test_length() { #[test] fn test_signed_encode() { let mut buf = [0; 512]; - assert_eq!(signed_encode(100, &mut buf), 2); + assert_eq!(signed_encode(100, &mut buf).unwrap(), 2); assert_eq!(buf[0], 200); assert_eq!(buf[1], 1); - assert_eq!(signed_encode(-100, &mut buf), 2); + assert_eq!(signed_encode(-100, &mut buf).unwrap(), 2); assert_eq!(buf[0], 199); assert_eq!(buf[1], 1); } #[test] fn test_signed_decode() { - let mut value = 0 as i64; - assert_eq!(signed_decode(&[200, 1], &mut value), 2); - assert_eq!(value, 100); + assert_eq!(signed_decode(&[200, 1]).unwrap(), (2, 100)); - assert_eq!(signed_decode(&[199, 1], &mut value), 2); - assert_eq!(value, -100); + assert_eq!(signed_decode(&[199, 1]).unwrap(), (2, -100)); } #[test] From ddc4e2c1da51d409a6abd08d7d52d93026f07e9a Mon Sep 17 00:00:00 2001 From: Karissa McKelvey <633012+okdistribute@users.noreply.github.com> Date: Mon, 24 May 2021 09:50:31 -0700 Subject: [PATCH 2/2] update based on frando's comments --- Cargo.toml | 5 +---- src/lib.rs | 35 ++++++++++++++++++++++------------- tests/test.rs | 2 -- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 41a21b1..520b654 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,4 @@ authors = ["James Halliday", "Karissa McKelvey ", "M repository = "https://github.com/datrs/varinteger" keywords = ["protobuf", "varint", "variable", "integer"] readme = "README.md" -license = "MIT" - -[dependencies] -failure = "0.1.8" +license = "MIT" \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index 5029b9c..7372638 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,10 +1,20 @@ -extern crate failure; -use failure::{bail, Error}; +use std::fmt; + +type Result = std::result::Result; + +#[derive(Debug, Clone)] +pub struct EncodeError; + +impl fmt::Display for EncodeError{ + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "buffer is too small to write varint") + } +} /// Encode a `u64` integer to the byte slice. Returns how many bytes were /// encoded. #[inline] -pub fn encode(value: u64, buf: &mut [u8]) -> Result { +pub fn encode(value: u64, buf: &mut [u8]) -> Result { encode_with_offset(value, buf, 0) } @@ -15,10 +25,10 @@ pub fn encode_with_offset( value: u64, buf: &mut [u8], offset: usize, -) -> Result { +) -> Result { let len = length(value); if buf.len() < len { - bail!["buffer is too small to write varint"] + return Err(EncodeError) } let mut v = value; let mut off = offset; @@ -34,7 +44,7 @@ pub fn encode_with_offset( /// Decode a byte slice into a `u64` integer. Returns how many bytes were /// decoded. #[inline] -pub fn decode(buf: &[u8]) -> Result<(usize, u64), Error> { +pub fn decode(buf: &[u8]) -> Result<(usize, u64)> { decode_with_offset(buf, 0usize) } @@ -44,13 +54,13 @@ pub fn decode(buf: &[u8]) -> Result<(usize, u64), Error> { pub fn decode_with_offset( buf: &[u8], _offset: usize, -) -> Result<(usize, u64), Error> { +) -> Result<(usize, u64)> { let mut value = 0u64; let mut m = 1u64; let mut offset = _offset; for _i in 0..8 { if offset >= buf.len() { - bail!["buffer supplied to varint decoding too small"] + return Err(EncodeError) } let byte = buf[offset]; offset += 1; @@ -79,7 +89,7 @@ pub fn signed_length(value: i64) -> usize { /// Encode a `i64` (signed) integer at a specific offset in the byte slice. /// Returns how many bytes were encoded. #[inline] -pub fn signed_encode(value: i64, buf: &mut [u8]) -> Result { +pub fn signed_encode(value: i64, buf: &mut [u8]) -> Result { encode_with_offset(unsign(value), buf, 0) } @@ -90,14 +100,14 @@ pub fn signed_encode_with_offset( value: i64, buf: &mut [u8], offset: usize, -) -> Result { +) -> Result { encode_with_offset(unsign(value), buf, offset) } /// Decode a byte slice into a `i64` (signed) integer. Returns how many bytes /// were decoded. #[inline] -pub fn signed_decode(buf: &[u8]) -> Result<(usize, i64), Error> { +pub fn signed_decode(buf: &[u8]) -> Result<(usize, i64)> { signed_decode_with_offset(buf, 0) } @@ -107,8 +117,7 @@ pub fn signed_decode(buf: &[u8]) -> Result<(usize, i64), Error> { pub fn signed_decode_with_offset( buf: &[u8], offset: usize, -) -> Result<(usize, i64), Error> { - let mut val = 0; +) -> Result<(usize, i64)> { let (off, value) = decode_with_offset(buf, offset)?; Ok((off, sign(value))) } diff --git a/tests/test.rs b/tests/test.rs index 38e3086..ccdb5d1 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1,6 +1,4 @@ -extern crate failure; extern crate varinteger; -use failure::Error; use varinteger::{ decode, encode, length, signed_decode, signed_encode, signed_length,