feat(primitive-types, serde): deserialize hex strings without 0x pref…

…ix (#598)

* feat(primitive-types): deserialize hex strings without 0x prefix

* change both to stripped

* Revert "change both to stripped"

This reverts commit 9d8d532.

* accept only stripped to from_hex_raw

* cargo fmt

* fix tests

* oneliner for (v, stripped)

* bump ethereum-types impl-serde version

* remove unused lifetime

* update changelog

* allow deprecate

* fix changelog date format (god bless america)

ethereum-types/Cargo.toml

@@ -12,7 +12,7 @@ ethbloom = { path = "../ethbloom", version = "0.11", default-features = false }
 fixed-hash = { path = "../fixed-hash", version = "0.7", default-features = false, features = ["byteorder", "rustc-hex"] }
 uint-crate = { path = "../uint", package = "uint", version = "0.9", default-features = false }
 primitive-types = { path = "../primitive-types", version = "0.10", features = ["byteorder", "rustc-hex"], default-features = false }
-impl-serde = { path = "../primitive-types/impls/serde", version = "0.3.0", default-features = false, optional = true }
+impl-serde = { path = "../primitive-types/impls/serde", version = "0.3.2", default-features = false, optional = true }
 impl-rlp = { path = "../primitive-types/impls/rlp", version = "0.3", default-features = false, optional = true }
 impl-codec = { version = "0.5.0", path = "../primitive-types/impls/codec", default-features = false, optional = true }
 scale-info = { version = "1.0", features = ["derive"], default-features = false, optional = true }

ethereum-types/src/uint.rs

@@ -58,12 +58,29 @@ mod tests {
 					assert_eq!(number, ser::from_str(&format!("{:?}", expected)).unwrap());
 				}
 
+				let tests = vec![
+					($name::from(0), "0"),
+					($name::from(1), "1"),
+					($name::from(2), "2"),
+					($name::from(10), "a"),
+					($name::from(15), "f"),
+					($name::from(15), "f"),
+					($name::from(16), "10"),
+					($name::from(1_000), "3e8"),
+					($name::from(100_000), "186a0"),
+					($name::from(u64::max_value()), "ffffffffffffffff"),
+					($name::from(u64::max_value()) + 1, "10000000000000000"),
+				];
+
+				for (number, expected) in tests {
+					assert_eq!(format!("{:?}", "0x".to_string() + expected), ser::to_string_pretty(&number).unwrap());
+					assert_eq!(number, ser::from_str(&format!("{:?}", expected)).unwrap());
+				}
+
 				// Invalid examples
 				assert!(ser::from_str::<$name>("\"0x\"").unwrap_err().is_data());
 				assert!(ser::from_str::<$name>("\"0xg\"").unwrap_err().is_data());
 				assert!(ser::from_str::<$name>("\"\"").unwrap_err().is_data());
-				assert!(ser::from_str::<$name>("\"10\"").unwrap_err().is_data());
-				assert!(ser::from_str::<$name>("\"0\"").unwrap_err().is_data());
 			}
 		};
 	}

ethereum-types/tests/serde.rs

@@ -32,12 +32,29 @@ macro_rules! test {
 				assert_eq!(number, ser::from_str(&format!("{:?}", expected)).unwrap());
 			}
 
+			let tests = vec![
+				($name::from(0), "0"),
+				($name::from(1), "1"),
+				($name::from(2), "2"),
+				($name::from(10), "a"),
+				($name::from(15), "f"),
+				($name::from(15), "f"),
+				($name::from(16), "10"),
+				($name::from(1_000), "3e8"),
+				($name::from(100_000), "186a0"),
+				($name::from(u64::max_value()), "ffffffffffffffff"),
+				($name::from(u64::max_value()) + $name::from(1u64), "10000000000000000"),
+			];
+
+			for (number, expected) in tests {
+				assert_eq!(format!("{:?}", "0x".to_string() + expected), ser::to_string_pretty(&number).unwrap());
+				assert_eq!(number, ser::from_str(&format!("{:?}", expected)).unwrap());
+			}
+
 			// Invalid examples
 			assert!(ser::from_str::<$name>("\"0x\"").unwrap_err().is_data());
 			assert!(ser::from_str::<$name>("\"0xg\"").unwrap_err().is_data());
 			assert!(ser::from_str::<$name>("\"\"").unwrap_err().is_data());
