This is an automated email from the ASF dual-hosted git repository.
Kriskras99 pushed a commit to branch fix/decimal_zero_length_bytes
in repository https://gitbox.apache.org/repos/asf/avro-rs.git
The following commit(s) were added to refs/heads/fix/decimal_zero_length_bytes
by this push:
new 9ea014b fix!: `Decimal` should not be able to decode from a
zero-length bytes
9ea014b is described below
commit 9ea014b001fb8caf70693c9f8914b96cc08a23b1
Author: Kriskras99 <[email protected]>
AuthorDate: Wed Sep 16 10:09:44 2026 +0200
fix!: `Decimal` should not be able to decode from a zero-length bytes
This is a breaking change because it is not possible to do
`impl<T: AsRef<[u8]>> TryFrom<T> for Decimal` as we have a
`TryFrom<Decimal>`
implementation on `Vec<u8>` which causes auto-trait conflicts.
---
avro/src/decimal.rs | 40 +++++++++++++++++++-------------------
avro/src/decode.rs | 22 ++++++++++++++++-----
avro/src/error.rs | 3 +++
avro/src/schema_compatibility.rs | 2 +-
avro/src/serde/de.rs | 10 +++++-----
avro/src/serde/deser_schema/mod.rs | 4 ++--
avro/src/serde/ser.rs | 2 +-
avro/src/serde/ser_schema/mod.rs | 4 ++--
avro/src/types.rs | 22 ++++++++++-----------
avro/src/writer/datum.rs | 4 ++--
10 files changed, 64 insertions(+), 49 deletions(-)
diff --git a/avro/src/decimal.rs b/avro/src/decimal.rs
index c219b5d..0d31e16 100644
--- a/avro/src/decimal.rs
+++ b/avro/src/decimal.rs
@@ -18,11 +18,12 @@
use crate::{AvroResult, Error, error::Details};
use num_bigint::{BigInt, Sign};
use serde::{Deserialize, Serialize, Serializer, de::SeqAccess};
+use std::num::NonZero;
#[derive(Debug, Clone, Eq)]
pub struct Decimal {
value: BigInt,
- len: usize,
+ len: NonZero<usize>,
}
impl Serialize for Decimal {
@@ -53,18 +54,19 @@ impl<'de> Deserialize<'de> for Decimal {
where
E: serde::de::Error,
{
- Ok(Decimal::from(v))
+ Decimal::new(v).map_err(E::custom)
}
fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
where
A: SeqAccess<'de>,
{
+ use serde::de::Error;
let mut bytes = Vec::new();
while let Some(value) = seq.next_element::<u8>()? {
bytes.push(value);
}
- Ok(Decimal::from(bytes))
+ Decimal::new(bytes).map_err(A::Error::custom)
}
}
deserializer.deserialize_bytes(DecimalVisitor)
@@ -80,12 +82,20 @@ impl PartialEq for Decimal {
}
impl Decimal {
+ pub fn new(bytes: impl AsRef<[u8]>) -> AvroResult<Self> {
+ let bytes_ref = bytes.as_ref();
+ Ok(Self {
+ value: BigInt::from_signed_bytes_be(bytes_ref),
+ len:
NonZero::new(bytes_ref.len()).ok_or(Details::DecimalIsZeroLength)?,
+ })
+ }
+
pub(crate) fn len(&self) -> usize {
- self.len
+ self.len.get()
}
pub(crate) fn to_vec(&self) -> AvroResult<Vec<u8>> {
- self.to_sign_extended_bytes_with_len(self.len)
+ self.to_sign_extended_bytes_with_len(self.len.get())
}
pub(crate) fn to_sign_extended_bytes_with_len(&self, len: usize) ->
AvroResult<Vec<u8>> {
@@ -117,7 +127,7 @@ impl From<Decimal> for BigInt {
/// let decimal = Decimal::from(vec![1, 24]);
/// let maybe_bytes = <Vec<u8>>::try_from(&decimal);
/// ```
-impl std::convert::TryFrom<&Decimal> for Vec<u8> {
+impl TryFrom<&Decimal> for Vec<u8> {
type Error = Error;
fn try_from(decimal: &Decimal) -> Result<Self, Self::Error> {
@@ -134,7 +144,7 @@ impl std::convert::TryFrom<&Decimal> for Vec<u8> {
/// let decimal = Decimal::from(vec![1, 24]);
/// let maybe_bytes = <Vec<u8>>::try_from(decimal);
/// ```
-impl std::convert::TryFrom<Decimal> for Vec<u8> {
+impl TryFrom<Decimal> for Vec<u8> {
type Error = Error;
fn try_from(decimal: Decimal) -> Result<Self, Self::Error> {
@@ -142,16 +152,6 @@ impl std::convert::TryFrom<Decimal> for Vec<u8> {
}
}
-impl<T: AsRef<[u8]>> From<T> for Decimal {
- fn from(bytes: T) -> Self {
- let bytes_ref = bytes.as_ref();
- Self {
- value: BigInt::from_signed_bytes_be(bytes_ref),
- len: bytes_ref.len(),
- }
- }
-}
-
#[cfg(test)]
mod tests {
use super::*;
@@ -161,7 +161,7 @@ mod tests {
#[test]
fn test_decimal_from_bytes_from_ref_decimal() -> TestResult {
let input = vec![1, 24];
- let d = Decimal::from(&input);
+ let d = Decimal::new(&input)?;
let output = <Vec<u8>>::try_from(&d)?;
assert_eq!(output, input);
@@ -172,7 +172,7 @@ mod tests {
#[test]
fn test_decimal_from_bytes_from_owned_decimal() -> TestResult {
let input = vec![1, 24];
- let d = Decimal::from(&input);
+ let d = Decimal::new(&input)?;
let output = <Vec<u8>>::try_from(d)?;
assert_eq!(output, input);
@@ -182,7 +182,7 @@ mod tests {
#[test]
fn avro_3949_decimal_serde() -> TestResult {
- let decimal = Decimal::from(&[1, 2, 3]);
+ let decimal = Decimal::new([1, 2, 3])?;
let ser = serde_json::to_string(&decimal)?;
let de = serde_json::from_str(&ser)?;
diff --git a/avro/src/decode.rs b/avro/src/decode.rs
index 9c06eb9..4c62647 100644
--- a/avro/src/decode.rs
+++ b/avro/src/decode.rs
@@ -194,13 +194,13 @@ fn decode_internal_body<R: Read, S: Borrow<Schema>>(
reader,
ctx,
)? {
- Value::Fixed(_, bytes) =>
Ok(Value::Decimal(Decimal::from(bytes))),
+ Value::Fixed(_, bytes) =>
Ok(Value::Decimal(Decimal::new(bytes)?)),
value => Err(Details::FixedValue(value).into()),
}
}
InnerDecimalSchema::Bytes => {
match decode_internal(&Schema::Bytes, names,
enclosing_namespace, reader, ctx)? {
- Value::Bytes(bytes) =>
Ok(Value::Decimal(Decimal::from(bytes))),
+ Value::Bytes(bytes) =>
Ok(Value::Decimal(Decimal::new(bytes)?)),
value => Err(Details::BytesValue(value).into()),
}
}
@@ -697,7 +697,7 @@ mod tests {
scale: 2,
});
let bigint = (-423).to_bigint().unwrap();
- let value = Value::Decimal(Decimal::from(bigint.to_signed_bytes_be()));
+ let value = Value::Decimal(Decimal::new(bigint.to_signed_bytes_be())?);
let mut buffer = Vec::new();
encode(&value, &schema, &mut buffer).expect(&success(&value, &schema));
@@ -724,9 +724,9 @@ mod tests {
precision: NonZero::new(4).unwrap(),
scale: 2,
});
- let value = Value::Decimal(Decimal::from(
+ let value = Value::Decimal(Decimal::new(
((-423).to_bigint().unwrap()).to_signed_bytes_be(),
- ));
+ )?);
let mut buffer = Vec::<u8>::new();
encode(&value, &schema, &mut buffer).expect(&success(&value, &schema));
@@ -1182,4 +1182,16 @@ mod tests {
Ok(())
}
+
+ #[test]
+ fn avro_rs_680_decimal_bytes_zero_length() -> TestResult {
+ let schema =
+ Schema::parse_str(r#"{"type": "bytes", "logicalType": "decimal",
"precision": 4}"#)?;
+ // 0x00 is the varint encoding of length 0 (zero-length bytes)
+ let mut buf: &[u8] = &[0x00];
+ let error = decode(&schema, &mut buf).unwrap_err().into_details();
+ assert_eq!(error.to_string(), "Decimal bytes cannot be zero length");
+
+ Ok(())
+ }
}
diff --git a/avro/src/error.rs b/avro/src/error.rs
index 924e8f9..6811cfe 100644
--- a/avro/src/error.rs
+++ b/avro/src/error.rs
@@ -92,6 +92,9 @@ pub enum Details {
#[error("Not a fixed or bytes type, required for decimal schema, got:
{0:?}")]
ResolveDecimalSchema(SchemaKind),
+ #[error("Decimal bytes cannot be zero length")]
+ DecimalIsZeroLength,
+
#[error("Invalid utf-8 string")]
ConvertToUtf8(#[source] std::string::FromUtf8Error),
diff --git a/avro/src/schema_compatibility.rs b/avro/src/schema_compatibility.rs
index 13f07fc..32dd8a7 100644
--- a/avro/src/schema_compatibility.rs
+++ b/avro/src/schema_compatibility.rs
@@ -1712,7 +1712,7 @@ mod tests {
SchemaCompatibility::mutual_read(&bytes, &fixed)?
);
- let value = Value::Decimal(Decimal::from(vec![1; 10]));
+ let value = Value::Decimal(Decimal::new([1; 10])?);
let fixed_value = value.clone().resolve(&fixed)?;
let bytes_value = value.resolve(&bytes)?;
diff --git a/avro/src/serde/de.rs b/avro/src/serde/de.rs
index 20e3beb..52f9229 100644
--- a/avro/src/serde/de.rs
+++ b/avro/src/serde/de.rs
@@ -1150,12 +1150,12 @@ mod tests {
let test = Value::Record(vec![
("a".to_owned(), Value::Long(27)),
("b".to_owned(), Value::String("foo".to_owned())),
- ("c".to_owned(), Value::Decimal(Decimal::from(vec![1, 24]))),
+ ("c".to_owned(), Value::Decimal(Decimal::new([1, 24])?)),
]);
let expected = Test {
a: 27,
b: "foo".to_owned(),
- c: Decimal::from(vec![1, 24]),
+ c: Decimal::new([1, 24])?,
};
let final_value: Test = from_value(&test)?;
assert_eq!(final_value, expected);
@@ -1166,7 +1166,7 @@ mod tests {
Value::Record(vec![
("a".to_owned(), Value::Long(27)),
("b".to_owned(), Value::String("foo".to_owned())),
- ("c".to_owned(), Value::Decimal(Decimal::from(vec![1,
24]))),
+ ("c".to_owned(), Value::Decimal(Decimal::new([1, 24])?)),
]),
),
("b".to_owned(), Value::Int(35)),
@@ -1794,11 +1794,11 @@ mod tests {
#[test]
fn test_avro_3892_deserialize_bytes_from_decimal() -> TestResult {
let expected_bytes = BigInt::from(123456789).to_signed_bytes_be();
- let value = Value::Decimal(Decimal::from(&expected_bytes));
+ let value = Value::Decimal(Decimal::new(&expected_bytes)?);
let raw_bytes = from_value::<Bytes>(&value)?;
assert_eq!(raw_bytes.0, expected_bytes);
- let value = Value::Union(0,
Box::new(Value::Decimal(Decimal::from(&expected_bytes))));
+ let value = Value::Union(0,
Box::new(Value::Decimal(Decimal::new(&expected_bytes)?)));
let raw_bytes = from_value::<Option<Bytes>>(&value)?;
assert_eq!(raw_bytes.unwrap().0, expected_bytes);
Ok(())
diff --git a/avro/src/serde/deser_schema/mod.rs
b/avro/src/serde/deser_schema/mod.rs
index 0d9d3ad..7ef90ee 100644
--- a/avro/src/serde/deser_schema/mod.rs
+++ b/avro/src/serde/deser_schema/mod.rs
@@ -991,7 +991,7 @@ mod tests {
let test = Test {
a: 27,
b: "foo".to_string(),
- c: Decimal::from(vec![1, 24]),
+ c: Decimal::new([1, 24])?,
};
assert_roundtrip(test.clone(), &schemas[0], Vec::new())?;
@@ -1845,7 +1845,7 @@ mod tests {
)?;
let expected_bytes = BigInt::from(123456789).to_signed_bytes_be();
- let value = Decimal::from(&expected_bytes);
+ let value = Decimal::new(&expected_bytes)?;
assert_roundtrip(value.clone(), &schema, Vec::new())?;
assert_roundtrip(value.clone(), &Schema::union(vec![schema])?,
Vec::new())?;
diff --git a/avro/src/serde/ser.rs b/avro/src/serde/ser.rs
index efd6fe8..bd41a56 100644
--- a/avro/src/serde/ser.rs
+++ b/avro/src/serde/ser.rs
@@ -689,7 +689,7 @@ mod tests {
let test = Test {
a: 27,
b: "foo".to_owned(),
- decimal: Decimal::from(vec![1, 24]),
+ decimal: Decimal::new([1, 24])?,
};
let expected = Value::Record(vec![
("a".to_owned(), Value::Long(27)),
diff --git a/avro/src/serde/ser_schema/mod.rs b/avro/src/serde/ser_schema/mod.rs
index 4369235..f47beec 100644
--- a/avro/src/serde/ser_schema/mod.rs
+++ b/avro/src/serde/ser_schema/mod.rs
@@ -1447,7 +1447,7 @@ mod tests {
let names = HashMap::new();
- let val = Decimal::from(&[251, 155]);
+ let val = Decimal::new([251, 155])?;
assert_serialize(val, &schema, &names, &[4, 251, 155]);
assert_serialize_err(
(),
@@ -1474,7 +1474,7 @@ mod tests {
let names = HashMap::new();
- let val = Decimal::from(&[0, 0, 0, 0, 0, 0, 251, 155]);
+ let val = Decimal::new([0, 0, 0, 0, 0, 0, 251, 155])?;
assert_serialize(val, &schema, &names, &[0, 0, 0, 0, 0, 0, 251, 155]);
assert_serialize_err(
(),
diff --git a/avro/src/types.rs b/avro/src/types.rs
index ddae551..9d5819a 100644
--- a/avro/src/types.rs
+++ b/avro/src/types.rs
@@ -935,7 +935,7 @@ impl Value {
.into())
} else {
// precision and scale match, can we assume the underlying
type can hold the data?
- Ok(Value::Decimal(Decimal::from(bytes)))
+ Ok(Value::Decimal(Decimal::new(bytes)?))
}
}
@@ -956,7 +956,7 @@ impl Value {
}
})
.collect::<Result<Vec<u8>, Error>>()?;
- Ok(Value::Decimal(Decimal::from(bytes)))
+ Ok(Value::Decimal(Decimal::new(bytes)?))
}
other => Err(Details::ResolveDecimal(other).into()),
}
@@ -1894,7 +1894,7 @@ Field with name '"b"' is not a member of the map items"#,
#[test]
fn resolve_decimal_bytes() -> TestResult {
- let value = Value::Decimal(Decimal::from(vec![1, 2, 3, 4, 5]));
+ let value = Value::Decimal(Decimal::new([1, 2, 3, 4, 5])?);
value.clone().resolve(&Schema::Decimal(DecimalSchema {
precision: NonZero::new(10).unwrap(),
scale: 4,
@@ -1913,7 +1913,7 @@ Field with name '"b"' is not a member of the map items"#,
scale: 4,
inner: InnerDecimalSchema::Bytes,
}))?;
- assert_eq!(resolved, Value::Decimal(Decimal::from(vec![0u8])));
+ assert_eq!(resolved, Value::Decimal(Decimal::new([0u8])?));
let mut all_bytes_str = String::new();
for b in 0u8..=255u8 {
@@ -1926,7 +1926,7 @@ Field with name '"b"' is not a member of the map items"#,
}))?;
assert_eq!(
resolved,
- Value::Decimal(Decimal::from((0u8..=255u8).collect::<Vec<_>>()))
+ Value::Decimal(Decimal::new((0u8..=255u8).collect::<Vec<_>>())?)
);
let value = Value::String("\u{0100}".to_string());
@@ -1970,7 +1970,7 @@ Field with name '"b"' is not a member of the map items"#,
#[test]
fn resolve_decimal_invalid_scale() {
- let value = Value::Decimal(Decimal::from(vec![1, 2]));
+ let value = Value::Decimal(Decimal::new([1, 2]).unwrap());
assert!(
value
.resolve(&Schema::Decimal(DecimalSchema {
@@ -1984,7 +1984,7 @@ Field with name '"b"' is not a member of the map items"#,
#[test]
fn resolve_decimal_invalid_precision_for_length() {
- let value =
Value::Decimal(Decimal::from((1u8..=8u8).rev().collect::<Vec<_>>()));
+ let value =
Value::Decimal(Decimal::new((1u8..=8u8).rev().collect::<Vec<_>>()).unwrap());
assert!(
value
.resolve(&Schema::Decimal(DecimalSchema {
@@ -1998,7 +1998,7 @@ Field with name '"b"' is not a member of the map items"#,
#[test]
fn resolve_decimal_fixed() {
- let value = Value::Decimal(Decimal::from(vec![1, 2, 3, 4, 5]));
+ let value = Value::Decimal(Decimal::new([1, 2, 3, 4, 5]).unwrap());
assert!(
value
.clone()
@@ -2325,7 +2325,7 @@ Field with name '"b"' is not a member of the map items"#,
JsonValue::Number(1.into())
);
assert_eq!(
- JsonValue::try_from(Value::Decimal(vec![1, 2, 3].into()))?,
+ JsonValue::try_from(Value::Decimal(Decimal::new([1, 2, 3])?))?,
JsonValue::Array(vec