fix(dyn-abi): correctly parse empty lists of bytes (#548)

* fix(dyn-abi): correctly parse empty lists of bytes * test: bless tests * chore: clippy * chore: use FromHex
alloy-rs · Feb 29, 2024 · 7e39882 · 7e39882
1 parent e1c33ab
commit 7e39882
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 35 deletions.
diff --git a/crates/dyn-abi/src/coerce.rs b/crates/dyn-abi/src/coerce.rs
@@ -255,6 +255,7 @@ enum Error {
     TooManyDecimals(usize, usize),
     InvalidFixedBytesLength(usize),
     FixedArrayLengthMismatch(usize, usize),
+    EmptyHexStringWithoutPrefix,
 }
 
 #[cfg(feature = "std")]
@@ -284,6 +285,7 @@ impl fmt::Display for Error {
                 f,
                 "fixed array length mismatch: expected {expected} elements, got {actual}"
             ),
+            Self::EmptyHexStringWithoutPrefix => f.write_str("expected hex digits or the `0x` prefix for an empty hex string"),
         }
     }
 }
@@ -474,27 +476,35 @@ fn fixed_bytes<'i>(len: usize) -> impl Parser<&'i str, Word, ContextError> {
 
 #[inline]
 fn address(input: &mut &str) -> PResult<Address> {
-    trace("address", fixed_bytes_inner).parse_next(input).map(Address::from)
+    trace("address", hex_str.try_map(hex::FromHex::from_hex)).parse_next(input)
 }
 
 #[inline]
 fn function(input: &mut &str) -> PResult<Function> {
-    trace("function", fixed_bytes_inner).parse_next(input).map(Function::from)
+    trace("function", hex_str.try_map(hex::FromHex::from_hex)).parse_next(input)
 }
 
 #[inline]
 fn bytes(input: &mut &str) -> PResult<Vec<u8>> {
     trace("bytes", hex_str.try_map(hex::decode)).parse_next(input)
 }
 
-#[inline]
-fn fixed_bytes_inner<const N: usize>(input: &mut &str) -> PResult<FixedBytes<N>> {
-    hex_str.try_map(|s| hex::decode_to_array(s).map(Into::into)).parse_next(input)
-}
-
 #[inline]
 fn hex_str<'i>(input: &mut &'i str) -> PResult<&'i str> {