-			assert!(ser::from_str::<$name>("\"10\"").unwrap_err().is_data());
-			assert!(ser::from_str::<$name>("\"0\"").unwrap_err().is_data());
 		}
 	};
 }
@@ -109,8 +126,15 @@ fn test_invalid() {
 }
 
 #[test]
-fn test_invalid_char() {
+fn test_invalid_char_with_prefix() {
 	const INVALID_STR: &str = "\"0x000000000000000000000000000000000000000000000000000000000000000g\"";
 	const EXPECTED_MSG: &str = "invalid hex character: g, at 65 at line 1 column 68";
 	assert_eq!(ser::from_str::<H256>(INVALID_STR).unwrap_err().to_string(), EXPECTED_MSG);
 }
+
+#[test]
+fn test_invalid_char_without_prefix() {
+	const INVALID_STR: &str = "\"000000000000000000000000000000000000000000000000000000000000000g\"";
+	const EXPECTED_MSG: &str = "invalid hex character: g, at 63 at line 1 column 66";
+	assert_eq!(ser::from_str::<H256>(INVALID_STR).unwrap_err().to_string(), EXPECTED_MSG);
+}

parity-util-mem/derive/lib.rs

@@ -30,7 +30,7 @@ fn malloc_size_of_derive(s: synstructure::Structure) -> proc_macro2::TokenStream
 					"#[ignore_malloc_size_of] should have an explanation, \
 					 e.g. #[ignore_malloc_size_of = \"because reasons\"]"
 				);
-			}
+			},
 			syn::Meta::NameValue(syn::MetaNameValue { ref path, .. }) if path.is_ident("ignore_malloc_size_of") => true,
 			_ => false,
 		});

primitive-types/impls/serde/CHANGELOG.md

@@ -6,6 +6,9 @@ The format is based on [Keep a Changelog].
 
 ## [Unreleased]
 
