Skip to content

Commit 091577f

Browse files
authored
Strict and non-strict parsing for core (#448)
* Strict and non-strict mode. * Remove the non-strict mode. * Reformat.
1 parent 584327b commit 091577f

File tree

11 files changed

+85
-32
lines changed

11 files changed

+85
-32
lines changed

core-client/transports/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,10 @@ mod tests {
432432
});
433433
tokio::run(fut);
434434
assert_eq!(called.load(Ordering::SeqCst), true);
435-
assert!(!received.lock().unwrap().is_empty(), "Expected at least one received item.");
435+
assert!(
436+
!received.lock().unwrap().is_empty(),
437+
"Expected at least one received item."
438+
);
436439
}
437440

438441
}

core-client/transports/src/transports/duplex.rs

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,7 @@ struct Subscription {
2525
}
2626

2727
impl Subscription {
28-
fn new(
29-
channel: mpsc::Sender<Result<Value, RpcError>>,
30-
notification: String,
31-
unsubscribe: String,
32-
) -> Self {
28+
fn new(channel: mpsc::Sender<Result<Value, RpcError>>, notification: String, unsubscribe: String) -> Self {
3329
Subscription {
3430
id: None,
3531
notification,
@@ -120,7 +116,11 @@ where
120116
RpcMessage::Call(msg) => {
121117
let (id, request_str) = self.request_builder.call_request(&msg);
122118

123-
if self.pending_requests.insert(id.clone(), PendingRequest::Call(msg.sender)).is_some() {
119+
if self
120+
.pending_requests
121+
.insert(id.clone(), PendingRequest::Call(msg.sender))
122+
.is_some()
123+
{
124124
log::error!("reuse of request id {:?}", id);
125125
}
126126
request_str
@@ -132,13 +132,14 @@ where
132132
notification,
133133
unsubscribe,
134134
} = msg.subscription;
135-
let (id, request_str) = self.request_builder.subscribe_request(
136-
subscribe,
137-
subscribe_params,
138-
);
135+
let (id, request_str) = self.request_builder.subscribe_request(subscribe, subscribe_params);
139136
log::debug!("subscribing to {}", notification);
140137
let subscription = Subscription::new(msg.sender, notification, unsubscribe);
141-
if self.pending_requests.insert(id.clone(), PendingRequest::Subscription(subscription)).is_some() {
138+
if self
139+
.pending_requests
140+
.insert(id.clone(), PendingRequest::Subscription(subscription))
141+
.is_some()
142+
{
142143
log::error!("reuse of request id {:?}", id);
143144
}
144145
request_str
@@ -166,7 +167,13 @@ where
166167
};
167168
log::debug!("incoming: {}", response_str);
168169
for (id, result, method, sid) in super::parse_response(&response_str)? {
169-
log::debug!("id: {:?} (sid: {:?}) result: {:?} method: {:?}", id, sid, result, method);
170+
log::debug!(
171+
"id: {:?} (sid: {:?}) result: {:?} method: {:?}",
172+
id,
173+
sid,
174+
result,
175+
method
176+
);
170177
self.incoming.push_back((id, result, method, sid));
171178
}
172179
}
@@ -184,7 +191,7 @@ where
184191
tx.send(result)
185192
.map_err(|_| RpcError::Other(format_err!("oneshot channel closed")))?;
186193
continue;
187-
},
194+
}
188195
// It was a subscription request,
189196
// turn it into a proper subscription.
190197
Some(PendingRequest::Subscription(mut subscription)) => {
@@ -193,10 +200,14 @@ where
193200

194201
if let Some(sid) = sid {
195202
subscription.id = Some(sid.clone());
196-
if self.subscriptions.insert((sid.clone(), method.clone()), subscription).is_some() {
203+
if self
204+
.subscriptions
205+
.insert((sid.clone(), method.clone()), subscription)
206+
.is_some()
207+
{
197208
log::warn!(
198209
"Overwriting existing subscription under {:?} ({:?}). \
199-
Seems that server returned the same subscription id.",
210+
Seems that server returned the same subscription id.",
200211
sid,
201212
method,
202213
);
@@ -210,12 +221,12 @@ where
210221
);
211222
}
212223
continue;
213-
},
224+
}
214225
// It's not a pending request nor a notification
215226
None if sid_and_method.is_none() => {
216227
log::warn!("Got unexpected response with id {:?} ({:?})", id, sid_and_method);
217228
continue;
218-
},
229+
}
219230
// just fall-through in case it's a notification
220231
None => {}
221232
};
@@ -229,20 +240,25 @@ where
229240
if let Some(subscription) = self.subscriptions.get_mut(&sid_and_method) {
230241
match subscription.channel.poll_ready() {
231242
Ok(Async::Ready(())) => {
232-
subscription.channel.try_send(result).expect("The channel is ready; qed");
243+
subscription
244+
.channel
245+
.try_send(result)
246+
.expect("The channel is ready; qed");
233247
}
234248
Ok(Async::NotReady) => {
235249
let (sid, method) = sid_and_method;
236250
self.incoming.push_front((id, result, Some(method), Some(sid)));
237251
break;
238252
}
239253
Err(_) => {
240-
let subscription = self.subscriptions
254+
let subscription = self
255+
.subscriptions
241256
.remove(&sid_and_method)
242257
.expect("Subscription was just polled; qed");
243-
let sid = subscription.id
244-
.expect("Every subscription that ends up in `self.subscriptions` has id already \
245-
assigned; assignment happens during response to subscribe request.");
258+
let sid = subscription.id.expect(
259+
"Every subscription that ends up in `self.subscriptions` has id already \
260+
assigned; assignment happens during response to subscribe request.",
261+
);
246262
let (_id, request_str) =
247263
self.request_builder.unsubscribe_request(subscription.unsubscribe, sid);
248264
log::debug!("outgoing: {}", request_str);
@@ -253,7 +269,7 @@ where
253269
} else {
254270
log::warn!("Received unexpected subscription notification: {:?}", sid_and_method);
255271
}
256-
},
272+
}
257273
None => break,
258274
}
259275
}

