From d24f8c8cb44a9193f3d0adcbeb7ceb6b20b0f7a6 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 25 Mar 2018 22:56:32 +0200 Subject: [PATCH] fix: Correctly handle debug meta in ser/de --- src/protocol/v7.rs | 50 ++++++++++--- tests/test_protocol_v7.rs | 146 +++++++++++++++++++++++++++++++++++++- 2 files changed, 185 insertions(+), 11 deletions(-) diff --git a/src/protocol/v7.rs b/src/protocol/v7.rs index 113ec41b..cc771992 100644 --- a/src/protocol/v7.rs +++ b/src/protocol/v7.rs @@ -19,7 +19,7 @@ use protocol::utils::ts_seconds_float; /// An arbitrary (JSON) value (`serde_json::value::Value`) pub mod value { - pub use serde_json::value::{Value, Index, Number, from_value, to_value}; + pub use serde_json::value::{Map, Value, Index, Number, from_value, to_value}; } /// The internally use arbitrary data map type (`linked_hash_map::LinkedHashMap`) @@ -198,6 +198,12 @@ impl From for ThreadId { } } +impl From for ThreadId { + fn from(id: i32) -> ThreadId { + ThreadId::Int(id as u64) + } +} + impl From for ThreadId { fn from(id: u32) -> ThreadId { ThreadId::Int(id as u64) @@ -223,6 +229,13 @@ impl fmt::Display for ThreadId { #[derive(Default, Debug, Clone, PartialEq, Eq, Ord, PartialOrd, Hash)] pub struct Addr(pub u64); +impl Addr { + /// Returns `true` if this address is the null pointer. + pub fn is_null(&self) -> bool { + self.0 == 0 + } +} + impl fmt::Display for Addr { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "0x{:x}", self.0) @@ -235,6 +248,12 @@ impl From for Addr { } } +impl From for Addr { + fn from(addr: i32) -> Addr { + Addr(addr as u64) + } +} + impl From for Addr { fn from(addr: u32) -> Addr { Addr(addr as u64) @@ -460,7 +479,7 @@ pub struct SystemSdkInfo { /// the major version of the SDK as integer or 0 pub version_major: u32, /// the minor version of the SDK as integer or 0 - pub version_minior: u32, + pub version_minor: u32, /// the patch version of the SDK as integer or 0 pub version_patchlevel: u32, } @@ -506,6 +525,7 @@ pub struct AppleDebugImage { /// The size of the image in bytes. pub image_size: u64, /// The address where the image is loaded at runtime. + #[serde(skip_serializing_if = "Addr::is_null")] pub image_vmaddr: Addr, /// The unique UUID of the image. pub uuid: Uuid, @@ -526,8 +546,17 @@ pub struct DebugMeta { #[serde(skip_serializing_if = "Option::is_none")] pub sdk_info: Option, /// A list of debug information files. - #[serde(skip_serializing_if = "Option::is_none")] - pub images: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub images: Vec, +} + +impl DebugMeta { + /// Returns true if the debug meta is empty. + /// + /// This is used by the serializer to entirely skip the section. + pub fn is_empty(&self) -> bool { + self.sdk_info.is_none() && self.images.is_empty() + } } /// Represents a repository reference. @@ -643,8 +672,8 @@ pub struct Event { #[serde(skip_serializing_if = "Map::is_empty")] pub extra: Map, /// Debug meta information. - #[serde(skip_serializing_if = "Option::is_none")] - pub debug_meta: Option, + #[serde(skip_serializing_if = "DebugMeta::is_empty")] + pub debug_meta: DebugMeta, /// SDK metadata #[serde(skip_serializing_if = "Option::is_none")] pub sdk_info: Option, @@ -689,7 +718,7 @@ impl Default for Event { threads: Vec::new(), tags: Map::new(), extra: Map::new(), - debug_meta: None, + debug_meta: Default::default(), sdk_info: None, other: Map::new(), } @@ -1006,7 +1035,12 @@ impl Serialize for DebugImage { where S: Serializer, { - let mut c = match to_value(self).map_err(S::Error::custom)? { + let actual = match *self { + DebugImage::Apple(ref img) => to_value(img), + DebugImage::Proguard(ref img) => to_value(img), + DebugImage::Unknown(ref img) => to_value(img), + }; + let mut c = match actual.map_err(S::Error::custom)? { Value::Object(map) => map, _ => unreachable!(), }; diff --git a/tests/test_protocol_v7.rs b/tests/test_protocol_v7.rs index 67334a4e..29ebd3e3 100644 --- a/tests/test_protocol_v7.rs +++ b/tests/test_protocol_v7.rs @@ -1,6 +1,6 @@ extern crate sentry_types; extern crate serde; -extern crate serde_json; +#[macro_use] extern crate serde_json; extern crate uuid; extern crate chrono; @@ -380,7 +380,7 @@ fn test_threads() { let event = v7::Event { threads: vec![ v7::Thread { - id: Some(42u32.into()), + id: Some(42.into()), name: Some("Awesome Thread".into()), crashed: true, current: true, @@ -396,6 +396,51 @@ fn test_threads() { "{\"threads\":{\"values\":[{\"id\":42,\"name\":\"Awesome Thread\",\ \"crashed\":true,\"current\":true}]}}" ); + + let event = v7::Event { + threads: vec![ + v7::Thread { + stacktrace: Some(v7::Stacktrace { + frames: vec![ + v7::Frame { + function: Some("main".into()), + location: v7::FileLocation { + filename: Some("hello.py".into()), + line: Some(1), + ..Default::default() + }, + ..Default::default() + }, + ], + ..Default::default() + }), + raw_stacktrace: Some(v7::Stacktrace { + frames: vec![ + v7::Frame { + function: Some("main".into()), + location: v7::FileLocation { + filename: Some("hello.py".into()), + line: Some(1), + ..Default::default() + }, + ..Default::default() + }, + ], + ..Default::default() + }), + ..Default::default() + } + ], + ..Default::default() + }; + + assert_roundtrip(&event); + assert_eq!( + serde_json::to_string(&event).unwrap(), + "{\"threads\":{\"values\":[{\"stacktrace\":{\"frames\":[{\"function\":\ + \"main\",\"filename\":\"hello.py\",\"lineno\":1}]},\"raw_stacktrace\"\ + :{\"frames\":[{\"function\":\"main\",\"filename\":\"hello.py\",\"lineno\":1}]}}]}}" + ); } #[test] @@ -470,6 +515,101 @@ fn test_request() { ); } +#[test] +fn test_tags() { + let event = v7::Event { + tags: { + let mut m = v7::Map::new(); + m.insert("device_type".into(), "mobile".into()); + m.insert("interpreter".into(), "7".into()); + m + }, + ..Default::default() + }; + + assert_roundtrip(&event); + assert_eq!( + serde_json::to_string(&event).unwrap(), + "{\"tags\":{\"device_type\":\"mobile\",\"interpreter\":\"7\"}}" + ); +} + +#[test] +fn test_extra() { + let event = v7::Event { + extra: { + let mut m = v7::Map::new(); + m.insert("component_state".into(), json!({ + "dirty": true, + "revision": 17 + })); + m + }, + ..Default::default() + }; + + assert_roundtrip(&event); + assert_eq!( + serde_json::to_string(&event).unwrap(), + "{\"extra\":{\"component_state\":{\"dirty\":true,\"revision\":17}}}" + ); +} + +#[test] +fn test_debug_meta() { + let event = v7::Event { + debug_meta: v7::DebugMeta { + sdk_info: Some(v7::SystemSdkInfo { + sdk_name: "iOS".into(), + version_major: 10, + version_minor: 3, + version_patchlevel: 0, + }), + ..Default::default() + }, + ..Default::default() + }; + + assert_roundtrip(&event); + assert_eq!( + serde_json::to_string(&event).unwrap(), + "{\"debug_meta\":{\"sdk_info\":{\"sdk_name\":\"iOS\",\"version_major\":10,\ + \"version_minor\":3,\"version_patchlevel\":0}}}" + ); + + let event = v7::Event { + debug_meta: v7::DebugMeta { + images: vec![ + v7::DebugImage::Apple(v7::AppleDebugImage { + name: "CoreFoundation".into(), + arch: Some("arm64".into()), + cpu_type: Some(1233), + cpu_subtype: Some(3), + image_addr: 0.into(), + image_size: 4096, + image_vmaddr: 32768.into(), + uuid: "494f3aea-88fa-4296-9644-fa8ef5d139b6".parse().unwrap(), + }), + v7::DebugImage::Proguard(v7::ProguardDebugImage { + uuid: "8c954262-f905-4992-8a61-f60825f4553b".parse().unwrap(), + }) + ], + ..Default::default() + }, + ..Default::default() + }; + + assert_roundtrip(&event); + assert_eq!( + serde_json::to_string(&event).unwrap(), + "{\"debug_meta\":{\"images\":[{\"name\":\"CoreFoundation\",\"arch\":\ + \"arm64\",\"cpu_type\":1233,\"cpu_subtype\":3,\"image_addr\":\"0x0\",\ + \"image_size\":4096,\"image_vmaddr\":\"0x8000\",\"uuid\":\ + \"494f3aea-88fa-4296-9644-fa8ef5d139b6\",\"type\":\"apple\"},\ + {\"uuid\":\"8c954262-f905-4992-8a61-f60825f4553b\",\"type\":\"proguard\"}]}}" + ); +} + #[test] fn test_canonical_exception() { let mut event: v7::Event = Default::default(); @@ -691,7 +831,7 @@ fn test_addr_format() { fn test_addr_api() { use std::ptr; assert_eq!(v7::Addr::from(42u64), v7::Addr(42)); - assert_eq!(v7::Addr::from(42u32), v7::Addr(42)); + assert_eq!(v7::Addr::from(42), v7::Addr(42)); assert_eq!(v7::Addr::from(ptr::null::<()>()), v7::Addr(0)); }