Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Lenient bytes deserialization #2036

Merged
merged 3 commits into from
Sep 26, 2016
Merged

Lenient bytes deserialization #2036

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1906c8b
Lenient bytes deserialization
Sep 1, 2016
cc7456d
Merge branch 'master' into lenient-data
tomusdrw Sep 26, 2016
55b804a
Printing warning
tomusdrw Sep 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions rpc/src/v1/types/bytes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,13 @@ impl Visitor for BytesVisitor {
type Value = Bytes;

fn visit_str<E>(&mut self, value: &str) -> Result<Self::Value, E> where E: Error {
if value.len() >= 2 && &value[0..2] == "0x" && value.len() & 1 == 0 {
if value.is_empty() {
warn!(
target: "deprecated",
"Deserializing empty string as empty bytes. This is a non-standard behaviour that will be removed in future versions. Please update your code to send `0x` instead!"
);
Ok(Bytes::new(Vec::new()))
} else if value.len() >= 2 && &value[0..2] == "0x" && value.len() & 1 == 0 {
Ok(Bytes::new(FromHex::from_hex(&value[2..]).unwrap_or_else(|_| vec![])))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated

unwrap_or_else(|_| vec![]) part is really interesting here. Is it really what geth is doing? It should be an error imo

} else {
Err(Error::custom("invalid hex"))
Expand Down Expand Up @@ -98,18 +104,26 @@ mod tests {

#[test]
fn test_bytes_deserialize() {
let bytes1: Result<Bytes, serde_json::Error> = serde_json::from_str(r#""""#);
// TODO [ToDr] Uncomment when Mist starts sending correct data
// let bytes1: Result<Bytes, serde_json::Error> = serde_json::from_str(r#""""#);
let bytes2: Result<Bytes, serde_json::Error> = serde_json::from_str(r#""0x123""#);

let bytes3: Bytes = serde_json::from_str(r#""0x""#).unwrap();
let bytes4: Bytes = serde_json::from_str(r#""0x12""#).unwrap();
let bytes5: Bytes = serde_json::from_str(r#""0x0123""#).unwrap();

assert!(bytes1.is_err());
// assert!(bytes1.is_err());
assert!(bytes2.is_err());
assert_eq!(bytes3, Bytes(vec![]));
assert_eq!(bytes4, Bytes(vec![0x12]));
assert_eq!(bytes5, Bytes(vec![0x1, 0x23]));
}

// TODO [ToDr] Remove when Mist starts sending correct data
#[test]
fn test_bytes_lenient_against_the_spec_deserialize_for_empty_string_for_mist_compatibility() {
let deserialized: Bytes = serde_json::from_str(r#""""#).unwrap();
assert_eq!(deserialized, Bytes(Vec::new()));
}
}