core-client/transports/src/transports/local.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ where
2525
/// Creates a new `LocalRpc` with default metadata.
2626
pub fn new(handler: THandler) -> Self
2727
where
28-
TMetadata: Default
28+
TMetadata: Default,
2929
{
3030
Self::with_metadata(handler, Default::default())
3131
}

core-client/transports/src/transports/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ impl RequestBuilder {
6161
}
6262

6363
/// Parse raw string into JSON values, together with the request Id
64-
pub fn parse_response(response: &str) -> Result<Vec<(Id, Result<Value, RpcError>, Option<String>, Option<SubscriptionId>)>, RpcError> {
64+
pub fn parse_response(
65+
response: &str,
66+
) -> Result<Vec<(Id, Result<Value, RpcError>, Option<String>, Option<SubscriptionId>)>, RpcError> {
6567
serde_json::from_str::<Response>(&response)
6668
.map_err(|e| RpcError::ParseError(e.to_string(), e.into()))
6769
.map(|response| {

core/examples/params.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use jsonrpc_core::*;
2-
use serde::Deserialize;
2+
use serde_derive::Deserialize;
33

44
#[derive(Deserialize)]
55
struct HelloParams {

core/src/types/error.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ impl Serialize for ErrorCode {
8383

8484
/// Error object as defined in Spec
8585
#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
86+
#[serde(deny_unknown_fields)]
8687
pub struct Error {
8788
/// Code
8889
pub code: ErrorCode,

core/src/types/id.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
/// Request Id
44
#[derive(Debug, PartialEq, Clone, Hash, Eq, Deserialize, Serialize)]
5+
#[serde(deny_unknown_fields)]
56
#[serde(untagged)]
67
pub enum Id {
78
/// No id (notification)

core/src/types/params.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use super::{Error, Value};
88

99
/// Request parameters
1010
#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
11+
#[serde(deny_unknown_fields)]
1112
#[serde(untagged)]
1213
pub enum Params {
1314
/// No parameters

core/src/types/request.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ impl From<Notification> for Call {
7272

7373
/// Represents jsonrpc request.
7474
#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
75+
#[serde(deny_unknown_fields)]
7576
#[serde(untagged)]
7677
pub enum Request {
7778
/// Single request (call)
@@ -187,7 +188,7 @@ mod tests {
187188

188189
let s = r#"{"jsonrpc": "2.0", "method": "update", "params": [1,2], "id": 1}"#;
189190
let deserialized: Result<Notification, _> = serde_json::from_str(s);
190-
assert!(deserialized.is_err())
191+
assert!(deserialized.is_err());
191192
}
192193

193194
#[test]
@@ -285,6 +286,7 @@ mod tests {
285286

286287
let s = r#"{"id":120,"method":"my_method","params":["foo", "bar"],"extra_field":[]}"#;
287288
let deserialized: Request = serde_json::from_str(s).unwrap();
289+
288290
match deserialized {
289291
Request::Single(Call::Invalid { id: Id::Num(120) }) => {}
290292
_ => panic!("Request wrongly deserialized: {:?}", deserialized),

core/src/types/response.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::Result as CoreResult;
44

55
/// Successful response
66
#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
7+
#[serde(deny_unknown_fields)]
78
pub struct Success {
89
/// Protocol version
910
#[serde(skip_serializing_if = "Option::is_none")]
@@ -16,6 +17,7 @@ pub struct Success {
1617

1718
/// Unsuccessful response
1819
#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
20+
#[serde(deny_unknown_fields)]
1921
pub struct Failure {
2022
/// Protocol Version
2123
#[serde(skip_serializing_if = "Option::is_none")]
@@ -28,6 +30,7 @@ pub struct Failure {
2830

2931
/// Represents output - failure or success
3032
#[derive(Debug, PartialEq, Clone, Deserialize, Serialize)]
33+
#[serde(deny_unknown_fields)]
3134
#[serde(untagged)]
3235
pub enum Output {
3336
/// Notification
@@ -114,6 +117,7 @@ impl From<Output> for CoreResult<Value> {
114117

115118
/// Synchronous response
116119
#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
120+
#[serde(deny_unknown_fields)]
117121
#[serde(untagged)]
118122
pub enum Response {
119123
/// Single response
@@ -290,3 +294,26 @@ fn notification_deserialize() {
290294
}))
291295
);
292296
}
297+
298+
#[test]
299+
fn handle_incorrect_responses() {
300+
use serde_json;
301+
302+
let dsr = r#"
303+
{
304+
"id": 2,
305+
"jsonrpc": "2.0",
306+
"result": "0x62d3776be72cc7fa62cad6fe8ed873d9bc7ca2ee576e400d987419a3f21079d5",
307+
"error": {
308+
"message": "VM Exception while processing transaction: revert",
309+
"code": -32000,
310+
"data": {}
311+
}
312+
}"#;
313+
314+
let deserialized: Result<Response, _> = serde_json::from_str(dsr);
315+
assert!(
316+
deserialized.is_err(),
317+
"Expected error when deserializing invalid payload."
318+
);
319+
}

0 commit comments

Comments
 (0)