Skip to content

Commit

Permalink
AVRO-3965: [Rust] Default values for fixed can be longer than size (#…
Browse files Browse the repository at this point in the history 
…2907)

Return an error if the default value's length is not the same as the
specified size of a Fixed schema

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
  • Loading branch information
@martin-g
martin-g committed May 15, 2024
1 parent 5cdbefa commit 0661bfe
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 3 deletions.
4 changes: 4 additions & 0 deletions lang/rust/avro/src/decode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ pub(crate) fn decode_internal<R: Read, S: Borrow<Schema>>(
name: "uuid".into(),
aliases: None,
doc: None,
default: None,
attributes: Default::default(),
}),
names,
Expand Down Expand Up @@ -419,6 +420,7 @@ mod tests {
doc: None,
name: Name::new("decimal")?,
aliases: None,
default: None,
attributes: Default::default(),
}));
let schema = Schema::Decimal(DecimalSchema {
Expand Down Expand Up @@ -448,6 +450,7 @@ mod tests {
name: Name::new("decimal")?,
aliases: None,
doc: None,
default: None,
attributes: Default::default(),
}));
let schema = Schema::Decimal(DecimalSchema {
Expand Down Expand Up @@ -893,6 +896,7 @@ mod tests {
name: "uuid".into(),
aliases: None,
doc: None,
default: None,
attributes: Default::default(),
});
let value = Value::Uuid(Uuid::parse_str("550e8400-e29b-41d4-a716-446655440000")?);
Expand Down
1 change: 1 addition & 0 deletions lang/rust/avro/src/encode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ pub(crate) mod tests {
name: "uuid".into(),
aliases: None,
doc: None,
default: None,
attributes: Default::default(),
});
let value = Value::Uuid(Uuid::parse_str("550e8400-e29b-41d4-a716-446655440000")?);
Expand Down
3 changes: 3 additions & 0 deletions lang/rust/avro/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,9 @@ pub enum Error {
#[error("Fixed schema has no `size`")]
GetFixedSizeField,

#[error("Fixed schema's default value length ({0}) does not match its size ({1})")]
FixedDefaultLenSizeMismatch(usize, u64),

#[error("Failed to compress with flate")]
DeflateCompress(#[source] std::io::Error),

Expand Down
51 changes: 48 additions & 3 deletions lang/rust/avro/src/schema.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,8 @@ pub struct FixedSchema {
pub doc: Documentation,
/// The size of the fixed schema
pub size: usize,
/// An optional default symbol used for compatibility
pub default: Option<String>,
/// The custom attributes of the schema
pub attributes: BTreeMap<String, Value>,
}
Expand Down Expand Up @@ -1832,6 +1834,18 @@ impl Parser {
None => Err(Error::GetFixedSizeField),
}?;

let default = complex.get("default").and_then(|v| match &v {
Value::String(ref default) => Some(default.clone()),
_ => None,
});

if default.is_some() {
let len = default.clone().unwrap().len();
if len != size as usize {
return Err(Error::FixedDefaultLenSizeMismatch(len, size));
}
}

let fully_qualified_name = Name::parse(complex, enclosing_namespace)?;
let aliases = fix_aliases_namespace(complex.aliases(), &fully_qualified_name.namespace);

Expand All @@ -1840,6 +1854,7 @@ impl Parser {
aliases: aliases.clone(),
doc,
size: size as usize,
default,
attributes: self.get_custom_attributes(complex, vec!["size"]),
});

Expand Down Expand Up @@ -2080,6 +2095,7 @@ impl Serialize for Schema {
aliases: None,
doc: None,
size: 12,
default: None,
attributes: Default::default(),
});
map.serialize_entry("type", &inner)?;
Expand Down Expand Up @@ -3192,6 +3208,7 @@ mod tests {
aliases: None,
doc: None,
size: 456,
default: None,
attributes: Default::default(),
}),
order: RecordFieldOrder::Ascending,
Expand All @@ -3211,6 +3228,7 @@ mod tests {
aliases: None,
doc: None,
size: 456,
default: None,
attributes: Default::default(),
}),
order: RecordFieldOrder::Ascending,
Expand Down Expand Up @@ -3285,7 +3303,8 @@ mod tests {
name: Name::new("test")?,
aliases: None,
doc: None,
size: 16usize,
size: 16_usize,
default: None,
attributes: Default::default(),
});

Expand All @@ -3304,7 +3323,8 @@ mod tests {
name: Name::new("test")?,
aliases: None,
doc: Some(String::from("FixedSchema documentation")),
size: 16usize,
size: 16_usize,
default: None,
attributes: Default::default(),
});