-    trace("hex_str", preceded(opt("0x"), hex_digit0)).parse_next(input)
+    trace("hex_str", |input: &mut &'i str| {
+        // Allow empty `bytes` only with a prefix.
+        let has_prefix = opt("0x").parse_next(input)?.is_some();
+        let s = hex_digit0(input)?;
+        if !has_prefix && s.is_empty() {
+            return Err(ErrMode::from_external_error(
+                input,
+                ErrorKind::Verify,
+                Error::EmptyHexStringWithoutPrefix,
+            ));
+        }
+        Ok(s)
+    })
+    .parse_next(input)
 }
 
 fn hex_error(input: &&str, e: FromHexError) -> ErrMode<ContextError> {
@@ -849,7 +859,7 @@ mod tests {
         );
 
         let e = DynSolType::FixedBytes(1).coerce_str("").unwrap_err();
-        assert_error_contains(&e, "Invalid string length");
+        assert_error_contains(&e, &Error::EmptyHexStringWithoutPrefix.to_string());
         let e = DynSolType::FixedBytes(1).coerce_str("0").unwrap_err();
         assert_error_contains(&e, "Odd number of digits");
         let e = DynSolType::FixedBytes(1).coerce_str("0x").unwrap_err();
@@ -919,7 +929,9 @@ mod tests {
 
     #[test]
     fn coerce_bytes() {
-        assert_eq!(DynSolType::Bytes.coerce_str("").unwrap(), DynSolValue::Bytes(vec![]));
+        let e = DynSolType::Bytes.coerce_str("").unwrap_err();
+        assert_error_contains(&e, &Error::EmptyHexStringWithoutPrefix.to_string());
+
         assert_eq!(DynSolType::Bytes.coerce_str("0x").unwrap(), DynSolValue::Bytes(vec![]));
         assert!(DynSolType::Bytes.coerce_str("0x0").is_err());
         assert!(DynSolType::Bytes.coerce_str("0").is_err());
@@ -1038,6 +1050,24 @@ mod tests {
         assert_eq!(arr.coerce_str("[\"\", \"\"]").unwrap(), mk_arr(&["", ""]));
     }
 
+    #[test]
+    fn coerce_array_of_bytes_and_strings() {
+        let ty = DynSolType::Array(Box::new(DynSolType::Bytes));
+        assert_eq!(ty.coerce_str("[]"), Ok(DynSolValue::Array(vec![])));
+        assert_eq!(ty.coerce_str("[0x]"), Ok(DynSolValue::Array(vec![DynSolValue::Bytes(vec![])])));
+
+        let ty = DynSolType::Array(Box::new(DynSolType::String));
+        assert_eq!(ty.coerce_str("[]"), Ok(DynSolValue::Array(vec![])));
+        assert_eq!(
+            ty.coerce_str("[\"\"]"),
+            Ok(DynSolValue::Array(vec![DynSolValue::String(String::new())]))
+        );
+        assert_eq!(
+            ty.coerce_str("[0x]"),
+            Ok(DynSolValue::Array(vec![DynSolValue::String("0x".into())]))
+        );
+    }
+
     #[test]
     fn coerce_empty_array() {
         assert_eq!(

diff --git a/crates/json-abi/src/utils.rs b/crates/json-abi/src/utils.rs
@@ -427,8 +427,8 @@ mod tests {
     fn test_item_parse() {
         assert_eq!(parse_sig::<true>("foo()"), Ok(("foo".into(), vec![], vec![], false)));
         assert_eq!(parse_sig::<true>("foo()()"), Ok(("foo".into(), vec![], vec![], false)));
-        assert_eq!(parse_sig::<true>("foo(,) \t ()"), Ok(("foo".into(), vec![], vec![], false)));
-        assert_eq!(parse_sig::<true>("foo(,)  (,)"), Ok(("foo".into(), vec![], vec![], false)));
+        assert_eq!(parse_sig::<true>("foo() \t ()"), Ok(("foo".into(), vec![], vec![], false)));
+        assert_eq!(parse_sig::<true>("foo()  ()"), Ok(("foo".into(), vec![], vec![], false)));
 
         assert_eq!(parse_sig::<false>("foo()"), Ok(("foo".into(), vec![], vec![], false)));
         parse_sig::<false>("foo()()").unwrap_err();

diff --git a/crates/primitives/src/bits/fixed.rs b/crates/primitives/src/bits/fixed.rs
@@ -1,6 +1,7 @@
 use crate::aliases;
 use core::{fmt, iter, ops, str};
 use derive_more::{Deref, DerefMut, From, Index, IndexMut, IntoIterator};
+use hex::FromHex;
 
 /// A byte array of fixed length (`[u8; N]`).
 ///
@@ -307,9 +308,7 @@ impl<const N: usize> str::FromStr for FixedBytes<N> {
 
     #[inline]
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let mut buf = [0u8; N];
-        hex::decode_to_slice(s, &mut buf)?;
-        Ok(Self(buf))
+        Self::from_hex(s)
     }
 }
 

diff --git a/crates/primitives/src/bits/macros.rs b/crates/primitives/src/bits/macros.rs
@@ -414,13 +414,8 @@ macro_rules! impl_fb_traits {
             type Error = $crate::hex::FromHexError;
 
             #[inline]
-            fn from_hex<T>(hex: T) -> Result<Self, Self::Error>
-            where
-                T: $crate::private::AsRef<[u8]>
-            {
-                let mut buf = [0u8; $n];
-                $crate::hex::decode_to_slice(hex.as_ref(), &mut buf)?;
-                Ok(Self::new(buf))
+            fn from_hex<T: $crate::private::AsRef<[u8]>>(hex: T) -> Result<Self, Self::Error> {
+                $crate::hex::decode_to_array(hex).map(Self::new)
             }
         }
     };

diff --git a/crates/sol-type-parser/src/parameter.rs b/crates/sol-type-parser/src/parameter.rs
@@ -206,9 +206,9 @@ mod tests {
     #[test]
     fn parse_params() {
         assert_eq!(Parameters::parse("()"), Ok(Parameters { span: "()", params: vec![] }));
-        assert_eq!(Parameters::parse("(,)"), Ok(Parameters { span: "(,)", params: vec![] }));
-        assert_eq!(Parameters::parse("(, )"), Ok(Parameters { span: "(, )", params: vec![] }));
-        assert_eq!(Parameters::parse("( , )"), Ok(Parameters { span: "( , )", params: vec![] }));
+        assert_eq!(Parameters::parse("( )"), Ok(Parameters { span: "( )", params: vec![] }));
+        assert_eq!(Parameters::parse("(  )"), Ok(Parameters { span: "(  )", params: vec![] }));
+        assert_eq!(Parameters::parse("(   )"), Ok(Parameters { span: "(   )", params: vec![] }));
 
         assert_eq!(
             Parameters::parse("(\tuint256   , \t)"),

diff --git a/crates/sol-type-parser/src/utils.rs b/crates/sol-type-parser/src/utils.rs
@@ -5,7 +5,7 @@ use alloc::{string::String, vec::Vec};
 use core::{slice, str};
 use winnow::{
     ascii::space0,
-    combinator::{cut_err, delimited, opt, preceded, separated, terminated, trace},
+    combinator::{cut_err, opt, preceded, separated, terminated, trace},
     error::{AddContext, ParserError, StrContext, StrContextValue},
     stream::Accumulate,
     PResult, Parser,
@@ -88,14 +88,19 @@ where
     let name = format!("list({open:?}, {delim:?}, {close:?})");
     #[cfg(not(feature = "debug"))]
     let name = "list";
-    trace(
-        name,
-        delimited(
-            (char_parser(open), space0),
-            separated(0.., f, (char_parser(delim), space0)),
-            (opt(delim), space0, cut_err(char_parser(close))),
-        ),
-    )
+
+    // These have to be outside of the closure for some reason.
+    let elems_1 = separated(1.., f, (char_parser(delim), space0));
+    let mut elems_and_end = terminated(elems_1, (opt(delim), space0, cut_err(char_parser(close))));
+    trace(name, move |input: &mut &'i str| {
+        let _ = char_parser(open).parse_next(input)?;
+        let _ = space0(input)?;
+        if let Some(stripped) = input.strip_prefix(close) {
+            *input = stripped;
+            return Ok(O2::initial(Some(0)));
+        }
+        elems_and_end.parse_next(input)
+    })
 }
 
 pub fn opt_ws_ident<'a>(input: &mut &'a str) -> PResult<Option<&'a str>> {

diff --git a/crates/sol-types/src/abi/encoder.rs b/crates/sol-types/src/abi/encoder.rs
@@ -706,7 +706,7 @@ mod tests {
     	"
         );
 
-        let data = (5, bytes.clone(), 3, bytes);
+        let data = (5, bytes, 3, bytes);
 
         let encoded = MyTy::abi_encode(&data);
         let encoded_params = MyTy::abi_encode_params(&data);

diff --git a/scripts/bless_tests.sh b/scripts/bless_tests.sh
@@ -0,0 +1,8 @@
+#!/usr/bin/env bash
+
+set -eo pipefail
+
+export RUSTFLAGS="-Zthreads=1"
+export TRYBUILD=overwrite
+cargo +nightly test -p alloy-sol-types --test compiletest
+cargo +nightly test -p alloy-sol-types --test compiletest --all-features