Skip to content

Commit

Permalink
fix(general): Reject exceptions with empty type and value
Browse files Browse the repository at this point in the history
  • Loading branch information
jan-auer committed Jan 25, 2019
1 parent 96ebf37 commit 1284589
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 20 deletions.
18 changes: 10 additions & 8 deletions general/src/protocol/exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,16 @@ fn test_exception_default_values() {
}

#[test]
fn test_exception_without_type() {
assert_eq!(
false,
Annotated::<Exception>::from_json("{}")
.unwrap()
.1
.has_errors()
);
fn test_exception_empty_fields() {
let json = r#"{"type":"","value":""}"#;
let exception = Annotated::new(Exception {
ty: Annotated::new("".to_string()),
value: Annotated::new("".to_string().into()),
..Default::default()
});

assert_eq_dbg!(exception, Annotated::from_json(json).unwrap());
assert_eq_str!(json, exception.to_json().unwrap());
}

#[test]
Expand Down
23 changes: 19 additions & 4 deletions general/src/store/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl<'a> NormalizeProcessor<'a> {

/// Ensures that the `release` and `dist` fields match up.
fn normalize_release_dist(&self, event: &mut Event) {
if event.dist.value().is_some() && event.release.value().is_none() {
if event.dist.value().is_some() && event.release.value().is_empty() {
event.dist.set_value(None);
}

Expand Down Expand Up @@ -332,7 +332,7 @@ impl<'a> Processor for NormalizeProcessor<'a> {
) -> ValueAction {
breadcrumb.process_child_values(self, state);

if breadcrumb.ty.value().is_none() {
if breadcrumb.ty.value().is_empty() {
breadcrumb.ty.set_value(Some("default".to_string()));
}

Expand Down Expand Up @@ -433,7 +433,7 @@ impl<'a> Processor for NormalizeProcessor<'a> {
static ref TYPE_VALUE_RE: Regex = Regex::new(r"^(\w+):(.*)$").unwrap();
}

if exception.ty.value().is_none() {
if exception.ty.value().is_empty() {
if let Some(value_str) = exception.value.value_mut() {
let new_values = TYPE_VALUE_RE
.captures(value_str)
Expand All @@ -446,7 +446,7 @@ impl<'a> Processor for NormalizeProcessor<'a> {
}
}

if exception.ty.value().is_none() && exception.value.value().is_none() {
if exception.ty.value().is_empty() && exception.value.value().is_empty() {
meta.add_error(Error::with(ErrorKind::MissingAttribute, |error| {
error.insert("attribute", "type or value");
}));
Expand Down Expand Up @@ -560,6 +560,21 @@ fn test_handles_type_in_value() {
assert_eq_dbg!(exception.ty.as_str(), Some("ValueError"));
}

#[test]
fn test_rejects_empty_exception_fields() {
let mut processor = NormalizeProcessor::new(Arc::new(StoreConfig::default()), None);

let mut exception = Annotated::new(Exception {
value: Annotated::new("".to_string().into()),
ty: Annotated::new("".to_string()),
..Default::default()
});

process_value(&mut exception, &mut processor, ProcessingState::root());
assert!(exception.value().is_none());
assert!(exception.meta().has_errors());
}

#[test]
fn test_json_value() {
let mut processor = NormalizeProcessor::new(Arc::new(StoreConfig::default()), None);
Expand Down
10 changes: 5 additions & 5 deletions general/src/store/normalize/contexts.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use regex::Regex;

use crate::protocol::{Context, LenientString, OsContext, RuntimeContext};
use crate::types::{Object, Value};
use crate::protocol::{Context, OsContext, RuntimeContext};
use crate::types::{Empty, Object, Value};

lazy_static::lazy_static! {
/// Environment.OSVersion (GetVersionEx) or RuntimeInformation.OSDescription on Windows
Expand All @@ -19,7 +19,7 @@ lazy_static::lazy_static! {
}

fn normalize_runtime_context(runtime: &mut RuntimeContext) {
if runtime.name.value().is_none() && runtime.version.value().is_none() {
if runtime.name.value().is_empty() && runtime.version.value().is_empty() {
if let Some(raw_description) = runtime.raw_description.as_str() {
if let Some(captures) = RUNTIME_DOTNET_REGEX.captures(raw_description) {
runtime.name = captures.name("name").map(|m| m.as_str().to_string()).into();
Expand Down Expand Up @@ -85,7 +85,7 @@ fn normalize_os_context(os: &mut OsContext) {
.into();
os.build = captures
.name("build")
.map(|m| LenientString(m.as_str().to_string()))
.map(|m| m.as_str().to_string().into())
.into();
} else if let Some(captures) = OS_UNAME_REGEX.captures(raw_description) {
os.name = captures.name("name").map(|m| m.as_str().to_string()).into();
Expand Down Expand Up @@ -115,7 +115,7 @@ pub fn normalize_context(context: &mut Context) {
}

#[cfg(test)]
use crate::types::Annotated;
use crate::{protocol::LenientString, types::Annotated};

#[test]
fn test_arbitrary_type() {
Expand Down
6 changes: 3 additions & 3 deletions general/src/store/normalize/stacktrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::mem;
use url::Url;

use crate::protocol::{Frame, Stacktrace};
use crate::types::{Annotated, Array, Meta};
use crate::types::{Annotated, Array, Empty, Meta};

fn is_url(filename: &str) -> bool {
filename.starts_with("file:")
Expand All @@ -22,11 +22,11 @@ pub fn process_non_raw_stacktrace(stacktrace: &mut Stacktrace, _meta: &mut Meta)
}

pub fn process_non_raw_frame(frame: &mut Frame, _meta: &mut Meta) {
if frame.abs_path.value().map_or(true, |p| p.is_empty()) {
if frame.abs_path.value().is_empty() {
frame.abs_path = mem::replace(&mut frame.filename, Annotated::empty());
}

if frame.filename.value().map_or(true, |p| p.is_empty()) {
if frame.filename.value().is_empty() {
if let Some(abs_path) = frame.abs_path.value_mut() {
frame.filename = Annotated::new(abs_path.clone());

Expand Down
40 changes: 40 additions & 0 deletions general/src/types/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,36 @@ impl Empty for f64 {

derive_string_meta_structure!(Uuid, "a uuid");

impl<T> Empty for &'_ T
where
T: Empty,
{
#[inline]
fn is_empty(&self) -> bool {
(*self).is_empty()
}

#[inline]
fn is_deep_empty(&self) -> bool {
(*self).is_deep_empty()
}
}

impl<T> Empty for Option<T>
where
T: Empty,
{
#[inline]
fn is_empty(&self) -> bool {
self.as_ref().map_or(true, Empty::is_empty)
}

#[inline]
fn is_deep_empty(&self) -> bool {
self.as_ref().map_or(true, Empty::is_deep_empty)
}
}

impl<T> Empty for Array<T>
where
T: Empty,
Expand Down Expand Up @@ -490,14 +520,24 @@ where
}
}

/// Checks whether annotated data contains either a value or meta data.
impl<T> Empty for Annotated<T>
where
T: Empty,
{
/// Returns if this object contains data to be serialized.
///
/// **Caution:** This has different behavior than `annotated.value().is_empty()`! Annotated is
/// **not** empty if there is meta data that needs to be serialized. This is in line with the
/// derived implementation of `Empty` on structs, which calls `Annotated::is_empty` on every
/// child.
///
/// To check if a value is missing or empty, use `Option::is_empty` on the value instead.
fn is_empty(&self) -> bool {
self.skip_serialization(SkipSerialization::Empty(false))
}

/// Returns if this object contains nested data to be serialized.
fn is_deep_empty(&self) -> bool {
self.skip_serialization(SkipSerialization::Empty(true))
}
Expand Down

0 comments on commit 1284589

Please sign in to comment.