From a93dad391c877789585164946f82d72d07f395d1 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 23 Jan 2019 11:14:17 +0100 Subject: [PATCH 1/5] fix: Do not allow too large tuples in tuple struct derive --- general/derive/src/lib.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/general/derive/src/lib.rs b/general/derive/src/lib.rs index b14cca7dd4..e0362bb323 100644 --- a/general/derive/src/lib.rs +++ b/general/derive/src/lib.rs @@ -475,6 +475,7 @@ fn derive_metastructure(s: synstructure::Structure<'_>, t: Trait) -> TokenStream } let mut is_tuple_struct = false; + for (index, bi) in variant.bindings().iter().enumerate() { let field_attrs = parse_field_attributes(index, &bi.ast(), &mut is_tuple_struct); let field_attrs_name = Ident::new(&format!("__field_attrs_{}", index), Span::call_site()); @@ -626,7 +627,7 @@ fn derive_metastructure(s: synstructure::Structure<'_>, t: Trait) -> TokenStream let ast = s.ast(); let expectation = LitStr::new( - &format!("expected {}", ast.ident.to_string().to_lowercase()), + &ast.ident.to_string().to_lowercase(), Span::call_site(), ); let mut variant = variant.clone(); @@ -661,12 +662,19 @@ fn derive_metastructure(s: synstructure::Structure<'_>, t: Trait) -> TokenStream } }), Trait::From => { + let bindings_count = variant.bindings().len(); let valid_match_arm = if is_tuple_struct { quote! { - crate::types::Annotated(Some(crate::types::Value::Array(mut __arr)), __meta) => { - let __arr = __arr.into_iter(); - #from_value_body; - crate::types::Annotated(Some(#to_structure_assemble_pat), __meta) + crate::types::Annotated(Some(crate::types::Value::Array(mut __arr)), mut __meta) => { + if __arr.len() != #bindings_count { + __meta.add_error(Error::expected(&format!("a {}-tuple", #bindings_count))); + __meta.set_original_value(Some(__arr)); + Annotated(None, __meta) + } else { + let mut __arr = __arr.into_iter(); + #from_value_body; + crate::types::Annotated(Some(#to_structure_assemble_pat), __meta) + } } } } else { From 27e69e1c5e5dfa210ea6b58a3dffb7eca606ebc3 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 23 Jan 2019 11:15:06 +0100 Subject: [PATCH 2/5] fix: Do not skip null values in pairlists --- general/src/protocol/request.rs | 41 ++++++++++----- general/src/protocol/tags.rs | 88 ++++++++++++++++++++++++++++----- 2 files changed, 105 insertions(+), 24 deletions(-) diff --git a/general/src/protocol/request.rs b/general/src/protocol/request.rs index 94092aed46..cf774d8232 100644 --- a/general/src/protocol/request.rs +++ b/general/src/protocol/request.rs @@ -6,11 +6,11 @@ use url::form_urlencoded; use crate::protocol::{JsonLenientString, LenientString, PairList}; use crate::types::{Annotated, Error, FromValue, Object, Value}; -type CookieEntry = Annotated<(Annotated, Annotated)>; +type CookieEntry = (Annotated, Annotated); /// A map holding cookies. #[derive(Clone, Debug, Default, PartialEq, Empty, ToValue, ProcessValue)] -pub struct Cookies(pub PairList<(Annotated, Annotated)>); +pub struct Cookies(#[metastructure(skip_serialization = "never")] pub PairList); impl Cookies { pub fn parse(string: &str) -> Result { @@ -18,14 +18,16 @@ impl Cookies { pairs.map(Cookies) } - fn iter_cookies<'a>(string: &'a str) -> impl Iterator> + 'a { + fn iter_cookies<'a>( + string: &'a str, + ) -> impl Iterator, Error>> + 'a { string .split(';') .filter(|cookie| !cookie.trim().is_empty()) .map(Cookies::parse_cookie) } - fn parse_cookie(string: &str) -> Result { + fn parse_cookie(string: &str) -> Result, Error> { match Cookie::parse_encoded(string) { Ok(cookie) => Ok(Annotated::from(( cookie.name().to_string().into(), @@ -37,7 +39,7 @@ impl Cookies { } impl std::ops::Deref for Cookies { - type Target = PairList<(Annotated, Annotated)>; + type Target = PairList; fn deref(&self) -> &Self::Target { &self.0 @@ -155,7 +157,9 @@ impl FromValue for HeaderName { /// A map holding headers. #[derive(Clone, Debug, Default, PartialEq, Empty, ToValue, ProcessValue)] -pub struct Headers(pub PairList<(Annotated, Annotated)>); +pub struct Headers(#[metastructure(skip_serialization = "never")] pub PairList
); + +type Header = (Annotated, Annotated); impl Headers { pub fn get_header(&self, key: &str) -> Option<&str> { @@ -172,7 +176,7 @@ impl Headers { } impl std::ops::Deref for Headers { - type Target = PairList<(Annotated, Annotated)>; + type Target = PairList
; fn deref(&self) -> &Self::Target { &self.0 @@ -192,8 +196,7 @@ impl FromValue for Headers { _ => false, // Preserve order if SDK sent headers as array }; - type HeaderTuple = (Annotated, Annotated); - PairList::::from_value(value).map_value(|mut pair_list| { + PairList::
::from_value(value).map_value(|mut pair_list| { if should_sort { pair_list.sort_unstable_by(|a, b| { a.value() @@ -209,7 +212,9 @@ impl FromValue for Headers { /// A map holding query string pairs. #[derive(Clone, Debug, Default, PartialEq, Empty, ToValue, ProcessValue)] -pub struct Query(pub PairList<(Annotated, Annotated)>); +pub struct Query(#[metastructure(skip_serialization = "never")] pub PairList); + +type QueryEntry = (Annotated, Annotated); impl Query { pub fn parse(mut string: &str) -> Self { @@ -222,7 +227,7 @@ impl Query { } impl std::ops::Deref for Query { - type Target = PairList<(Annotated, Annotated)>; + type Target = PairList; fn deref(&self) -> &Self::Target { &self.0 @@ -639,7 +644,8 @@ fn test_cookies_parsing() { #[test] fn test_cookies_array() { - let json = r#"[["foo", "bar"], ["invalid", 42]]"#; + let input = r#"{"cookies":[["foo","bar"],["invalid", 42],["none",null]]}"#; + let output = r#"{"cookies":[["foo","bar"],["invalid",null],["none",null]],"_meta":{"cookies":{"1":{"1":{"":{"err":[["invalid_data",{"reason":"expected a string"}]],"val":42}}}}}}"#; let mut map = Vec::new(); map.push(Annotated::new(( @@ -650,9 +656,18 @@ fn test_cookies_array() { Annotated::new("invalid".to_string()), Annotated::from_error(Error::expected("a string"), Some(Value::U64(42))), ))); + map.push(Annotated::new(( + Annotated::new("none".to_string()), + Annotated::empty(), + ))); let cookies = Annotated::new(Cookies(PairList(map))); - assert_eq_dbg!(cookies, Annotated::from_json(json).unwrap()); + let request = Annotated::new(Request { + cookies, + ..Default::default() + }); + assert_eq_dbg!(request, Annotated::from_json(input).unwrap()); + assert_eq_dbg!(request.to_json().unwrap(), output); } #[test] diff --git a/general/src/protocol/tags.rs b/general/src/protocol/tags.rs index 5b0675a6c1..a2925a900d 100644 --- a/general/src/protocol/tags.rs +++ b/general/src/protocol/tags.rs @@ -6,11 +6,17 @@ pub struct TagEntry( #[metastructure( pii = "true", max_chars = "tag_key", - match_regex = r"^[a-zA-Z0-9_\.:-]+\z" + match_regex = r"^[a-zA-Z0-9_\.:-]+\z", + skip_serialization = "never" + )] + pub Annotated, + #[metastructure( + pii = "true", + max_chars = "tag_value", + match_regex = r"^[^\n]+\z", + skip_serialization = "never" )] pub Annotated, - #[metastructure(pii = "true", max_chars = "tag_value", match_regex = r"^[^\n]+\z")] - pub Annotated, ); impl AsPair for TagEntry { @@ -70,10 +76,15 @@ fn test_tags_from_object() { "blah": "blub", "bool": true, "foo bar": "baz", - "non string": 42 + "non string": 42, + "bam": null }"#; let mut arr = Array::new(); + arr.push(Annotated::new(TagEntry( + Annotated::new("bam".to_string()), + Annotated::empty(), + ))); arr.push(Annotated::new(TagEntry( Annotated::new("blah".to_string()), Annotated::new("blub".to_string()), @@ -97,12 +108,57 @@ fn test_tags_from_object() { #[test] fn test_tags_from_array() { - let json = r#"[ - ["bool", true], - ["foo bar", "baz"], - [23, 42], - ["blah", "blub"] -]"#; + use crate::protocol::Event; + + let input = r#"{ + "tags": [ + [ + "bool", + true + ], + [ + "foo bar", + "baz" + ], + [ + 23, + 42 + ], + [ + "blah", + "blub" + ], + [ + "bam", + null + ] + ] +}"#; + + let output = r#"{ + "tags": [ + [ + "bool", + "True" + ], + [ + "foo-bar", + "baz" + ], + [ + "23", + "42" + ], + [ + "blah", + "blub" + ], + [ + "bam", + null + ] + ] +}"#; let mut arr = Array::new(); arr.push(Annotated::new(TagEntry( @@ -121,7 +177,17 @@ fn test_tags_from_array() { Annotated::new("blah".to_string()), Annotated::new("blub".to_string()), ))); + arr.push(Annotated::new(TagEntry( + Annotated::new("bam".to_string()), + Annotated::empty(), + ))); let tags = Annotated::new(Tags(arr.into())); - assert_eq_dbg!(tags, Annotated::from_json(json).unwrap()); + let event = Annotated::new(Event { + tags, + ..Default::default() + }); + + assert_eq_dbg!(event, Annotated::from_json(input).unwrap()); + assert_eq_str!(event.to_json_pretty().unwrap(), output); } From 7c709bef13840598e666a7eaf2a77909ba38f141 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 23 Jan 2019 11:21:10 +0100 Subject: [PATCH 3/5] fix: Avoid formatting in derive --- general/derive/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/general/derive/src/lib.rs b/general/derive/src/lib.rs index e0362bb323..0374c9cef5 100644 --- a/general/derive/src/lib.rs +++ b/general/derive/src/lib.rs @@ -667,7 +667,7 @@ fn derive_metastructure(s: synstructure::Structure<'_>, t: Trait) -> TokenStream quote! { crate::types::Annotated(Some(crate::types::Value::Array(mut __arr)), mut __meta) => { if __arr.len() != #bindings_count { - __meta.add_error(Error::expected(&format!("a {}-tuple", #bindings_count))); + __meta.add_error(Error::expected(concat!("a ", stringify!(#bindings_count), "-tuple"))); __meta.set_original_value(Some(__arr)); Annotated(None, __meta) } else { From d133267eb7689c2b041e38b269f7c898549f5d3c Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 23 Jan 2019 12:17:24 +0100 Subject: [PATCH 4/5] fix: Fix the true issue --- general/derive/src/lib.rs | 5 ++++- general/src/protocol/request.rs | 27 +++++++++++---------------- general/src/protocol/tags.rs | 12 +++--------- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/general/derive/src/lib.rs b/general/derive/src/lib.rs index 0374c9cef5..4dce373cb1 100644 --- a/general/derive/src/lib.rs +++ b/general/derive/src/lib.rs @@ -1018,7 +1018,7 @@ impl SkipSerialization { impl Default for SkipSerialization { fn default() -> SkipSerialization { - SkipSerialization::Null(true) + SkipSerialization::Never } } @@ -1049,6 +1049,9 @@ fn parse_field_attributes( } let mut rv = FieldAttrs::default(); + if !*is_tuple_struct { + rv.skip_serialization = SkipSerialization::Null(false); + } rv.field_name = bi_ast .ident .as_ref() diff --git a/general/src/protocol/request.rs b/general/src/protocol/request.rs index cf774d8232..ff6354cbd8 100644 --- a/general/src/protocol/request.rs +++ b/general/src/protocol/request.rs @@ -6,11 +6,11 @@ use url::form_urlencoded; use crate::protocol::{JsonLenientString, LenientString, PairList}; use crate::types::{Annotated, Error, FromValue, Object, Value}; -type CookieEntry = (Annotated, Annotated); +type CookieEntry = Annotated<(Annotated, Annotated)>; /// A map holding cookies. #[derive(Clone, Debug, Default, PartialEq, Empty, ToValue, ProcessValue)] -pub struct Cookies(#[metastructure(skip_serialization = "never")] pub PairList); +pub struct Cookies(pub PairList<(Annotated, Annotated)>); impl Cookies { pub fn parse(string: &str) -> Result { @@ -18,16 +18,14 @@ impl Cookies { pairs.map(Cookies) } - fn iter_cookies<'a>( - string: &'a str, - ) -> impl Iterator, Error>> + 'a { + fn iter_cookies<'a>(string: &'a str) -> impl Iterator> + 'a { string .split(';') .filter(|cookie| !cookie.trim().is_empty()) .map(Cookies::parse_cookie) } - fn parse_cookie(string: &str) -> Result, Error> { + fn parse_cookie(string: &str) -> Result { match Cookie::parse_encoded(string) { Ok(cookie) => Ok(Annotated::from(( cookie.name().to_string().into(), @@ -39,7 +37,7 @@ impl Cookies { } impl std::ops::Deref for Cookies { - type Target = PairList; + type Target = PairList<(Annotated, Annotated)>; fn deref(&self) -> &Self::Target { &self.0 @@ -157,9 +155,7 @@ impl FromValue for HeaderName { /// A map holding headers. #[derive(Clone, Debug, Default, PartialEq, Empty, ToValue, ProcessValue)] -pub struct Headers(#[metastructure(skip_serialization = "never")] pub PairList
); - -type Header = (Annotated, Annotated); +pub struct Headers(pub PairList<(Annotated, Annotated)>); impl Headers { pub fn get_header(&self, key: &str) -> Option<&str> { @@ -176,7 +172,7 @@ impl Headers { } impl std::ops::Deref for Headers { - type Target = PairList
; + type Target = PairList<(Annotated, Annotated)>; fn deref(&self) -> &Self::Target { &self.0 @@ -196,7 +192,8 @@ impl FromValue for Headers { _ => false, // Preserve order if SDK sent headers as array }; - PairList::
::from_value(value).map_value(|mut pair_list| { + type HeaderTuple = (Annotated, Annotated); + PairList::::from_value(value).map_value(|mut pair_list| { if should_sort { pair_list.sort_unstable_by(|a, b| { a.value() @@ -212,9 +209,7 @@ impl FromValue for Headers { /// A map holding query string pairs. #[derive(Clone, Debug, Default, PartialEq, Empty, ToValue, ProcessValue)] -pub struct Query(#[metastructure(skip_serialization = "never")] pub PairList); - -type QueryEntry = (Annotated, Annotated); +pub struct Query(pub PairList<(Annotated, Annotated)>); impl Query { pub fn parse(mut string: &str) -> Self { @@ -227,7 +222,7 @@ impl Query { } impl std::ops::Deref for Query { - type Target = PairList; + type Target = PairList<(Annotated, Annotated)>; fn deref(&self) -> &Self::Target { &self.0 diff --git a/general/src/protocol/tags.rs b/general/src/protocol/tags.rs index a2925a900d..eab909ad32 100644 --- a/general/src/protocol/tags.rs +++ b/general/src/protocol/tags.rs @@ -6,17 +6,11 @@ pub struct TagEntry( #[metastructure( pii = "true", max_chars = "tag_key", - match_regex = r"^[a-zA-Z0-9_\.:-]+\z", - skip_serialization = "never" - )] - pub Annotated, - #[metastructure( - pii = "true", - max_chars = "tag_value", - match_regex = r"^[^\n]+\z", - skip_serialization = "never" + match_regex = r"^[a-zA-Z0-9_\.:-]+\z" )] pub Annotated, + #[metastructure(pii = "true", max_chars = "tag_value", match_regex = r"^[^\n]+\z")] + pub Annotated, ); impl AsPair for TagEntry { From d6c5c4b66c401ed3ddf87ddc660ed0855c4c7359 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 23 Jan 2019 13:18:10 +0100 Subject: [PATCH 5/5] fix: Linting --- general/derive/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/general/derive/src/lib.rs b/general/derive/src/lib.rs index 4dce373cb1..7b709da2de 100644 --- a/general/derive/src/lib.rs +++ b/general/derive/src/lib.rs @@ -626,10 +626,7 @@ fn derive_metastructure(s: synstructure::Structure<'_>, t: Trait) -> TokenStream } let ast = s.ast(); - let expectation = LitStr::new( - &ast.ident.to_string().to_lowercase(), - Span::call_site(), - ); + let expectation = LitStr::new(&ast.ident.to_string().to_lowercase(), Span::call_site()); let mut variant = variant.clone(); for binding in variant.bindings_mut() { binding.style = synstructure::BindStyle::Move;