From 2bb96b5a2b030139f0c1503a5ceb056d5921304e Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Tue, 5 May 2020 22:51:46 +0200 Subject: [PATCH] Use serde_json::value::RawValue instead of serde_json::Value in most endpoints --- Cargo.toml | 4 + src/r0/config/set_global_account_data.rs | 6 +- src/r0/config/set_room_account_data.rs | 6 +- src/r0/media/get_media_preview.rs | 38 ++- src/r0/room/create_room.rs | 6 +- .../state/get_state_events_for_empty_key.rs | 6 +- src/r0/state/get_state_events_for_key.rs | 4 +- src/r0/uiaa.rs | 264 ++++++++++-------- 8 files changed, 202 insertions(+), 132 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c3502ef8..c0e19c4b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,3 +23,7 @@ ruma-serde = "0.1.3" serde = { version = "1.0.106", features = ["derive"] } serde_json = "1.0.52" strum = { version = "0.18.0", features = ["derive"] } + +[dev-dependencies] +maplit = "1.0.2" +matches = "0.1.8" diff --git a/src/r0/config/set_global_account_data.rs b/src/r0/config/set_global_account_data.rs index 184f4cf1..9214271c 100644 --- a/src/r0/config/set_global_account_data.rs +++ b/src/r0/config/set_global_account_data.rs @@ -2,7 +2,7 @@ use ruma_api::ruma_api; use ruma_identifiers::UserId; -use serde_json::Value as JsonValue; +use serde_json::value::RawValue as RawJsonValue; ruma_api! { metadata { @@ -16,8 +16,10 @@ ruma_api! { request { /// Arbitrary JSON to store as config data. + /// + /// To create a `Box`, use `serde_json::value::to_raw_value`. #[ruma_api(body)] - pub data: JsonValue, + pub data: Box, /// The event type of the account_data to set. /// diff --git a/src/r0/config/set_room_account_data.rs b/src/r0/config/set_room_account_data.rs index 26949547..457ee643 100644 --- a/src/r0/config/set_room_account_data.rs +++ b/src/r0/config/set_room_account_data.rs @@ -2,7 +2,7 @@ use ruma_api::ruma_api; use ruma_identifiers::{RoomId, UserId}; -use serde_json::Value as JsonValue; +use serde_json::value::RawValue as RawJsonValue; ruma_api! { metadata { @@ -16,8 +16,10 @@ ruma_api! { request { /// Arbitrary JSON to store as config data. + /// + /// To create a `Box`, use `serde_json::value::to_raw_value`. #[ruma_api(body)] - pub data: JsonValue, + pub data: Box, /// The event type of the account_data to set. /// diff --git a/src/r0/media/get_media_preview.rs b/src/r0/media/get_media_preview.rs index d387b142..ea96ccd6 100644 --- a/src/r0/media/get_media_preview.rs +++ b/src/r0/media/get_media_preview.rs @@ -3,7 +3,7 @@ use std::time::SystemTime; use ruma_api::ruma_api; -use serde_json::Value as JsonValue; +use serde_json::value::RawValue as RawJsonValue; ruma_api! { metadata { @@ -32,8 +32,42 @@ ruma_api! { /// Differences from OpenGraph: the image size in bytes is added to the `matrix:image:size` /// field, and `og:image` returns the MXC URI to the image, if any. #[ruma_api(body)] - pub data: Option, + pub data: Option>, } error: crate::Error } + +#[cfg(test)] +mod tests { + use serde_json::{ + from_value as from_json_value, json, + value::{to_raw_value as to_raw_json_value, RawValue as RawJsonValue}, + }; + + // Since BTreeMap> deserialization doesn't seem to + // work, test that Option works + #[test] + fn raw_json_deserialize() { + type OptRawJson = Option>; + + assert!(from_json_value::(json!(null)) + .unwrap() + .is_none()); + assert!(from_json_value::(json!("test")) + .unwrap() + .is_some()); + assert!(from_json_value::(json!({ "a": "b" })) + .unwrap() + .is_some()); + } + + // For completeness sake, make sure serialization works too + #[test] + fn raw_json_serialize() { + assert!(to_raw_json_value(&json!(null)).is_ok()); + assert!(to_raw_json_value(&json!("string")).is_ok()); + assert!(to_raw_json_value(&json!({})).is_ok()); + assert!(to_raw_json_value(&json!({ "a": "b" })).is_ok()); + } +} diff --git a/src/r0/room/create_room.rs b/src/r0/room/create_room.rs index 83b6c35f..b276a32f 100644 --- a/src/r0/room/create_room.rs +++ b/src/r0/room/create_room.rs @@ -4,7 +4,7 @@ use ruma_api::ruma_api; use ruma_events::{room::power_levels::PowerLevelsEventContent, EventJson}; use ruma_identifiers::{RoomId, UserId}; use serde::{Deserialize, Serialize}; -use serde_json::Value as JsonValue; +use serde_json::value::RawValue as RawJsonValue; use super::Visibility; use crate::r0::membership::Invite3pid; @@ -121,5 +121,7 @@ pub struct InitialStateEvent { pub state_key: Option, /// JSON content of the state event. - pub content: JsonValue, + /// + /// To create a `Box`, use `serde_json::value::to_raw_value`. + pub content: Box, } diff --git a/src/r0/state/get_state_events_for_empty_key.rs b/src/r0/state/get_state_events_for_empty_key.rs index 42320472..c9400be0 100644 --- a/src/r0/state/get_state_events_for_empty_key.rs +++ b/src/r0/state/get_state_events_for_empty_key.rs @@ -3,7 +3,7 @@ use ruma_api::ruma_api; use ruma_events::EventType; use ruma_identifiers::RoomId; -use serde_json::Value as JsonValue; +use serde_json::value::RawValue as RawJsonValue; ruma_api! { metadata { @@ -27,8 +27,10 @@ ruma_api! { response { /// The content of the state event. + /// + /// To create a `Box`, use `serde_json::value::to_raw_value`. #[ruma_api(body)] - pub content: JsonValue, + pub content: Box, } error: crate::Error diff --git a/src/r0/state/get_state_events_for_key.rs b/src/r0/state/get_state_events_for_key.rs index efd8c364..ad30024a 100644 --- a/src/r0/state/get_state_events_for_key.rs +++ b/src/r0/state/get_state_events_for_key.rs @@ -3,7 +3,7 @@ use ruma_api::ruma_api; use ruma_events::EventType; use ruma_identifiers::RoomId; -use serde_json::Value as JsonValue; +use serde_json::value::RawValue as RawJsonValue; ruma_api! { metadata { @@ -32,7 +32,7 @@ ruma_api! { response { /// The content of the state event. #[ruma_api(body)] - pub content: JsonValue, + pub content: Box, } error: crate::Error diff --git a/src/r0/uiaa.rs b/src/r0/uiaa.rs index 273317bf..73994d17 100644 --- a/src/r0/uiaa.rs +++ b/src/r0/uiaa.rs @@ -7,14 +7,16 @@ use std::{ use ruma_api::{error::ResponseDeserializationError, EndpointError}; use serde::{Deserialize, Serialize}; -use serde_json::{from_slice as from_json_slice, to_vec as to_json_vec, Value as JsonValue}; +use serde_json::{ + from_slice as from_json_slice, to_vec as to_json_vec, value::RawValue as RawJsonValue, + Value as JsonValue, +}; use crate::error::{Error as MatrixError, ErrorBody}; /// Additional authentication information for the user-interactive authentication API. #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(untagged)] -#[cfg_attr(test, derive(PartialEq))] pub enum AuthData { /// Used for sending UIAA authentication requests to the homeserver directly /// from the client. @@ -28,6 +30,7 @@ pub enum AuthData { session: Option, /// Parameters submitted for a particular authentication stage. + // FIXME: RawJsonValue doesn't work here, is that a bug? #[serde(flatten)] auth_parameters: BTreeMap, }, @@ -43,7 +46,6 @@ pub enum AuthData { /// Information about available authentication flows and status for /// User-Interactive Authenticiation API. #[derive(Clone, Debug, Deserialize, Serialize)] -#[cfg_attr(test, derive(PartialEq))] pub struct UiaaInfo { /// List of authentication flows available for this endpoint. pub flows: Vec, @@ -52,8 +54,11 @@ pub struct UiaaInfo { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub completed: Vec, - /// Authentication parameters required for the client to complete authentication. - pub params: JsonValue, + /// Authentication parameters required for the client to complete + /// authentication. + /// + /// To create a `Box`, use `serde_json::value::to_raw_value`. + pub params: Box, /// Session key for client to use to complete authentication. #[serde(skip_serializing_if = "Option::is_none")] @@ -129,12 +134,13 @@ impl From for http::Response> { #[cfg(test)] mod tests { - use std::collections::BTreeMap; - + use maplit::btreemap; + use matches::assert_matches; use ruma_api::EndpointError; use serde_json::{ - from_slice as from_json_slice, from_value as from_json_value, json, - to_value as to_json_value, Value as JsonValue, + from_slice as from_json_slice, from_str as from_json_str, from_value as from_json_value, + json, to_value as to_json_value, value::to_raw_value as to_raw_json_value, + Value as JsonValue, }; use super::{AuthData, AuthFlow, UiaaInfo, UiaaResponse}; @@ -142,40 +148,40 @@ mod tests { #[test] fn test_serialize_authentication_data_direct_request() { - let mut auth_parameters = BTreeMap::new(); - auth_parameters.insert( - "example_credential".into(), - JsonValue::String("verypoorsharedsecret".into()), - ); let authentication_data = AuthData::DirectRequest { kind: "example.type.foo".to_string(), session: Some("ZXY000".to_string()), - auth_parameters, + auth_parameters: btreemap! { + "example_credential".to_owned() => json!("verypoorsharedsecret") + }, }; assert_eq!( - json!({ "type": "example.type.foo", "session": "ZXY000", "example_credential": "verypoorsharedsecret" }), + json!({ + "type": "example.type.foo", + "session": "ZXY000", + "example_credential": "verypoorsharedsecret", + }), to_json_value(authentication_data).unwrap() ); } #[test] fn test_deserialize_authentication_data_direct_request() { - let mut auth_parameters = BTreeMap::new(); - auth_parameters.insert( - "example_credential".into(), - JsonValue::String("verypoorsharedsecret".into()), - ); - let authentication_data = AuthData::DirectRequest { - kind: "example.type.foo".into(), - session: Some("opaque_session_id".to_string()), - auth_parameters, - }; - let json = json!({ "type": "example.type.foo", "session": "opaque_session_id", "example_credential": "verypoorsharedsecret", }); + let json = json!({ + "type": "example.type.foo", + "session": "opaque_session_id", + "example_credential": "verypoorsharedsecret", + }); - assert_eq!( + assert_matches!( from_json_value::(json).unwrap(), - authentication_data + AuthData::DirectRequest { kind, session: Some(session), auth_parameters } + if kind == "example.type.foo" + && session == "opaque_session_id" + && auth_parameters == btreemap!{ + "example_credential".to_owned() => json!("verypoorsharedsecret") + } ); } @@ -193,31 +199,28 @@ mod tests { #[test] fn test_deserialize_authentication_data_fallback() { - let authentication_data = AuthData::FallbackAcknowledgement { - session: "opaque_session_id".to_string(), - }; let json = json!({ "session": "opaque_session_id" }); - assert_eq!( + assert_matches!( from_json_value::(json).unwrap(), - authentication_data + AuthData::FallbackAcknowledgement { session } + if session == "opaque_session_id" ); } #[test] fn test_serialize_uiaa_info() { - let params = json!({ - "example.type.baz": { - "example_key": "foobar" - } - }); - let uiaa_info = UiaaInfo { flows: vec![AuthFlow { stages: vec!["m.login.password".to_string(), "m.login.dummy".to_string()], }], completed: vec!["m.login.password".to_string()], - params, + params: to_raw_json_value(&json!({ + "example.type.baz": { + "example_key": "foobar" + } + })) + .unwrap(), session: None, auth_error: None, }; @@ -239,13 +242,13 @@ mod tests { let json = json!({ "errcode": "M_FORBIDDEN", "error": "Invalid password", - "completed": [ "example.type.foo" ], + "completed": ["example.type.foo"], "flows": [ { - "stages": [ "example.type.foo", "example.type.bar" ] + "stages": ["example.type.foo", "example.type.bar"] }, { - "stages": [ "example.type.foo", "example.type.baz" ] + "stages": ["example.type.foo", "example.type.baz"] } ], "params": { @@ -256,59 +259,77 @@ mod tests { "session": "xxxxxx" }); - let uiaa_info = UiaaInfo { - auth_error: Some(ErrorBody { - kind: ErrorKind::Forbidden, - message: "Invalid password".to_string(), - }), - completed: vec!["example.type.foo".to_string()], - flows: vec![ - AuthFlow { - stages: vec![ - "example.type.foo".to_string(), - "example.type.bar".to_string(), - ], - }, - AuthFlow { - stages: vec![ - "example.type.foo".to_string(), - "example.type.baz".to_string(), - ], - }, - ], - params: json!({ - "example.type.baz": { - "example_key": "foobar" - } - }), - session: Some("xxxxxx".to_string()), - }; - assert_eq!(from_json_value::(json).unwrap(), uiaa_info); + assert_matches!( + from_json_value::(json).unwrap(), + UiaaInfo { + auth_error: Some(ErrorBody { + kind: ErrorKind::Forbidden, + message: error_message, + }), + completed, + flows, + params, + session: Some(session), + } if error_message == "Invalid password" + && completed == vec!["example.type.foo".to_string()] + && flows == vec![ + AuthFlow { + stages: vec![ + "example.type.foo".to_string(), + "example.type.bar".to_string(), + ], + }, + AuthFlow { + stages: vec![ + "example.type.foo".to_string(), + "example.type.baz".to_string(), + ], + }, + ] + && from_json_str::(params.get()).unwrap() == json!({ + "example.type.baz": { + "example_key": "foobar" + } + }) + && session == "xxxxxx" + ); } #[test] fn test_try_uiaa_response_into_http_response() { - let params = json!({ - "example.type.baz": { - "example_key": "foobar" - } - }); - let uiaa_info = UiaaInfo { flows: vec![AuthFlow { stages: vec!["m.login.password".to_string(), "m.login.dummy".to_string()], }], completed: vec!["m.login.password".to_string()], - params, + params: to_raw_json_value(&json!({ + "example.type.baz": { + "example_key": "foobar" + } + })) + .unwrap(), session: None, auth_error: None, }; - let uiaa_response: http::Response> = - UiaaResponse::AuthResponse(uiaa_info.clone()).into(); + let uiaa_response: http::Response> = UiaaResponse::AuthResponse(uiaa_info).into(); - assert_eq!( + assert_matches!( from_json_slice::(uiaa_response.body()).unwrap(), - uiaa_info, + UiaaInfo { + flows, + completed, + params, + session: None, + auth_error: None, + } if flows == vec![AuthFlow { + stages: vec!["m.login.password".to_string(), "m.login.dummy".to_string()], + }] + && completed == vec!["m.login.password".to_string()] + && from_json_str::(params.get()).unwrap() == json!({ + "example.type.baz": { + "example_key": "foobar" + } + }) ); assert_eq!( uiaa_response.status(), @@ -324,10 +345,10 @@ mod tests { "completed": [ "example.type.foo" ], "flows": [ { - "stages": [ "example.type.foo", "example.type.bar" ] + "stages": [ "example.type.foo", "example.type.bar" ] }, { - "stages": [ "example.type.foo", "example.type.baz" ] + "stages": [ "example.type.foo", "example.type.baz" ] } ], "params": { @@ -338,47 +359,50 @@ mod tests { "session": "xxxxxx" })) .unwrap(); + let http_response = http::Response::builder() .status(http::StatusCode::UNAUTHORIZED) .body(json.into()) .unwrap(); - let uiaa_info = UiaaInfo { - auth_error: Some(ErrorBody { - kind: ErrorKind::Forbidden, - message: "Invalid password".to_string(), - }), - completed: vec!["example.type.foo".to_string()], - flows: vec![ - AuthFlow { - stages: vec![ - "example.type.foo".to_string(), - "example.type.bar".to_string(), - ], - }, - AuthFlow { - stages: vec![ - "example.type.foo".to_string(), - "example.type.baz".to_string(), - ], - }, - ], - params: json!({ - "example.type.baz": { - "example_key": "foobar" - } - }), - session: Some("xxxxxx".to_string()), + let parsed_uiaa_info = match UiaaResponse::try_from_response(http_response).unwrap() { + UiaaResponse::AuthResponse(uiaa_info) => uiaa_info, + _ => panic!("Expected UiaaResponse::AuthResponse"), }; - let parsed_uiaa_info = match UiaaResponse::try_from_response(http_response) { - Ok(auth_response) => match auth_response { - UiaaResponse::AuthResponse(uiaa_info) => Some(uiaa_info), - _ => None, - }, - _ => None, - }; - - assert_eq!(parsed_uiaa_info, Some(uiaa_info)); + assert_matches!( + parsed_uiaa_info, + UiaaInfo { + auth_error: Some(ErrorBody { + kind: ErrorKind::Forbidden, + message: error_message, + }), + completed, + flows, + params, + session: Some(session), + } if error_message == "Invalid password" + && completed == vec!["example.type.foo".to_string()] + && flows == vec![ + AuthFlow { + stages: vec![ + "example.type.foo".to_string(), + "example.type.bar".to_string(), + ], + }, + AuthFlow { + stages: vec![ + "example.type.foo".to_string(), + "example.type.baz".to_string(), + ], + }, + ] + && from_json_str::(params.get()).unwrap() == json!({ + "example.type.baz": { + "example_key": "foobar" + } + }) + && session == "xxxxxx" + ); } }