+## [0.3.2] - 2021-11-10
+- Supported decoding of hex strings without `0x` prefix. [#598](https://github.com/paritytech/parity-common/pull/598)
+
 ## [0.3.1] - 2020-05-05
 - Added `no_std` support. [#385](https://github.com/paritytech/parity-common/pull/385)
 

primitive-types/impls/serde/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "impl-serde"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Parity Technologies <[email protected]>"]
 edition = "2018"
 license = "MIT OR Apache-2.0"

primitive-types/impls/serde/src/serialize.rs

@@ -66,6 +66,7 @@ fn to_hex_raw<'a>(v: &'a mut [u8], bytes: &[u8], skip_leading_zero: bool) -> &'a
 #[derive(Debug, PartialEq, Eq)]
 pub enum FromHexError {
 	/// The `0x` prefix is missing.
+	#[deprecated(since = "0.3.2", note = "We support non 0x-prefixed hex strings")]
 	MissingPrefix,
 	/// Invalid (non-hex) character encountered.
 	InvalidHex {
@@ -82,38 +83,34 @@ impl std::error::Error for FromHexError {}
 impl fmt::Display for FromHexError {
 	fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
 		match *self {
+			#[allow(deprecated)]
 			Self::MissingPrefix => write!(fmt, "0x prefix is missing"),
 			Self::InvalidHex { character, index } => write!(fmt, "invalid hex character: {}, at {}", character, index),
 		}
 	}
 }
 
-/// Decode given hex string into a vector of bytes.
+/// Decode given (both 0x-prefixed or not) hex string into a vector of bytes.
 ///
-/// Returns an error if the string is not prefixed with `0x`
-/// or non-hex characters are present.
+/// Returns an error if non-hex characters are present.
 pub fn from_hex(v: &str) -> Result<Vec<u8>, FromHexError> {
-	if !v.starts_with("0x") {
-		return Err(FromHexError::MissingPrefix)
-	}
+	let (v, stripped) = v.strip_prefix("0x").map_or((v, false), |v| (v, true));
 
-	let mut bytes = vec![0u8; (v.len() - 1) / 2];
-	from_hex_raw(v, &mut bytes)?;
+	let mut bytes = vec![0u8; (v.len() + 1) / 2];
+	from_hex_raw(v, &mut bytes, stripped)?;
 	Ok(bytes)
 }
 
-/// Decode given 0x-prefixed hex string into provided slice.
+/// Decode given 0x-prefix-stripped hex string into provided slice.
 /// Used internally by `from_hex` and `deserialize_check_len`.
 ///
-/// The method will panic if:
-/// 1. `v` is shorter than 2 characters (you need to check 0x prefix outside).
-/// 2. `bytes` have incorrect length (make sure to allocate enough beforehand).
-fn from_hex_raw<'a>(v: &str, bytes: &mut [u8]) -> Result<usize, FromHexError> {
-	let bytes_len = v.len() - 2;
+/// The method will panic if `bytes` have incorrect length (make sure to allocate enough beforehand).
+fn from_hex_raw(v: &str, bytes: &mut [u8], stripped: bool) -> Result<usize, FromHexError> {
+	let bytes_len = v.len();
 	let mut modulus = bytes_len % 2;
 	let mut buf = 0;
 	let mut pos = 0;
-	for (index, byte) in v.bytes().enumerate().skip(2) {
+	for (index, byte) in v.bytes().enumerate() {
 		buf <<= 4;
 
 		match byte {
@@ -126,7 +123,7 @@ fn from_hex_raw<'a>(v: &str, bytes: &mut [u8]) -> Result<usize, FromHexError> {
 			},
 			b => {
 				let character = char::from(b);
-				return Err(FromHexError::InvalidHex { character, index })
+				return Err(FromHexError::InvalidHex { character, index: index + if stripped { 2 } else { 0 } })
 			},
 		}
 
@@ -208,7 +205,7 @@ where
 		type Value = Vec<u8>;
 
 		fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
-			write!(formatter, "a 0x-prefixed hex string")
+			write!(formatter, "a (both 0x-prefixed or not) hex string")
 		}
 
 		fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
@@ -237,30 +234,28 @@ where
 		type Value = usize;
 
 		fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
-			write!(formatter, "a 0x-prefixed hex string with {}", self.len)
+			write!(formatter, "a (both 0x-prefixed or not) hex string with {}", self.len)
 		}
 
 		fn visit_str<E: de::Error>(self, v: &str) -> Result<Self::Value, E> {
-			if !v.starts_with("0x") {
-				return Err(E::custom(FromHexError::MissingPrefix))
-			}
+			let (v, stripped) = v.strip_prefix("0x").map_or((v, false), |v| (v, true));
 
 			let len = v.len();
 			let is_len_valid = match self.len {
-				ExpectedLen::Exact(ref slice) => len == 2 * slice.len() + 2,
-				ExpectedLen::Between(min, ref slice) => len <= 2 * slice.len() + 2 && len > 2 * min + 2,
+				ExpectedLen::Exact(ref slice) => len == 2 * slice.len(),
+				ExpectedLen::Between(min, ref slice) => len <= 2 * slice.len() && len > 2 * min,
 			};
 
 			if !is_len_valid {
-				return Err(E::invalid_length(v.len() - 2, &self))
+				return Err(E::invalid_length(v.len(), &self))
 			}
 
 			let bytes = match self.len {
 				ExpectedLen::Exact(slice) => slice,
 				ExpectedLen::Between(_, slice) => slice,
 			};
 
-			from_hex_raw(v, bytes).map_err(E::custom)
+			from_hex_raw(v, bytes, stripped).map_err(E::custom)
 		}
 
 		fn visit_string<E: de::Error>(self, v: String) -> Result<Self::Value, E> {
@@ -280,7 +275,7 @@ mod tests {
 	struct Bytes(#[serde(with = "super")] Vec<u8>);
 
 	#[test]
-	fn should_not_fail_on_short_string() {
+	fn should_not_fail_on_short_string_with_prefix() {
 		let a: Bytes = serde_json::from_str("\"0x\"").unwrap();
 		let b: Bytes = serde_json::from_str("\"0x1\"").unwrap();
 		let c: Bytes = serde_json::from_str("\"0x12\"").unwrap();
@@ -297,7 +292,7 @@ mod tests {
 	}
 
 	#[test]
-	fn should_not_fail_on_other_strings() {
+	fn should_not_fail_on_other_strings_with_prefix() {
 		let a: Bytes =
 			serde_json::from_str("\"0x7f864e18e3dd8b58386310d2fe0919eef27c6e558564b7f67f22d99d20f587\"").unwrap();
 		let b: Bytes =
@@ -310,6 +305,37 @@ mod tests {
 		assert_eq!(c.0.len(), 32);
 	}
 
+	#[test]
+	fn should_not_fail_on_short_string_without_prefix() {
+		let a: Bytes = serde_json::from_str("\"\"").unwrap();
+		let b: Bytes = serde_json::from_str("\"1\"").unwrap();
+		let c: Bytes = serde_json::from_str("\"12\"").unwrap();
+		let d: Bytes = serde_json::from_str("\"123\"").unwrap();
+		let e: Bytes = serde_json::from_str("\"1234\"").unwrap();
+		let f: Bytes = serde_json::from_str("\"12345\"").unwrap();
+
+		assert!(a.0.is_empty());
+		assert_eq!(b.0, vec![1]);
+		assert_eq!(c.0, vec![0x12]);
+		assert_eq!(d.0, vec![0x1, 0x23]);
+		assert_eq!(e.0, vec![0x12, 0x34]);
+		assert_eq!(f.0, vec![0x1, 0x23, 0x45]);
+	}
+
+	#[test]
+	fn should_not_fail_on_other_strings_without_prefix() {
+		let a: Bytes =
+			serde_json::from_str("\"7f864e18e3dd8b58386310d2fe0919eef27c6e558564b7f67f22d99d20f587\"").unwrap();
+		let b: Bytes =
+			serde_json::from_str("\"7f864e18e3dd8b58386310d2fe0919eef27c6e558564b7f67f22d99d20f587b\"").unwrap();
+		let c: Bytes =
+			serde_json::from_str("\"7f864e18e3dd8b58386310d2fe0919eef27c6e558564b7f67f22d99d20f587b4\"").unwrap();
+
+		assert_eq!(a.0.len(), 31);
+		assert_eq!(b.0.len(), 32);
+		assert_eq!(c.0.len(), 32);
+	}
+
 	#[test]
 	fn should_serialize_and_deserialize_empty_bytes() {
 		let bytes = Bytes(Vec::new());
@@ -323,7 +349,7 @@ mod tests {
 	}
 
 	#[test]
-	fn should_encode_to_and_from_hex() {
+	fn should_encode_to_and_from_hex_with_prefix() {
 		assert_eq!(to_hex(&[0, 1, 2], true), "0x102");
 		assert_eq!(to_hex(&[0, 1, 2], false), "0x000102");
 		assert_eq!(to_hex(&[0], true), "0x0");
@@ -334,4 +360,11 @@ mod tests {
 		assert_eq!(from_hex("0x102"), Ok(vec![1, 2]));
 		assert_eq!(from_hex("0xf"), Ok(vec![0xf]));
 	}
+
+	#[test]
+	fn should_decode_hex_without_prefix() {
+		assert_eq!(from_hex("0102"), Ok(vec![1, 2]));
+		assert_eq!(from_hex("102"), Ok(vec![1, 2]));
+		assert_eq!(from_hex("f"), Ok(vec![0xf]));
+	}
 }