Expand Down Expand Up @@ -6256,6 +6276,7 @@ mod tests {
aliases: None,
doc: None,
size: 1,
default: None,
attributes: attributes.clone(),
});
let serialized = serde_json::to_string(&schema)?;
Expand Down Expand Up @@ -6380,11 +6401,12 @@ mod tests {
aliases: None,
doc: None,
size: 6,
default: None,
attributes: BTreeMap::from([("logicalType".to_string(), "uuid".into())]),
})
);
assert_logged(
r#"Ignoring uuid logical type for a Fixed schema because its size (6) is not 16! Schema: Fixed(FixedSchema { name: Name { name: "FixedUUID", namespace: None }, aliases: None, doc: None, size: 6, attributes: {"logicalType": String("uuid")} })"#,
r#"Ignoring uuid logical type for a Fixed schema because its size (6) is not 16! Schema: Fixed(FixedSchema { name: Name { name: "FixedUUID", namespace: None }, aliases: None, doc: None, size: 6, default: None, attributes: {"logicalType": String("uuid")} })"#,
);

Ok(())
Expand Down Expand Up @@ -6524,6 +6546,7 @@ mod tests {
aliases: None,
doc: None,
size: 16,
default: None,
attributes: Default::default(),
})),
});
Expand Down Expand Up @@ -6708,4 +6731,26 @@ mod tests {

Ok(())
}

#[test]
fn avro_3965_fixed_schema_with_default_bigger_than_size() -> TestResult {
match Schema::parse_str(
r#"{
"type": "fixed",
"name": "test",
"size": 1,
"default": "123456789"
}"#,
) {
Ok(_schema) => panic!("Must fail!"),
Err(err) => {
assert_eq!(
err.to_string(),
"Fixed schema's default value length (9) does not match its size (1)"
);
}
}

Ok(())
}
}
2 changes: 2 additions & 0 deletions lang/rust/avro/src/schema_compatibility.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ impl SchemaCompatibility {
aliases: _,
doc: _w_doc,
size: w_size,
default: _w_default,
attributes: _,
}) = writers_schema
{
Expand All @@ -401,6 +402,7 @@ impl SchemaCompatibility {
aliases: _,
doc: _r_doc,
size: r_size,
default: _r_default,
attributes: _,
}) = readers_schema
{
Expand Down
2 changes: 2 additions & 0 deletions lang/rust/avro/src/schema_equality.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ mod tests {
name: Name::from("fixed"),
doc: None,
size: 10,
default: None,
aliases: None,
attributes: BTreeMap::new(),
});
Expand All @@ -434,6 +435,7 @@ mod tests {
name: Name::from("fixed"),
doc: None,
size: 10,
default: None,
aliases: None,
attributes: BTreeMap::new(),
});
Expand Down
5 changes: 5 additions & 0 deletions lang/rust/avro/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,7 @@ mod tests {
name: Name::new("some_fixed").unwrap(),
aliases: None,
doc: None,
default: None,
attributes: Default::default(),
});

Expand Down Expand Up @@ -1722,6 +1723,7 @@ Field with name '"b"' is not a member of the map items"#,
aliases: None,
size: 20,
doc: None,
default: None,
attributes: Default::default(),
}))
}))
Expand Down Expand Up @@ -3036,6 +3038,7 @@ Field with name '"b"' is not a member of the map items"#,
aliases: None,
doc: None,
size: 3,
default: None,
attributes: Default::default()
}))?,
Value::Fixed(3, vec![97, 98, 99])
Expand All @@ -3048,6 +3051,7 @@ Field with name '"b"' is not a member of the map items"#,
aliases: None,
doc: None,
size: 3,
default: None,
attributes: Default::default()
}))
.is_err(),);
Expand All @@ -3059,6 +3063,7 @@ Field with name '"b"' is not a member of the map items"#,
aliases: None,
doc: None,
size: 3,
default: None,
attributes: Default::default()
}))
.is_err(),);
Expand Down
2 changes: 2 additions & 0 deletions lang/rust/avro/src/writer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,7 @@ mod tests {
aliases: None,
doc: None,
size,
default: None,
attributes: Default::default(),
});
let value = vec![0u8; size];
Expand Down Expand Up @@ -830,6 +831,7 @@ mod tests {
aliases: None,
doc: None,
size: 12,
default: None,
attributes: Default::default(),
});
let value = Value::Duration(Duration::new(
Expand Down

0 comments on commit 0661bfe

Please sign in to comment.