diff --git a/parsers/src/blocking.rs b/parsers/src/blocking.rs index 464b13948ff154a1cb7b387d1229edd3812129ee..8acb588a8d6c9148f317d6c4380dec80d0b7369b 100644 --- a/parsers/src/blocking.rs +++ b/parsers/src/blocking.rs @@ -37,7 +37,7 @@ macro_rules! generate_blocking_element { check_no_attributes!(elem, $name); let mut items = vec!(); for child in elem.children() { - check_self!(child, "item", BLOCKING); + check_child!(child, "item", BLOCKING); check_no_unknown_attributes!(child, "item", ["jid"]); check_no_children!(child, "item"); items.push(get_attr!(child, "jid", Required)); diff --git a/parsers/src/delay.rs b/parsers/src/delay.rs index 64d7f49dec1f718bc9d1bcb95242c21a8499f659..a38f4ec2c73cc02f5de86b820fa444b818f72bf6 100644 --- a/parsers/src/delay.rs +++ b/parsers/src/delay.rs @@ -69,12 +69,12 @@ mod tests { let elem: Element = "" .parse() .unwrap(); - let error = Delay::try_from(elem).unwrap_err(); - let message = match error { - Error::ParseError(string) => string, + let error = Delay::try_from(elem.clone()).unwrap_err(); + let returned_elem = match error { + Error::TypeMismatch(_, _, elem) => elem, _ => panic!(), }; - assert_eq!(message, "This is not a delay element."); + assert_eq!(elem, returned_elem); } #[test] diff --git a/parsers/src/eme.rs b/parsers/src/eme.rs index 5a624a30f98a224ede3da611fb3b088f535fd86d..256b878da11a50ef2a71d7ba077dd94800a48675 100644 --- a/parsers/src/eme.rs +++ b/parsers/src/eme.rs @@ -59,12 +59,12 @@ mod tests { let elem: Element = "" .parse() .unwrap(); - let error = ExplicitMessageEncryption::try_from(elem).unwrap_err(); - let message = match error { - Error::ParseError(string) => string, + let error = ExplicitMessageEncryption::try_from(elem.clone()).unwrap_err(); + let returned_elem = match error { + Error::TypeMismatch(_, _, elem) => elem, _ => panic!(), }; - assert_eq!(message, "This is not a encryption element."); + assert_eq!(elem, returned_elem); } #[test] diff --git a/parsers/src/hashes.rs b/parsers/src/hashes.rs index c79a71dd887ea6a82d01a548fb9226a6deb268b1..379f34cd8c4f7b5047d4f5bfd14919bead164310 100644 --- a/parsers/src/hashes.rs +++ b/parsers/src/hashes.rs @@ -253,12 +253,12 @@ mod tests { let elem: Element = "" .parse() .unwrap(); - let error = Hash::try_from(elem).unwrap_err(); - let message = match error { - Error::ParseError(string) => string, + let error = Hash::try_from(elem.clone()).unwrap_err(); + let returned_elem = match error { + Error::TypeMismatch(_, _, elem) => elem, _ => panic!(), }; - assert_eq!(message, "This is not a hash element."); + assert_eq!(elem, returned_elem); } #[test] diff --git a/parsers/src/jingle_ft.rs b/parsers/src/jingle_ft.rs index 67c7a98be55adb75e7caa32513fe709066bf46b6..e5a6d8158f6485b949326e5237083b999ea36009 100644 --- a/parsers/src/jingle_ft.rs +++ b/parsers/src/jingle_ft.rs @@ -289,7 +289,7 @@ impl TryFrom for Checksum { "JingleFT checksum element must have exactly one child.", )); } - file = Some(File::try_from(child.clone())?); + file = Some(File::try_from(child.clone()).map_err(|e| e.hide_type_mismatch())?); } if file.is_none() { return Err(Error::ParseError( @@ -541,9 +541,9 @@ mod tests { let error = Checksum::try_from(elem).unwrap_err(); let message = match error { Error::ParseError(string) => string, - _ => panic!(), + other => panic!("unexpected error: {:?}", other), }; - assert_eq!(message, "This is not a file element."); + assert_eq!(message, "Unexpected child element"); let elem: Element = "w0mcJylzCn+AfvuGdqkty2+KP48=".parse().unwrap(); let error = Checksum::try_from(elem).unwrap_err(); diff --git a/parsers/src/rsm.rs b/parsers/src/rsm.rs index af3aa744d5bc0bdb19cabe8c223088b70b506b5f..bb5adf3819735d1f87f583ac26a6467116badfce 100644 --- a/parsers/src/rsm.rs +++ b/parsers/src/rsm.rs @@ -213,22 +213,22 @@ mod tests { let elem: Element = "" .parse() .unwrap(); - let error = SetQuery::try_from(elem).unwrap_err(); - let message = match error { - Error::ParseError(string) => string, + let error = SetQuery::try_from(elem.clone()).unwrap_err(); + let returned_elem = match error { + Error::TypeMismatch(_, _, elem) => elem, _ => panic!(), }; - assert_eq!(message, "This is not a RSM set element."); + assert_eq!(elem, returned_elem); let elem: Element = "" .parse() .unwrap(); - let error = SetResult::try_from(elem).unwrap_err(); - let message = match error { - Error::ParseError(string) => string, + let error = SetResult::try_from(elem.clone()).unwrap_err(); + let returned_elem = match error { + Error::TypeMismatch(_, _, elem) => elem, _ => panic!(), }; - assert_eq!(message, "This is not a RSM set element."); + assert_eq!(elem, returned_elem); } #[test] diff --git a/parsers/src/util/error.rs b/parsers/src/util/error.rs index a2c6ea752556ef013b6c93be6c2ae87e6a6b0b29..b541266ef86a3082605e4e3658641e823c87f036 100644 --- a/parsers/src/util/error.rs +++ b/parsers/src/util/error.rs @@ -17,6 +17,12 @@ pub enum Error { /// of a freeform string. ParseError(&'static str), + /// Element local-name/namespace mismatch + /// + /// Returns the original element unaltered, as well as the expected ns and + /// local-name. + TypeMismatch(&'static str, &'static str, crate::Element), + /// Generated when some base64 content fails to decode, usually due to /// extra characters. Base64Error(base64::DecodeError), @@ -40,10 +46,24 @@ pub enum Error { ChronoParseError(chrono::ParseError), } +impl Error { + /// Converts the TypeMismatch error to a generic ParseError + /// + /// This must be used when TryFrom is called on children to avoid confusing + /// user code which assumes that TypeMismatch refers to the top level + /// element only. + pub(crate) fn hide_type_mismatch(self) -> Self { + match self { + Error::TypeMismatch(..) => Error::ParseError("Unexpected child element"), + other => other, + } + } +} + impl StdError for Error { fn cause(&self) -> Option<&dyn StdError> { match self { - Error::ParseError(_) => None, + Error::ParseError(_) | Error::TypeMismatch(..) => None, Error::Base64Error(e) => Some(e), Error::ParseIntError(e) => Some(e), Error::ParseStringError(e) => Some(e), @@ -58,6 +78,14 @@ impl fmt::Display for Error { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { match self { Error::ParseError(s) => write!(fmt, "parse error: {}", s), + Error::TypeMismatch(ns, localname, element) => write!( + fmt, + "element type mismatch: expected {{{}}}{}, got {{{}}}{}", + ns, + localname, + element.ns(), + element.name() + ), Error::Base64Error(e) => write!(fmt, "base64 error: {}", e), Error::ParseIntError(e) => write!(fmt, "integer parsing error: {}", e), Error::ParseStringError(e) => write!(fmt, "string parsing error: {}", e), diff --git a/parsers/src/util/macros.rs b/parsers/src/util/macros.rs index 996d3e22f010c23165c38b6feadb35d05ee1c3e7..8815654c6ff246c5aef65e1b7c0ee4c7e5d3e4ba 100644 --- a/parsers/src/util/macros.rs +++ b/parsers/src/util/macros.rs @@ -292,6 +292,21 @@ macro_rules! check_self { ($elem:ident, $name:tt, $ns:ident) => { check_self!($elem, $name, $ns, $name); }; + ($elem:ident, $name:tt, $ns:ident, $pretty_name:tt) => { + if !$elem.is($name, crate::ns::$ns) { + return Err(crate::util::error::Error::TypeMismatch( + $name, + crate::ns::$ns, + $elem, + )); + } + }; +} + +macro_rules! check_child { + ($elem:ident, $name:tt, $ns:ident) => { + check_child!($elem, $name, $ns, $name); + }; ($elem:ident, $name:tt, $ns:ident, $pretty_name:tt) => { if !$elem.is($name, crate::ns::$ns) { return Err(crate::util::error::Error::ParseError(concat!(