From 46625764e5a37acab6c416013ab1dd9d9b4aa57a Mon Sep 17 00:00:00 2001 From: Andrew Plaza Date: Thu, 21 Nov 2019 12:36:04 +0100 Subject: [PATCH 1/6] workaround serde deserializing incorrectly serde deserializes incorrectly when arbitrary precision is enabled. Fix by first deserializing into `Value` before `ClientResponse` --- core-client/Cargo.toml | 1 + core-client/transports/Cargo.toml | 1 + core-client/transports/src/transports/mod.rs | 31 +++++++++++++------- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/core-client/Cargo.toml b/core-client/Cargo.toml index 7e5c942a2..c74a8c14b 100644 --- a/core-client/Cargo.toml +++ b/core-client/Cargo.toml @@ -23,6 +23,7 @@ tls = ["jsonrpc-client-transports/tls"] http = ["jsonrpc-client-transports/http"] ws = ["jsonrpc-client-transports/ws"] ipc = ["jsonrpc-client-transports/ipc"] +arbitrary_precision = ["jsonrpc-client-transports/arbitrary_precision"] [dependencies] jsonrpc-client-transports = { version = "14.0", path = "./transports", default-features = false } diff --git a/core-client/transports/Cargo.toml b/core-client/transports/Cargo.toml index 50adbb2ac..3ad799370 100644 --- a/core-client/transports/Cargo.toml +++ b/core-client/transports/Cargo.toml @@ -31,6 +31,7 @@ ipc = [ "jsonrpc-server-utils", "tokio", ] +arbitrary_precision = [] [dependencies] failure = "0.1" diff --git a/core-client/transports/src/transports/mod.rs b/core-client/transports/src/transports/mod.rs index 3d5991c48..78f7370e2 100644 --- a/core-client/transports/src/transports/mod.rs +++ b/core-client/transports/src/transports/mod.rs @@ -81,16 +81,27 @@ impl RequestBuilder { pub fn parse_response( response: &str, ) -> Result<(Id, Result, Option, Option), RpcError> { - serde_json::from_str::(&response) - .map_err(|e| RpcError::ParseError(e.to_string(), e.into())) - .map(|response| { - let id = response.id().unwrap_or(Id::Null); - let sid = response.subscription_id(); - let method = response.method(); - let value: Result = response.into(); - let result = value.map_err(RpcError::JsonRpcError); - (id, result, method, sid) - }) + // https://github.com/serde-rs/json/issues/505 + // Arbitrary precision confuses serde when deserializing into untagged enums, + // this is a workaround + let response = if cfg!(feature = "arbitrary_precision") { + let value = serde_json::from_str::(&response) + .map_err(|e| RpcError::ParseError(e.to_string(), e.into()))?; + serde_json::from_value::(value) + .map_err(|e| RpcError::ParseError(e.to_string(), e.into())) + } else { + serde_json::from_str::(&response) + .map_err(|e| RpcError::ParseError(e.to_string(), e.into())) + }; + + response.map(|response| { + let id = response.id().unwrap_or(Id::Null); + let sid = response.subscription_id(); + let method = response.method(); + let value: Result = response.into(); + let result = value.map_err(RpcError::JsonRpcError); + (id, result, method, sid) + }) } /// A type representing all possible values sent from the server to the client. From 27c1ce14965c73ab171bd712af4596bf9ad5c44e Mon Sep 17 00:00:00 2001 From: Andrew Plaza Date: Thu, 21 Nov 2019 12:59:07 +0100 Subject: [PATCH 2/6] rustfmt format for stylecheck --- core-client/transports/src/transports/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/core-client/transports/src/transports/mod.rs b/core-client/transports/src/transports/mod.rs index 78f7370e2..12551c0f4 100644 --- a/core-client/transports/src/transports/mod.rs +++ b/core-client/transports/src/transports/mod.rs @@ -85,13 +85,11 @@ pub fn parse_response( // Arbitrary precision confuses serde when deserializing into untagged enums, // this is a workaround let response = if cfg!(feature = "arbitrary_precision") { - let value = serde_json::from_str::(&response) - .map_err(|e| RpcError::ParseError(e.to_string(), e.into()))?; - serde_json::from_value::(value) - .map_err(|e| RpcError::ParseError(e.to_string(), e.into())) + let value = + serde_json::from_str::(&response).map_err(|e| RpcError::ParseError(e.to_string(), e.into()))?; + serde_json::from_value::(value).map_err(|e| RpcError::ParseError(e.to_string(), e.into())) } else { - serde_json::from_str::(&response) - .map_err(|e| RpcError::ParseError(e.to_string(), e.into())) + serde_json::from_str::(&response).map_err(|e| RpcError::ParseError(e.to_string(), e.into())) }; response.map(|response| { From 9e71d2634f021406eb09415f770335767d66ea73 Mon Sep 17 00:00:00 2001 From: Andrew Plaza Date: Thu, 21 Nov 2019 18:48:56 +0100 Subject: [PATCH 3/6] fix tests when using feature `arbitrary_precision` add cfg! to other places JSON is being deserialized --- core-client/transports/Cargo.toml | 2 +- core-client/transports/src/transports/mod.rs | 17 ++++++++++++++--- core/Cargo.toml | 3 +++ core/src/io.rs | 9 +++++++-- core/src/types/response.rs | 7 ++++++- test/Cargo.toml | 3 +++ test/src/lib.rs | 10 +++++++++- 7 files changed, 43 insertions(+), 8 deletions(-) diff --git a/core-client/transports/Cargo.toml b/core-client/transports/Cargo.toml index 3ad799370..3ee0072e5 100644 --- a/core-client/transports/Cargo.toml +++ b/core-client/transports/Cargo.toml @@ -31,7 +31,7 @@ ipc = [ "jsonrpc-server-utils", "tokio", ] -arbitrary_precision = [] +arbitrary_precision = ["serde_json/arbitrary_precision", "jsonrpc-core/arbitrary_precision"] [dependencies] failure = "0.1" diff --git a/core-client/transports/src/transports/mod.rs b/core-client/transports/src/transports/mod.rs index 12551c0f4..8953b70dc 100644 --- a/core-client/transports/src/transports/mod.rs +++ b/core-client/transports/src/transports/mod.rs @@ -178,10 +178,20 @@ mod tests { use jsonrpc_core::{Failure, Notification, Output, Params, Success, Value, Version}; use serde_json; + fn arbitrary_precision_fix(input: &str) -> ClientResponse { + if cfg!(feature = "arbitrary_precision") { + let val: serde_json::Value = serde_json::from_str(input).unwrap(); + serde_json::from_value(val).unwrap() + } else { + serde_json::from_str(input).unwrap() + } + } + #[test] fn notification_deserialize() { let dsr = r#"{"jsonrpc":"2.0","method":"hello","params":[10]}"#; - let deserialized: ClientResponse = serde_json::from_str(dsr).unwrap(); + + let deserialized: ClientResponse = arbitrary_precision_fix(dsr); assert_eq!( deserialized, ClientResponse::Notification(Notification { @@ -195,7 +205,8 @@ mod tests { #[test] fn success_deserialize() { let dsr = r#"{"jsonrpc":"2.0","result":1,"id":1}"#; - let deserialized: ClientResponse = serde_json::from_str(dsr).unwrap(); + + let deserialized: ClientResponse = arbitrary_precision_fix(dsr); assert_eq!( deserialized, ClientResponse::Output(Output::Success(Success { @@ -210,7 +221,7 @@ mod tests { fn failure_output_deserialize() { let dfo = r#"{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":1}"#; - let deserialized: ClientResponse = serde_json::from_str(dfo).unwrap(); + let deserialized: ClientResponse = arbitrary_precision_fix(dfo); assert_eq!( deserialized, ClientResponse::Output(Output::Failure(Failure { diff --git a/core/Cargo.toml b/core/Cargo.toml index 5eb848e1e..57c1f4183 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -25,5 +25,8 @@ serde = "1.0" serde_json = "1.0" serde_derive = "1.0" +[features] +arbitrary_precision = ["serde_json/arbitrary_precision"] + [badges] travis-ci = { repository = "paritytech/jsonrpc", branch = "master"} diff --git a/core/src/io.rs b/core/src/io.rs index 320bd0385..c184caaa9 100644 --- a/core/src/io.rs +++ b/core/src/io.rs @@ -10,7 +10,7 @@ use serde_json; use crate::calls::{Metadata, RemoteProcedure, RpcMethod, RpcMethodSimple, RpcNotification, RpcNotificationSimple}; use crate::middleware::{self, Middleware}; -use crate::types::{Call, Output, Request, Response}; +use crate::types::{Call, Output, Request, Response, Value}; use crate::types::{Error, ErrorCode, Version}; /// A type representing middleware or RPC response before serialization. @@ -455,7 +455,12 @@ impl IoHandlerExtension for IoHandler { } fn read_request(request_str: &str) -> Result { - serde_json::from_str(request_str).map_err(|_| Error::new(ErrorCode::ParseError)) + if cfg!(feature = "arbitrary_precision") { + let val: Value = serde_json::from_str(request_str).map_err(|_| Error::new(ErrorCode::ParseError))?; + serde_json::from_value(val).map_err(|_| Error::new(ErrorCode::ParseError)) + } else { + serde_json::from_str(request_str).map_err(|_| Error::new(ErrorCode::ParseError)) + } } fn write_response(response: Response) -> String { diff --git a/core/src/types/response.rs b/core/src/types/response.rs index ba380a27f..4b047b5b7 100644 --- a/core/src/types/response.rs +++ b/core/src/types/response.rs @@ -113,7 +113,12 @@ impl Response { if s.is_empty() { Ok(Response::Batch(vec![])) } else { - serde_json::from_str(s) + if cfg!(feature = "arbitrary_precision") { + let val = serde_json::from_str::(s)?; + serde_json::from_value(val) + } else { + serde_json::from_str(s) + } } } } diff --git a/test/Cargo.toml b/test/Cargo.toml index 634562716..062e11ae2 100644 --- a/test/Cargo.toml +++ b/test/Cargo.toml @@ -17,6 +17,9 @@ log = "0.4" serde = "1.0" serde_json = "1.0" +[features] +arbitrary_precision = ["jsonrpc-core-client/arbitrary_precision", "serde_json/arbitrary_precision", "jsonrpc-core/arbitrary_precision"] + [dev-dependencies] jsonrpc-derive = { version = "14.0", path = "../derive" } diff --git a/test/src/lib.rs b/test/src/lib.rs index 4b5fe5889..2a76156e8 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -119,8 +119,16 @@ impl Rpc { .handle_request_sync(&request) .expect("We are sending a method call not notification."); + // arbitrary precision workaround, https://github.com/serde-rs/json/issues/505 + let response = if cfg!(feature = "arbitrary_precision") { + let val = serde_json::from_str::(&response).expect("We will always get a single output."); + serde_json::from_value(val) + } else { + serde_json::from_str(&response) + }; + // extract interesting part from the response - let extracted = match serde_json::from_str(&response).expect("We will always get a single output.") { + let extracted = match response.expect("We will always get a single output.") { response::Output::Success(response::Success { result, .. }) => match encoding { Encoding::Compact => serde_json::to_string(&result), Encoding::Pretty => serde_json::to_string_pretty(&result), From e5df933de925f1cb92171783693388ac6212547c Mon Sep 17 00:00:00 2001 From: Andrew Plaza Date: Thu, 21 Nov 2019 19:47:53 +0100 Subject: [PATCH 4/6] add generic function for arbitrary_precision --- core-client/transports/src/transports/mod.rs | 45 ++++++-------------- core/src/io.rs | 9 +--- core/src/lib.rs | 17 ++++++++ core/src/types/response.rs | 7 +-- test/src/lib.rs | 10 +---- 5 files changed, 34 insertions(+), 54 deletions(-) diff --git a/core-client/transports/src/transports/mod.rs b/core-client/transports/src/transports/mod.rs index 8953b70dc..fdf54d74a 100644 --- a/core-client/transports/src/transports/mod.rs +++ b/core-client/transports/src/transports/mod.rs @@ -81,25 +81,16 @@ impl RequestBuilder { pub fn parse_response( response: &str, ) -> Result<(Id, Result, Option, Option), RpcError> { - // https://github.com/serde-rs/json/issues/505 - // Arbitrary precision confuses serde when deserializing into untagged enums, - // this is a workaround - let response = if cfg!(feature = "arbitrary_precision") { - let value = - serde_json::from_str::(&response).map_err(|e| RpcError::ParseError(e.to_string(), e.into()))?; - serde_json::from_value::(value).map_err(|e| RpcError::ParseError(e.to_string(), e.into())) - } else { - serde_json::from_str::(&response).map_err(|e| RpcError::ParseError(e.to_string(), e.into())) - }; - - response.map(|response| { - let id = response.id().unwrap_or(Id::Null); - let sid = response.subscription_id(); - let method = response.method(); - let value: Result = response.into(); - let result = value.map_err(RpcError::JsonRpcError); - (id, result, method, sid) - }) + jsonrpc_core::serde_from_str::(response) + .map_err(|e| RpcError::ParseError(e.to_string(), e.into())) + .map(|response| { + let id = response.id().unwrap_or(Id::Null); + let sid = response.subscription_id(); + let method = response.method(); + let value: Result = response.into(); + let result = value.map_err(RpcError::JsonRpcError); + (id, result, method, sid) + }) } /// A type representing all possible values sent from the server to the client. @@ -176,22 +167,12 @@ impl From for Result { mod tests { use super::*; use jsonrpc_core::{Failure, Notification, Output, Params, Success, Value, Version}; - use serde_json; - - fn arbitrary_precision_fix(input: &str) -> ClientResponse { - if cfg!(feature = "arbitrary_precision") { - let val: serde_json::Value = serde_json::from_str(input).unwrap(); - serde_json::from_value(val).unwrap() - } else { - serde_json::from_str(input).unwrap() - } - } #[test] fn notification_deserialize() { let dsr = r#"{"jsonrpc":"2.0","method":"hello","params":[10]}"#; - let deserialized: ClientResponse = arbitrary_precision_fix(dsr); + let deserialized: ClientResponse = jsonrpc_core::serde_from_str(dsr).unwrap(); assert_eq!( deserialized, ClientResponse::Notification(Notification { @@ -206,7 +187,7 @@ mod tests { fn success_deserialize() { let dsr = r#"{"jsonrpc":"2.0","result":1,"id":1}"#; - let deserialized: ClientResponse = arbitrary_precision_fix(dsr); + let deserialized: ClientResponse = jsonrpc_core::serde_from_str(dsr).unwrap(); assert_eq!( deserialized, ClientResponse::Output(Output::Success(Success { @@ -221,7 +202,7 @@ mod tests { fn failure_output_deserialize() { let dfo = r#"{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":1}"#; - let deserialized: ClientResponse = arbitrary_precision_fix(dfo); + let deserialized: ClientResponse = jsonrpc_core::serde_from_str(dfo).unwrap(); assert_eq!( deserialized, ClientResponse::Output(Output::Failure(Failure { diff --git a/core/src/io.rs b/core/src/io.rs index c184caaa9..3ca0ed1eb 100644 --- a/core/src/io.rs +++ b/core/src/io.rs @@ -10,7 +10,7 @@ use serde_json; use crate::calls::{Metadata, RemoteProcedure, RpcMethod, RpcMethodSimple, RpcNotification, RpcNotificationSimple}; use crate::middleware::{self, Middleware}; -use crate::types::{Call, Output, Request, Response, Value}; +use crate::types::{Call, Output, Request, Response}; use crate::types::{Error, ErrorCode, Version}; /// A type representing middleware or RPC response before serialization. @@ -455,12 +455,7 @@ impl IoHandlerExtension for IoHandler { } fn read_request(request_str: &str) -> Result { - if cfg!(feature = "arbitrary_precision") { - let val: Value = serde_json::from_str(request_str).map_err(|_| Error::new(ErrorCode::ParseError))?; - serde_json::from_value(val).map_err(|_| Error::new(ErrorCode::ParseError)) - } else { - serde_json::from_str(request_str).map_err(|_| Error::new(ErrorCode::ParseError)) - } + crate::serde_from_str(request_str).map_err(|_| Error::new(ErrorCode::ParseError)) } fn write_response(response: Response) -> String { diff --git a/core/src/lib.rs b/core/src/lib.rs index 87d05f7a0..0b7aaf3ad 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -54,3 +54,20 @@ pub use crate::io::{ }; pub use crate::middleware::{Middleware, Noop as NoopMiddleware}; pub use crate::types::*; + +use serde_json::{Error as SerdeError}; + +/// workaround for https://github.com/serde-rs/json/issues/505 +/// Arbitrary precision confuses serde when deserializing into untagged enums, +/// this is a workaround +pub fn serde_from_str<'a, T>(input: &'a str) -> ::std::result::Result +where + T: serde::de::Deserialize<'a> +{ + if cfg!(feature = "arbitrary_precision") { + let val = serde_json::from_str::(input)?; + T::deserialize(val) + } else { + serde_json::from_str::(input) + } +} diff --git a/core/src/types/response.rs b/core/src/types/response.rs index 4b047b5b7..f7d54b07b 100644 --- a/core/src/types/response.rs +++ b/core/src/types/response.rs @@ -113,12 +113,7 @@ impl Response { if s.is_empty() { Ok(Response::Batch(vec![])) } else { - if cfg!(feature = "arbitrary_precision") { - let val = serde_json::from_str::(s)?; - serde_json::from_value(val) - } else { - serde_json::from_str(s) - } + crate::serde_from_str(s) } } } diff --git a/test/src/lib.rs b/test/src/lib.rs index 2a76156e8..985709fdb 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -119,16 +119,8 @@ impl Rpc { .handle_request_sync(&request) .expect("We are sending a method call not notification."); - // arbitrary precision workaround, https://github.com/serde-rs/json/issues/505 - let response = if cfg!(feature = "arbitrary_precision") { - let val = serde_json::from_str::(&response).expect("We will always get a single output."); - serde_json::from_value(val) - } else { - serde_json::from_str(&response) - }; - // extract interesting part from the response - let extracted = match response.expect("We will always get a single output.") { + let extracted = match rpc::serde_from_str(&response).expect("We will always get a single output.") { response::Output::Success(response::Success { result, .. }) => match encoding { Encoding::Compact => serde_json::to_string(&result), Encoding::Pretty => serde_json::to_string_pretty(&result), From 3f8ae2264d22cbb14604bb46764ab6762b01e2cc Mon Sep 17 00:00:00 2001 From: Andrew Plaza Date: Thu, 21 Nov 2019 19:49:54 +0100 Subject: [PATCH 5/6] rustfmt --- core/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/lib.rs b/core/src/lib.rs index 0b7aaf3ad..28b4e7b89 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -55,14 +55,14 @@ pub use crate::io::{ pub use crate::middleware::{Middleware, Noop as NoopMiddleware}; pub use crate::types::*; -use serde_json::{Error as SerdeError}; +use serde_json::Error as SerdeError; /// workaround for https://github.com/serde-rs/json/issues/505 /// Arbitrary precision confuses serde when deserializing into untagged enums, /// this is a workaround pub fn serde_from_str<'a, T>(input: &'a str) -> ::std::result::Result where - T: serde::de::Deserialize<'a> + T: serde::de::Deserialize<'a>, { if cfg!(feature = "arbitrary_precision") { let val = serde_json::from_str::(input)?; From a2b939e4c408c21d5adb19842e5fb88f2c3ef338 Mon Sep 17 00:00:00 2001 From: Andrew Plaza Date: Fri, 22 Nov 2019 12:09:56 +0100 Subject: [PATCH 6/6] Update core/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Tomasz Drwięga --- core/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/lib.rs b/core/src/lib.rs index 28b4e7b89..1e0f27fd1 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -60,7 +60,7 @@ use serde_json::Error as SerdeError; /// workaround for https://github.com/serde-rs/json/issues/505 /// Arbitrary precision confuses serde when deserializing into untagged enums, /// this is a workaround -pub fn serde_from_str<'a, T>(input: &'a str) -> ::std::result::Result +pub fn serde_from_str<'a, T>(input: &'a str) -> std::result::Result where T: serde::de::Deserialize<'a>, {