You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "RanaPriyansh (via GitHub)" <gi...@apache.org> on 2026/09/20 19:27:09 UTC
[PR] fix: Reject empty Decimal bytes during schema-aware serialization [avro-rs]
RanaPriyansh opened a new pull request, #684:
URL: https://github.com/apache/avro-rs/pull/684
Fixes #683.
Schema-aware serialization accepted empty bytes for a bytes-backed Decimal, although the reader rejects that encoding. Reject empty Decimal bytes in direct and selected union branches before writing the length or bare union index. Ordinary empty bytes and existing union selection remain unchanged.
Regression coverage includes direct and union serialization, container writes, nullable Decimal values, and valid bytes, Decimal, and fixed controls.
Verification on macOS with Rust 1.98.1: focused regressions with default and all features, formatting, and workspace all-feature/all-target Clippy with warnings denied passed. Other platform and CI checks remain pending.
Implementation, tests, and review were assisted by OpenAI Codex.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: Reject empty Decimal bytes during schema-aware serialization [avro-rs]
Posted by "Kriskras99 (via GitHub)" <gi...@apache.org>.
Kriskras99 merged PR #684:
URL: https://github.com/apache/avro-rs/pull/684
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: Reject empty Decimal bytes during schema-aware serialization [avro-rs]
Posted by "Kriskras99 (via GitHub)" <gi...@apache.org>.
Kriskras99 commented on code in PR #684:
URL: https://github.com/apache/avro-rs/pull/684#discussion_r4060848932
##########
avro/src/serde/ser_schema/mod.rs:
##########
@@ -355,6 +355,10 @@ impl<'s, 'w, W: Write, S: Borrow<Schema>> Serializer for SchemaAwareSerializer<'
fn serialize_bytes(mut self, v: &[u8]) -> Result<Self::Ok, Self::Error> {
match self.schema {
+ Schema::Decimal(DecimalSchema {
+ inner: InnerDecimalSchema::Bytes,
+ ..
+ }) if v.is_empty() => Err(Details::DecimalIsZeroLength.into()),
Review Comment:
Use the builtin error function of the serializer:
```rust
Err(self.error("bytes", "Bytes cannot be empty for a Schema::Decimal"))
```
##########
avro/src/serde/ser_schema/union.rs:
##########
@@ -314,6 +314,18 @@ impl<'s, 'w, W: Write, S: Borrow<Schema>> Serializer for UnionSerializer<'s, 'w,
}
}
};
+ if with_len
+ && v.is_empty()
+ && matches!(
+ self.union.get_variant(index)?,
+ Schema::Decimal(crate::schema::DecimalSchema {
+ inner: crate::schema::InnerDecimalSchema::Bytes,
+ ..
+ })
+ )
+ {
+ return Err(Details::DecimalIsZeroLength.into());
Review Comment:
Use the builtin error type of the serializer:
```rust
Err(self.error("bytes", "Bytes cannot be empty for a Schema::Decimal"))
```
##########
avro/src/serde/ser_schema/mod.rs:
##########
@@ -1459,6 +1467,158 @@ mod tests {
Ok(())
}
+ #[test]
+ fn avro_rs_683_rejects_direct_empty_decimal_bytes() -> TestResult {
+ let decimal =
+ Schema::parse_str(r#"{"type":"bytes","logicalType":"decimal","precision":4}"#)?;
+ let writer = GenericDatumWriter::builder(&decimal).build()?;
+ let mut buffer = Vec::new();
+ let error = writer
+ .write_ser(&mut buffer, &ByteBuf::new())
+ .expect_err("empty Decimal bytes must be rejected");
+ assert!(matches!(error.into_details(), Details::DecimalIsZeroLength));
+ assert!(buffer.is_empty());
+
+ let mut writer = Writer::new(&decimal, Vec::new())?;
+ let error = writer
+ .append_ser(ByteBuf::new())
+ .expect_err("Writer::append_ser must reject empty Decimal bytes");
+ assert!(matches!(error.into_details(), Details::DecimalIsZeroLength));
+
+ Ok(())
+ }
+
+ #[test]
+ fn avro_rs_683_rejects_union_empty_decimal_bytes() -> TestResult {
+ let union_first = Schema::parse_str(
+ r#"[{"type":"bytes","logicalType":"decimal","precision":4},"string"]"#,
+ )?;
+ let union_second = Schema::parse_str(
+ r#"["string",{"type":"bytes","logicalType":"decimal","precision":4}]"#,
+ )?;
+ for schema in [&union_first, &union_second] {
+ let writer = GenericDatumWriter::builder(schema).build()?;
+ let error = writer
+ .write_ser_to_vec(&ByteBuf::new())
+ .expect_err("empty union Decimal bytes must be rejected");
+ assert!(matches!(error.into_details(), Details::DecimalIsZeroLength));
+
+ let mut writer = Writer::new(schema, Vec::new())?;
+ let error = writer
+ .append_ser(ByteBuf::new())
+ .expect_err("Writer::append_ser must reject empty union Decimal bytes");
+ assert!(matches!(error.into_details(), Details::DecimalIsZeroLength));
+ }
+
+ let nullable = Schema::parse_str(
+ r#"["null",{"type":"bytes","logicalType":"decimal","precision":4}]"#,
+ )?;
+ let writer = GenericDatumWriter::builder(&nullable).build()?;
+ let error = writer
+ .write_ser_to_vec(&Some(ByteBuf::new()))
+ .expect_err("Some(empty Decimal bytes) must be rejected");
+ assert!(matches!(error.into_details(), Details::DecimalIsZeroLength));
+
+ Ok(())
+ }
+
+ #[test]
+ fn avro_rs_683_union_error_writes_no_prefix() -> TestResult {
+ let schema = Schema::parse_str(
+ r#"[{"type":"bytes","logicalType":"decimal","precision":4},"string"]"#,
+ )?;
+ let writer = GenericDatumWriter::builder(&schema).build()?;
+ let mut buffer = Vec::new();
+ let error = writer
+ .write_ser(&mut buffer, &ByteBuf::new())
+ .expect_err("empty Decimal bytes must be rejected");
+ assert!(matches!(error.into_details(), Details::DecimalIsZeroLength));
+ assert!(buffer.is_empty());
+ Ok(())
+ }
+
+ #[test]
+ fn avro_rs_683_controls() -> TestResult {
+ let decimal =
+ Schema::parse_str(r#"{"type":"bytes","logicalType":"decimal","precision":4}"#)?;
+ let union_first = Schema::parse_str(
+ r#"[{"type":"bytes","logicalType":"decimal","precision":4},"string"]"#,
+ )?;
+ let union_second = Schema::parse_str(
+ r#"["string",{"type":"bytes","logicalType":"decimal","precision":4}]"#,
+ )?;
+
+ let bytes = Schema::parse_str(r#"["bytes","string"]"#)?;
+ let writer = GenericDatumWriter::builder(&bytes).build()?;
+ let encoded = writer.write_ser_to_vec(&ByteBuf::new())?;
+ assert_eq!(encoded, [0, 0]);
+ let reader = GenericDatumReader::builder(&bytes).build()?;
+ assert!(reader.read_value(&mut &encoded[..]).is_ok());
+
+ let direct_bytes = Schema::Bytes;
+ let writer = GenericDatumWriter::builder(&direct_bytes).build()?;
+ let encoded = writer.write_ser_to_vec(&ByteBuf::new())?;
+ assert_eq!(encoded, [0]);
+ let reader = GenericDatumReader::builder(&direct_bytes).build()?;
+ assert!(reader.read_value(&mut &encoded[..]).is_ok());
+
+ let decimal_zero = Decimal::new([0])?;
+ let direct_writer = GenericDatumWriter::builder(&decimal).build()?;
+ let direct = direct_writer.write_ser_to_vec(&decimal_zero)?;
+ assert_eq!(direct, [2, 0]);
+ let direct_reader = GenericDatumReader::builder(&decimal).build()?;
+ assert_eq!(
+ direct_reader.read_value(&mut &direct[..])?,
+ Value::Decimal(decimal_zero.clone())
+ );
+
+ let decimal_nonzero = Decimal::new([251, 155])?;
+ let nonzero = direct_writer.write_ser_to_vec(&decimal_nonzero)?;
+ assert_eq!(nonzero, [4, 251, 155]);
+ assert_eq!(
+ direct_reader.read_value(&mut &nonzero[..])?,
+ Value::Decimal(decimal_nonzero)
+ );
+
+ for (schema, expected) in [(&union_first, [0, 2, 0]), (&union_second, [2, 2, 0])] {
+ let writer = GenericDatumWriter::builder(schema).build()?;
+ let encoded = writer.write_ser_to_vec(&decimal_zero)?;
+ assert_eq!(encoded, expected);
+ let reader = GenericDatumReader::builder(schema).build()?;
+ let index = u32::from(expected[0] / 2);
+ assert_eq!(
+ reader.read_value(&mut &encoded[..])?,
+ Value::Union(index, Box::new(Value::Decimal(decimal_zero.clone())))
+ );
+ }
+
+ let fixed_first = Schema::parse_str(
+ r#"[{"type":"fixed","name":"OneByte","size":1},{"type":"bytes","logicalType":"decimal","precision":4}]"#,
+ )?;
+ let decimal_first = Schema::parse_str(
+ r#"[{"type":"bytes","logicalType":"decimal","precision":4},{"type":"fixed","name":"OneByte","size":1}]"#,
+ )?;
+ let writer = GenericDatumWriter::builder(&fixed_first).build()?;
+ assert_eq!(writer.write_ser_to_vec(&ByteBuf::from(vec![7]))?, [0, 7]);
+ let writer = GenericDatumWriter::builder(&decimal_first).build()?;
+ assert_eq!(writer.write_ser_to_vec(&ByteBuf::from(vec![7]))?, [0, 2, 7]);
+
+ let fixed_decimal = Schema::parse_str(
+ r#"{"type":"fixed","name":"FixedZero","size":2,"logicalType":"decimal","precision":4}"#,
+ )?;
+ let fixed_zero = Decimal::new([0, 0])?;
+ let writer = GenericDatumWriter::builder(&fixed_decimal).build()?;
+ let encoded = writer.write_ser_to_vec(&fixed_zero)?;
+ assert_eq!(encoded, [0, 0]);
+ let reader = GenericDatumReader::builder(&fixed_decimal).build()?;
+ assert_eq!(
+ reader.read_value(&mut &encoded[..])?,
+ Value::Decimal(fixed_zero)
+ );
+
+ Ok(())
+ }
+
Review Comment:
These tests are overly complex, verbose, and mostly irrelevant to the changes
```suggestion
#[test]
fn avro_rs_683_empty_decimal_bytes() -> TestResult {
let schema =
Schema::parse_str(r#"{"type": "bytes", "logicalType": "decimal", "precision": 42}"#)?;
let names = HashMap::new();
assert_serialize(Bytes::new(&[0]), &schema, &names, &[0x2, 0x0]);
assert_serialize_err(
Bytes::new(&[]),
&schema,
&names,
r"Failed to serialize value of type `bytes` using Schema::Decimal(DecimalSchema { precision: 42, scale: 0, inner: Bytes }): Bytes cannot be empty for a Schema::Decimal",
);
Ok(())
}
#[test]
fn avro_rs_683_union_empty_decimal_bytes() -> TestResult {
let schema =
Schema::parse_str(r#"[{"type": "bytes", "logicalType": "decimal", "precision": 42}]"#)?;
let names = HashMap::new();
assert_serialize(Bytes::new(&[0]), &schema, &names, &[0x00, 0x2, 0x0]);
assert_serialize_err(
Bytes::new(&[]),
&schema,
&names,
r"Failed to serialize value of type `bytes` using Schema::Union(UnionSchema { schemas: [Decimal(DecimalSchema { precision: 42, scale: 0, inner: Bytes })] }): Bytes cannot be empty for a Schema::Decimal",
);
Ok(())
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org