From 90adbf73b6472de56d13a7388c51da00844e8c1f Mon Sep 17 00:00:00 2001 From: Volker Mische Date: Mon, 2 Sep 2024 23:59:39 +0200 Subject: [PATCH] feat: introduce feature to limit integer to i64 By default the IPLD Integer kind is represented internally as an `i128`. Serde has problems with untagged enums that contain i128 types. Therefore a feature flag called `integer-max-i64` is introduced, which reduces the internal integer representation to `i64`. This flag should be used with caution as e.g. not all valid DAG-CBOR data can now be represented. Closes #19. --- Cargo.toml | 4 +++ README.md | 1 + src/arb.rs | 3 +++ src/convert.rs | 47 +++++++++++++++++++------------- src/ipld.rs | 4 +++ src/serde/de.rs | 27 ++++++++++++++++--- src/serde/ser.rs | 39 ++++++++++++++++++++++----- tests/serde_deserializer.rs | 54 ++++++++++++++++++++++--------------- tests/serde_serializer.rs | 4 ++- 9 files changed, 134 insertions(+), 49 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8d78016..d8896d0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,10 @@ serde = ["dep:serde", "dep:serde_bytes", "cid/serde"] arb = ["dep:quickcheck", "cid/arb"] # Enables support for the Codec trait, needs at least Rust 1.75 codec = [] +# Makes the internal representation of an IPLD integer an `i64` instead of the default `i128`. This +# is usefult to work around Serde limitations in regards to untagged enums that contain `i128` +# types. **Warning** enabling this feature might break compatibility with existing data. +integer-max-i64 = [] [dependencies] cid = { version = "0.11.1", default-features = false, features = ["alloc"] } diff --git a/README.md b/README.md index eeaea0c..0ab4cd1 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,7 @@ Feature flags - `codec` (enabled by default): Provides the `Codec` trait, which enables encoding and decoding independent of the IPLD Codec. The minimum supported Rust version (MSRV) can significantly be reduced to 1.64 by disabling this feature. - `serde`: Enables support for Serde serialization into/deserialization from the `Ipld` enum. - `arb`: Enables support for property based testing. + - `integer-max-i64`: The IPLD integer type is by default an `i128`. With this feature set it's an `i64`. This is useful to work around Serde limitations in regards to untagged enums that contain `i128` types. **Warning** enabling this feature might break compatibility with existing data. License diff --git a/src/arb.rs b/src/arb.rs index 17efdde..7425e17 100644 --- a/src/arb.rs +++ b/src/arb.rs @@ -36,7 +36,10 @@ impl Ipld { match index { 0 => Ipld::Null, 1 => Ipld::Bool(bool::arbitrary(g)), + #[cfg(not(feature = "integer-max-i64"))] 2 => Ipld::Integer(i128::arbitrary(g)), + #[cfg(feature = "integer-max-i64")] + 2 => Ipld::Integer(i64::arbitrary(g)), 3 => Ipld::Float(f64::arbitrary(g)), 4 => Ipld::String(String::arbitrary(g)), 5 => Ipld::Bytes(Vec::arbitrary(g)), diff --git a/src/convert.rs b/src/convert.rs index 5fc834f..62aca0a 100644 --- a/src/convert.rs +++ b/src/convert.rs @@ -217,16 +217,21 @@ mod tests { use crate::ipld::Ipld; + #[cfg(not(feature = "integer-max-i64"))] + type Integer = i128; + #[cfg(feature = "integer-max-i64")] + type Integer = i64; + #[test] #[should_panic] fn try_into_wrong_type() { - let _boolean: bool = Ipld::Integer(u8::MAX as i128).try_into().unwrap(); + let _boolean: bool = Ipld::Integer(u8::MAX as Integer).try_into().unwrap(); } #[test] #[should_panic] fn try_into_wrong_range() { - let int: u128 = Ipld::Integer(-1i128).try_into().unwrap(); + let int: u128 = Ipld::Integer(-1 as Integer).try_into().unwrap(); assert_eq!(int, u128::MIN); } @@ -241,41 +246,47 @@ mod tests { #[test] fn try_into_ints() { - let int: u8 = Ipld::Integer(u8::MAX as i128).try_into().unwrap(); + let int: u8 = Ipld::Integer(u8::MAX as Integer).try_into().unwrap(); assert_eq!(int, u8::MAX); - let int: u16 = Ipld::Integer(u16::MAX as i128).try_into().unwrap(); + let int: u16 = Ipld::Integer(u16::MAX as Integer).try_into().unwrap(); assert_eq!(int, u16::MAX); - let int: u32 = Ipld::Integer(u32::MAX as i128).try_into().unwrap(); + let int: u32 = Ipld::Integer(u32::MAX as Integer).try_into().unwrap(); assert_eq!(int, u32::MAX); - let int: u64 = Ipld::Integer(u64::MAX as i128).try_into().unwrap(); - assert_eq!(int, u64::MAX); + #[cfg(not(feature = "integer-max-i64"))] + { + let int: u64 = Ipld::Integer(u64::MAX as i128).try_into().unwrap(); + assert_eq!(int, u64::MAX); - let int: usize = Ipld::Integer(usize::MAX as i128).try_into().unwrap(); - assert_eq!(int, usize::MAX); + let int: usize = Ipld::Integer(usize::MAX as i128).try_into().unwrap(); + assert_eq!(int, usize::MAX); - let int: u128 = Ipld::Integer(i128::MAX).try_into().unwrap(); - assert_eq!(int, i128::MAX as u128); + let int: u128 = Ipld::Integer(i128::MAX).try_into().unwrap(); + assert_eq!(int, i128::MAX as u128); + } - let int: i8 = Ipld::Integer(i8::MIN as i128).try_into().unwrap(); + let int: i8 = Ipld::Integer(i8::MIN as Integer).try_into().unwrap(); assert_eq!(int, i8::MIN); - let int: i16 = Ipld::Integer(i16::MIN as i128).try_into().unwrap(); + let int: i16 = Ipld::Integer(i16::MIN as Integer).try_into().unwrap(); assert_eq!(int, i16::MIN); - let int: i32 = Ipld::Integer(i32::MIN as i128).try_into().unwrap(); + let int: i32 = Ipld::Integer(i32::MIN as Integer).try_into().unwrap(); assert_eq!(int, i32::MIN); - let int: i64 = Ipld::Integer(i64::MIN as i128).try_into().unwrap(); + let int: i64 = Ipld::Integer(i64::MIN as Integer).try_into().unwrap(); assert_eq!(int, i64::MIN); - let int: isize = Ipld::Integer(isize::MIN as i128).try_into().unwrap(); + let int: isize = Ipld::Integer(isize::MIN as Integer).try_into().unwrap(); assert_eq!(int, isize::MIN); - let int: i128 = Ipld::Integer(i128::MIN).try_into().unwrap(); - assert_eq!(int, i128::MIN); + #[cfg(not(feature = "integer-max-i64"))] + { + let int: i128 = Ipld::Integer(i128::MIN).try_into().unwrap(); + assert_eq!(int, i128::MIN); + } let int: Option = Ipld::Null.try_into().unwrap(); assert_eq!(int, Option::None) diff --git a/src/ipld.rs b/src/ipld.rs index ff0b9f7..6694986 100644 --- a/src/ipld.rs +++ b/src/ipld.rs @@ -41,7 +41,11 @@ pub enum Ipld { /// Represents a boolean value. Bool(bool), /// Represents an integer. + #[cfg(not(feature = "integer-max-i64"))] Integer(i128), + /// Represents an integer. + #[cfg(feature = "integer-max-i64")] + Integer(i64), /// Represents a floating point value. Float(f64), /// Represents an UTF-8 string. diff --git a/src/serde/de.rs b/src/serde/de.rs index cd890a6..f89e1dd 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -98,7 +98,13 @@ impl<'de> de::Deserialize<'de> for Ipld { where E: de::Error, { - Ok(Ipld::Integer(v.into())) + #[cfg(not(feature = "integer-max-i64"))] + let integer = v.into(); + #[cfg(feature = "integer-max-i64")] + let integer = v + .try_into() + .map_err(|_| de::Error::custom("integer out of i64 bounds"))?; + Ok(Ipld::Integer(integer)) } #[inline] @@ -106,15 +112,27 @@ impl<'de> de::Deserialize<'de> for Ipld { where E: de::Error, { - Ok(Ipld::Integer(v.into())) + #[cfg(not(feature = "integer-max-i64"))] + let integer = v.into(); + #[cfg(feature = "integer-max-i64")] + let integer = v; + Ok(Ipld::Integer(integer)) } #[inline] + #[cfg_attr(feature = "integer-max-i64", allow(unused_variables))] fn visit_i128(self, v: i128) -> Result where E: de::Error, { - Ok(Ipld::Integer(v)) + #[cfg(not(feature = "integer-max-i64"))] + { + Ok(Ipld::Integer(v)) + } + #[cfg(feature = "integer-max-i64")] + { + Err(de::Error::custom("integer out of i64 bounds")) + } } #[inline] @@ -266,7 +284,10 @@ impl<'de> de::Deserializer<'de> for Ipld { match self { Self::Null => visitor.visit_none(), Self::Bool(bool) => visitor.visit_bool(bool), + #[cfg(not(feature = "integer-max-i64"))] Self::Integer(i128) => visitor.visit_i128(i128), + #[cfg(feature = "integer-max-i64")] + Self::Integer(i64) => visitor.visit_i64(i64), Self::Float(f64) => visitor.visit_f64(f64), Self::String(string) => visitor.visit_str(&string), Self::Bytes(bytes) => visitor.visit_bytes(&bytes), diff --git a/src/serde/ser.rs b/src/serde/ser.rs index f31a2ea..e218938 100644 --- a/src/serde/ser.rs +++ b/src/serde/ser.rs @@ -87,7 +87,10 @@ impl ser::Serialize for Ipld { match &self { Self::Null => serializer.serialize_none(), Self::Bool(value) => serializer.serialize_bool(*value), + #[cfg(not(feature = "integer-max-i64"))] Self::Integer(value) => serializer.serialize_i128(*value), + #[cfg(feature = "integer-max-i64")] + Self::Integer(value) => serializer.serialize_i64(*value), Self::Float(value) => serializer.serialize_f64(*value), Self::String(value) => serializer.serialize_str(value), Self::Bytes(value) => serializer.serialize_bytes(value), @@ -135,31 +138,55 @@ impl serde::Serializer for Serializer { #[inline] fn serialize_i64(self, value: i64) -> Result { - self.serialize_i128(i128::from(value)) + #[cfg(not(feature = "integer-max-i64"))] + { + self.serialize_i128(i128::from(value)) + } + #[cfg(feature = "integer-max-i64")] + { + Ok(Self::Ok::Integer(value)) + } } + #[cfg_attr(feature = "integer-max-i64", allow(unused_variables))] fn serialize_i128(self, value: i128) -> Result { - Ok(Self::Ok::Integer(value)) + #[cfg(not(feature = "integer-max-i64"))] + { + Ok(Self::Ok::Integer(value)) + } + #[cfg(feature = "integer-max-i64")] + { + Err(ser::Error::custom("integer out of i64 bounds")) + } } #[inline] fn serialize_u8(self, value: u8) -> Result { - self.serialize_i128(value.into()) + self.serialize_i64(value.into()) } #[inline] fn serialize_u16(self, value: u16) -> Result { - self.serialize_i128(value.into()) + self.serialize_i64(value.into()) } #[inline] fn serialize_u32(self, value: u32) -> Result { - self.serialize_i128(value.into()) + self.serialize_i64(value.into()) } #[inline] fn serialize_u64(self, value: u64) -> Result { - self.serialize_i128(value.into()) + #[cfg(not(feature = "integer-max-i64"))] + { + self.serialize_i128(value.into()) + } + #[cfg(feature = "integer-max-i64")] + { + Ok(Self::Ok::Integer(value.try_into().map_err(|_| { + ser::Error::custom("integer out of i64 bounds") + })?)) + } } #[inline] diff --git a/tests/serde_deserializer.rs b/tests/serde_deserializer.rs index 5b04b90..d2a614e 100644 --- a/tests/serde_deserializer.rs +++ b/tests/serde_deserializer.rs @@ -12,6 +12,11 @@ use serde_json::json; use ipld_core::cid::Cid; use ipld_core::ipld::Ipld; +#[cfg(not(feature = "integer-max-i64"))] +type Integer = i128; +#[cfg(feature = "integer-max-i64")] +type Integer = i64; + /// This function is to test that all IPLD kinds except the given one errors, when trying to /// deserialize to the given Rust type. fn error_except<'de, T>(_input: T, except: &Ipld) @@ -98,9 +103,9 @@ fn ipld_deserializer_u8() { "Correctly deserialize Ipld::Integer to u8." ); - let too_large = u8::deserialize(Ipld::Integer((u8::MAX as i128) + 10)); + let too_large = u8::deserialize(Ipld::Integer((u8::MAX as Integer) + 10)); assert!(too_large.is_err(), "Number must be within range."); - let too_small = u8::deserialize(Ipld::Integer((u8::MIN as i128) - 10)); + let too_small = u8::deserialize(Ipld::Integer((u8::MIN as Integer) - 10)); assert!(too_small.is_err(), "Number must be within range."); } @@ -116,9 +121,9 @@ fn ipld_deserializer_u16() { "Correctly deserialize Ipld::Integer to u16." ); - let too_large = u16::deserialize(Ipld::Integer((u16::MAX as i128) + 10)); + let too_large = u16::deserialize(Ipld::Integer((u16::MAX as Integer) + 10)); assert!(too_large.is_err(), "Number must be within range."); - let too_small = u16::deserialize(Ipld::Integer((u16::MIN as i128) - 10)); + let too_small = u16::deserialize(Ipld::Integer((u16::MIN as Integer) - 10)); assert!(too_small.is_err(), "Number must be within range."); } @@ -134,16 +139,17 @@ fn ipld_deserializer_u32() { "Correctly deserialize Ipld::Integer to u32." ); - let too_large = u32::deserialize(Ipld::Integer((u32::MAX as i128) + 10)); + let too_large = u32::deserialize(Ipld::Integer((u32::MAX as Integer) + 10)); assert!(too_large.is_err(), "Number must be within range."); - let too_small = u32::deserialize(Ipld::Integer((u32::MIN as i128) - 10)); + let too_small = u32::deserialize(Ipld::Integer((u32::MIN as Integer) - 10)); assert!(too_small.is_err(), "Number must be within range."); } #[test] +#[allow(clippy::unnecessary_fallible_conversions)] fn ipld_deserializer_u64() { let integer = 34567890123u64; - let ipld = Ipld::Integer(integer.into()); + let ipld = Ipld::Integer(integer.try_into().unwrap()); error_except(integer, &ipld); let deserialized = u64::deserialize(ipld).unwrap(); @@ -152,10 +158,13 @@ fn ipld_deserializer_u64() { "Correctly deserialize Ipld::Integer to u64." ); - let too_large = u64::deserialize(Ipld::Integer((u64::MAX as i128) + 10)); - assert!(too_large.is_err(), "Number must be within range."); - let too_small = u64::deserialize(Ipld::Integer((u64::MIN as i128) - 10)); - assert!(too_small.is_err(), "Number must be within range."); + #[cfg(not(feature = "integer-max-i64"))] + { + let too_large = u64::deserialize(Ipld::Integer((u64::MAX as Integer) + 10)); + assert!(too_large.is_err(), "Number must be within range."); + let too_small = u64::deserialize(Ipld::Integer((u64::MIN as Integer) - 10)); + assert!(too_small.is_err(), "Number must be within range."); + } } #[test] @@ -170,9 +179,9 @@ fn ipld_deserializer_i8() { "Correctly deserialize Ipld::Integer to i8." ); - let too_large = i8::deserialize(Ipld::Integer((i8::MAX as i128) + 10)); + let too_large = i8::deserialize(Ipld::Integer((i8::MAX as Integer) + 10)); assert!(too_large.is_err(), "Number must be within range."); - let too_small = i8::deserialize(Ipld::Integer((i8::MIN as i128) - 10)); + let too_small = i8::deserialize(Ipld::Integer((i8::MIN as Integer) - 10)); assert!(too_small.is_err(), "Number must be within range."); } @@ -188,9 +197,9 @@ fn ipld_deserializer_i16() { "Correctly deserialize Ipld::Integer to i16." ); - let too_large = i16::deserialize(Ipld::Integer((i16::MAX as i128) + 10)); + let too_large = i16::deserialize(Ipld::Integer((i16::MAX as Integer) + 10)); assert!(too_large.is_err(), "Number must be within range."); - let too_small = i16::deserialize(Ipld::Integer((i16::MIN as i128) - 10)); + let too_small = i16::deserialize(Ipld::Integer((i16::MIN as Integer) - 10)); assert!(too_small.is_err(), "Number must be within range."); } @@ -206,9 +215,9 @@ fn ipld_deserializer_i32() { "Correctly deserialize Ipld::Integer to i32." ); - let too_large = i32::deserialize(Ipld::Integer((i32::MAX as i128) + 10)); + let too_large = i32::deserialize(Ipld::Integer((i32::MAX as Integer) + 10)); assert!(too_large.is_err(), "Number must be within range."); - let too_small = i32::deserialize(Ipld::Integer((i32::MIN as i128) - 10)); + let too_small = i32::deserialize(Ipld::Integer((i32::MIN as Integer) - 10)); assert!(too_small.is_err(), "Number must be within range."); } @@ -224,10 +233,13 @@ fn ipld_deserializer_i64() { "Correctly deserialize Ipld::Integer to i64." ); - let too_large = i64::deserialize(Ipld::Integer((i64::MAX as i128) + 10)); - assert!(too_large.is_err(), "Number must be within range."); - let too_small = i64::deserialize(Ipld::Integer((i64::MIN as i128) - 10)); - assert!(too_small.is_err(), "Number must be within range."); + #[cfg(not(feature = "integer-max-i64"))] + { + let too_large = i64::deserialize(Ipld::Integer((i64::MAX as i128) + 10)); + assert!(too_large.is_err(), "Number must be within range."); + let too_small = i64::deserialize(Ipld::Integer((i64::MIN as i128) - 10)); + assert!(too_small.is_err(), "Number must be within range."); + } } #[test] diff --git a/tests/serde_serializer.rs b/tests/serde_serializer.rs index 56952c4..308b0fa 100644 --- a/tests/serde_serializer.rs +++ b/tests/serde_serializer.rs @@ -68,9 +68,10 @@ fn ipld_serializer_u32() { } #[test] +#[allow(clippy::unnecessary_fallible_conversions)] fn ipld_serializer_u64() { let integer = 34567890123u64; - let ipld = Ipld::Integer(integer.into()); + let ipld = Ipld::Integer(integer.try_into().unwrap()); assert_serialized(integer, ipld); } @@ -103,6 +104,7 @@ fn ipld_serializer_i64() { } #[test] +#[cfg(not(feature = "integer-max-i64"))] fn ipld_serializer_i128() { let integer = 34567890123467890123i128; let ipld = Ipld::Integer(integer);