Jonas Schäfer created
It was broken in multiple ways:
- xso did not honour it: unknown children and attributes would cause a
parse error even with `--features disable-validation` set on parsers.
For this, we introduce a new feature flag on xso, `non-pedantic`,
which defaults unknown children and attributes to discard instead of
fail.
Note that individual XSOs can still choose to be always pedantic or
always lenient by explicitly declaring the intent via the
`on_unknown_child` and `on_unknown_attribute` metas.
- Many tests in `xmpp_parsers` were broken with `--features
disable-validation`. They now all pass while *still* being rn with
`disable-validation` set: In that case, they test that parsing in fact
succeeds.parsers/Cargo.toml | 2 +-
parsers/src/bind.rs | 1 +
parsers/src/blocking.rs | 1 +
parsers/src/bob.rs | 1 +
parsers/src/data_forms_validate.rs | 8 +++++---
parsers/src/delay.rs | 1 +
parsers/src/disco.rs | 1 +
parsers/src/ecaps2.rs | 1 +
parsers/src/eme.rs | 1 +
parsers/src/forwarding.rs | 3 ++-
parsers/src/hashes.rs | 1 +
parsers/src/idle.rs | 1 +
parsers/src/jingle.rs | 19 +++++++++----------
parsers/src/jingle_ft.rs | 26 +++++++++++++++++++++++---
parsers/src/media_element.rs | 1 +
parsers/src/message_correct.rs | 1 +
parsers/src/muc/muc.rs | 1 +
parsers/src/muc/user.rs | 2 ++
parsers/src/occupant_id.rs | 1 +
parsers/src/roster.rs | 9 ++++++++-
parsers/src/sasl.rs | 2 +-
parsers/src/stanza_id.rs | 1 +
parsers/src/util/macro_tests.rs | 24 ++++++++++++++++++++++++
xso/Cargo.toml | 1 +
xso/src/lib.rs | 18 ++++++++++++++----
25 files changed, 104 insertions(+), 24 deletions(-)
@@ -32,7 +32,7 @@ uuid = { version = "1.9.1", features = ["v4"] }
# Build xmpp-parsers to make components instead of clients.
component = []
# Disable validation of unknown attributes.
-disable-validation = []
+disable-validation = [ "xso/non-pedantic" ]
# Enable serde support in jid crate
serde = [ "jid/serde" ]
# Enable some additional logging in helpers
@@ -77,6 +77,7 @@ impl From<BindResponse> for Jid {
mod tests {
use super::*;
use minidom::Element;
+ #[cfg(not(feature = "disable-validation"))]
use xso::error::{Error, FromElementError};
#[cfg(target_pointer_width = "32")]
@@ -62,6 +62,7 @@ pub struct Blocked;
#[cfg(test)]
mod tests {
+ #[cfg(not(feature = "disable-validation"))]
use xso::error::{Error, FromElementError};
use super::*;
@@ -194,6 +194,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn unknown_child() {
let elem: Element = "<data xmlns='urn:xmpp:bob' cid='sha1+8f35fef110ffc5df08d579a50083ff9308fb6242@bob.xmpp.org'><coucou/></data>"
.parse()
@@ -448,7 +448,11 @@ mod tests {
}
#[test]
- fn test_fails_with_invalid_children() -> Result<(), Error> {
+ #[cfg_attr(
+ feature = "disable-validation",
+ should_panic = "Validate::try_from(element).is_err()"
+ )]
+ fn test_fails_with_invalid_children() {
let cases = [
r#"<validate xmlns='http://jabber.org/protocol/xdata-validate'><basic /><open /></validate>"#,
r#"<validate xmlns='http://jabber.org/protocol/xdata-validate'><unknown /></validate>"#,
@@ -460,7 +464,5 @@ mod tests {
.expect(&format!("Failed to parse {}", case));
assert!(Validate::try_from(element).is_err());
}
-
- Ok(())
}
}
@@ -81,6 +81,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
let elem: Element =
"<delay xmlns='urn:xmpp:delay' stamp='2002-09-10T23:08:25+00:00'><coucou/></delay>"
@@ -258,6 +258,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid() {
let elem: Element =
"<query xmlns='http://jabber.org/protocol/disco#info'><coucou/></query>"
@@ -219,6 +219,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
let elem: Element = "<c xmlns='urn:xmpp:caps'><hash xmlns='urn:xmpp:hashes:2' algo='sha-256'>K1Njy3HZBThlo4moOD5gBGhn0U0oK7/CbfLlIUDi6o4=</hash><hash xmlns='urn:xmpp:hashes:1' algo='sha3-256'>+sDTQqBmX6iG/X3zjt06fjZMBBqL/723knFIyRf0sg8=</hash></c>".parse().unwrap();
let error = ECaps2::try_from(elem).unwrap_err();
@@ -72,6 +72,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
let elem: Element =
"<encryption xmlns='urn:xmpp:eme:0' namespace='urn:xmpp:otr:0'><coucou/></encryption>"
@@ -56,8 +56,9 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
- let elem: Element = "<forwarded xmlns='urn:xmpp:forward:0'><coucou/></forwarded>"
+ let elem: Element = "<forwarded xmlns='urn:xmpp:forward:0'><message xmlns='jabber:client'/><coucou/></forwarded>"
.parse()
.unwrap();
let error = Forwarded::try_from(elem).unwrap_err();
@@ -305,6 +305,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
let elem: Element = "<hash xmlns='urn:xmpp:hashes:2' algo='sha-1'><coucou/></hash>"
.parse()
@@ -42,6 +42,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
let elem: Element =
"<idle xmlns='urn:xmpp:idle:1' since='2017-05-21T20:19:55+01:00'><coucou/></idle>"
@@ -754,7 +754,7 @@ mod tests {
}
#[test]
- fn test_invalid_reason() {
+ fn test_missing_reason_text() {
let elem: Element = "<jingle xmlns='urn:xmpp:jingle:1' action='session-initiate' sid='coucou'><reason/></jingle>".parse().unwrap();
let error = Jingle::try_from(elem).unwrap_err();
let message = match error {
@@ -765,23 +765,22 @@ mod tests {
message,
"Missing child field 'reason' in ReasonElement element."
);
+ }
- let elem: Element = "<jingle xmlns='urn:xmpp:jingle:1' action='session-initiate' sid='coucou'><reason><a/></reason></jingle>".parse().unwrap();
- let error = Jingle::try_from(elem).unwrap_err();
- let message = match error {
- FromElementError::Invalid(Error::Other(string)) => string,
- _ => panic!(),
- };
- assert_eq!(message, "Unknown child in ReasonElement element.");
-
- let elem: Element = "<jingle xmlns='urn:xmpp:jingle:1' action='session-initiate' sid='coucou'><reason><a xmlns='http://www.w3.org/1999/xhtml'/></reason></jingle>".parse().unwrap();
+ #[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
+ fn test_invalid_child_in_reason() {
+ let elem: Element = "<jingle xmlns='urn:xmpp:jingle:1' action='session-initiate' sid='coucou'><reason><decline/><a/></reason></jingle>".parse().unwrap();
let error = Jingle::try_from(elem).unwrap_err();
let message = match error {
FromElementError::Invalid(Error::Other(string)) => string,
_ => panic!(),
};
assert_eq!(message, "Unknown child in ReasonElement element.");
+ }
+ #[test]
+ fn test_multiple_reason_children() {
let elem: Element = "<jingle xmlns='urn:xmpp:jingle:1' action='session-initiate' sid='coucou'><reason><decline/></reason><reason/></jingle>".parse().unwrap();
let error = Jingle::try_from(elem).unwrap_err();
let message = match error {
@@ -304,7 +304,7 @@ mod tests {
}
#[test]
- fn test_received() {
+ fn test_received_valid() {
let elem: Element = "<received xmlns='urn:xmpp:jingle:apps:file-transfer:5' name='coucou' creator='initiator'/>".parse().unwrap();
let received = Received::try_from(elem).unwrap();
assert_eq!(received.name, ContentId(String::from("coucou")));
@@ -313,7 +313,11 @@ mod tests {
let received2 = Received::try_from(elem2).unwrap();
assert_eq!(received2.name, ContentId(String::from("coucou")));
assert_eq!(received2.creator, Creator::Initiator);
+ }
+ #[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
+ fn test_received_unknown_child() {
let elem: Element = "<received xmlns='urn:xmpp:jingle:apps:file-transfer:5' name='coucou' creator='initiator'><coucou/></received>".parse().unwrap();
let error = Received::try_from(elem).unwrap_err();
let message = match error {
@@ -321,7 +325,10 @@ mod tests {
_ => panic!(),
};
assert_eq!(message, "Unknown child in Received element.");
+ }
+ #[test]
+ fn test_received_missing_name() {
let elem: Element =
"<received xmlns='urn:xmpp:jingle:apps:file-transfer:5' creator='initiator'/>"
.parse()
@@ -335,7 +342,10 @@ mod tests {
message,
"Required attribute field 'name' on Received element missing."
);
+ }
+ #[test]
+ fn test_received_invalid_creator() {
let elem: Element = "<received xmlns='urn:xmpp:jingle:apps:file-transfer:5' name='coucou' creator='coucou'/>".parse().unwrap();
let error = Received::try_from(elem).unwrap_err();
let message = match error {
@@ -361,7 +371,7 @@ mod tests {
}
#[test]
- fn test_checksum() {
+ fn test_checksum_valid() {
let elem: Element = "<checksum xmlns='urn:xmpp:jingle:apps:file-transfer:5' name='coucou' creator='initiator'><file><hash xmlns='urn:xmpp:hashes:2' algo='sha-1'>w0mcJylzCn+AfvuGdqkty2+KP48=</hash></file></checksum>".parse().unwrap();
let hash = vec![
195, 73, 156, 39, 41, 115, 10, 127, 128, 126, 251, 134, 118, 169, 45, 203, 111, 138,
@@ -388,15 +398,22 @@ mod tests {
hash: hash.clone()
})
);
+ }
- let elem: Element = "<checksum xmlns='urn:xmpp:jingle:apps:file-transfer:5' name='coucou' creator='initiator'><coucou/></checksum>".parse().unwrap();
+ #[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
+ fn test_checksum_unknown_child() {
+ let elem: Element = "<checksum xmlns='urn:xmpp:jingle:apps:file-transfer:5' name='coucou' creator='initiator'><file><hash xmlns='urn:xmpp:hashes:2' algo='sha-1'>w0mcJylzCn+AfvuGdqkty2+KP48=</hash></file><coucou/></checksum>".parse().unwrap();
let error = Checksum::try_from(elem).unwrap_err();
let message = match error {
FromElementError::Invalid(Error::Other(string)) => string,
other => panic!("unexpected error: {:?}", other),
};
assert_eq!(message, "Unknown child in Checksum element.");
+ }
+ #[test]
+ fn test_checksum_missing_name() {
let elem: Element = "<checksum xmlns='urn:xmpp:jingle:apps:file-transfer:5' creator='initiator'><file><hash xmlns='urn:xmpp:hashes:2' algo='sha-1'>w0mcJylzCn+AfvuGdqkty2+KP48=</hash></file></checksum>".parse().unwrap();
let error = Checksum::try_from(elem).unwrap_err();
let message = match error {
@@ -407,7 +424,10 @@ mod tests {
message,
"Required attribute field 'name' on Checksum element missing."
);
+ }
+ #[test]
+ fn test_checksum_invalid_creator() {
let elem: Element = "<checksum xmlns='urn:xmpp:jingle:apps:file-transfer:5' name='coucou' creator='coucou'><file><hash xmlns='urn:xmpp:hashes:2' algo='sha-1'>w0mcJylzCn+AfvuGdqkty2+KP48=</hash></file></checksum>".parse().unwrap();
let error = Checksum::try_from(elem).unwrap_err();
let message = match error {
@@ -154,6 +154,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_unknown_child() {
let elem: Element = "<media xmlns='urn:xmpp:media-element'><coucou/></media>"
.parse()
@@ -62,6 +62,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
let elem: Element =
"<replace xmlns='urn:xmpp:message-correct:0' id='coucou'><coucou/></replace>"
@@ -113,6 +113,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_muc_invalid_child() {
let elem: Element = "<x xmlns='http://jabber.org/protocol/muc'><coucou/></x>"
.parse()
@@ -317,6 +317,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
let elem: Element = "<x xmlns='http://jabber.org/protocol/muc#user'>
<coucou/>
@@ -490,6 +491,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_continue_invalid() {
let elem: Element =
"<continue xmlns='http://jabber.org/protocol/muc#user'><foobar/></continue>"
@@ -53,6 +53,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
let elem: Element =
"<occupant-id xmlns='urn:xmpp:occupant-id:0' id='foo'><coucou/></occupant-id>"
@@ -289,7 +289,7 @@ mod tests {
}
#[test]
- fn test_invalid_item() {
+ fn test_item_missing_jid() {
let elem: Element = "<query xmlns='jabber:iq:roster'><item/></query>"
.parse()
.unwrap();
@@ -302,7 +302,10 @@ mod tests {
message,
"Required attribute field 'jid' on Item element missing."
);
+ }
+ #[test]
+ fn test_item_invalid_jid() {
let elem: Element = "<query xmlns='jabber:iq:roster'><item jid=''/></query>"
.parse()
.unwrap();
@@ -311,7 +314,11 @@ mod tests {
format!("{error}"),
"text parse error: no domain found in this JID"
);
+ }
+ #[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
+ fn test_item_unknown_child() {
let elem: Element =
"<query xmlns='jabber:iq:roster'><item jid='coucou'><coucou/></item></query>"
.parse()
@@ -157,7 +157,7 @@ pub struct Failure {
/// A human-readable explanation for the failure.
#[xml(extract(n = .., name = "text", fields(
- attribute(type_ = String, name = "xml:lang"),
+ attribute(type_ = String, name = "xml:lang", default),
text(type_ = String),
)))]
pub texts: BTreeMap<Lang, String>,
@@ -76,6 +76,7 @@ mod tests {
}
#[test]
+ #[cfg_attr(feature = "disable-validation", should_panic = "Result::unwrap_err")]
fn test_invalid_child() {
let elem: Element =
"<stanza-id xmlns='urn:xmpp:sid:0' by='a@b' id='x'><coucou/></stanza-id>"
@@ -121,6 +121,10 @@ fn empty_namespace_mismatch() {
}
#[test]
+#[cfg_attr(
+ feature = "disable-validation",
+ should_panic = "unexpected result: Ok("
+)]
fn empty_unexpected_attribute() {
#[allow(unused_imports)]
use core::{
@@ -136,6 +140,10 @@ fn empty_unexpected_attribute() {
}
#[test]
+#[cfg_attr(
+ feature = "disable-validation",
+ should_panic = "unexpected result: Ok("
+)]
fn empty_unexpected_child() {
#[allow(unused_imports)]
use core::{
@@ -869,6 +877,10 @@ fn text_extract_negative_absent_child() {
}
#[test]
+#[cfg_attr(
+ feature = "disable-validation",
+ should_panic = "unexpected result: Ok("
+)]
fn text_extract_negative_unexpected_attribute_in_child() {
#[allow(unused_imports)]
use core::{
@@ -886,6 +898,10 @@ fn text_extract_negative_unexpected_attribute_in_child() {
}
#[test]
+#[cfg_attr(
+ feature = "disable-validation",
+ should_panic = "unexpected result: Ok("
+)]
fn text_extract_negative_unexpected_child_in_child() {
#[allow(unused_imports)]
use core::{
@@ -1807,6 +1823,10 @@ fn ignore_unknown_attributes_positive() {
}
#[test]
+#[cfg_attr(
+ feature = "disable-validation",
+ should_panic = "unexpected result: Ok("
+)]
fn ignore_unknown_attributes_negative_unexpected_child() {
#[allow(unused_imports)]
use core::{
@@ -1849,6 +1869,10 @@ fn ignore_unknown_children_positive() {
}
#[test]
+#[cfg_attr(
+ feature = "disable-validation",
+ should_panic = "unexpected result: Ok("
+)]
fn ignore_unknown_children_negative_unexpected_attribute() {
#[allow(unused_imports)]
use core::{
@@ -29,6 +29,7 @@ default = [ "std" ]
macros = [ "dep:xso_proc", "rxml/macros" ]
minidom = [ "xso_proc/minidom"]
panicking-into-impl = ["xso_proc/panicking-into-impl"]
+non-pedantic = []
std = []
[package.metadata.docs.rs]
@@ -324,13 +324,18 @@ impl<T: AsXmlText> AsOptionalXmlText for Option<T> {
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Default)]
pub enum UnknownAttributePolicy {
/// All unknown attributes are discarded.
+ ///
+ /// This is the default policy if the crate is built with the
+ /// `non-pedantic` feature.
+ #[cfg_attr(feature = "non-pedantic", default)]
Discard,
/// The first unknown attribute which is encountered generates a fatal
/// parsing error.
///
- /// This is the default policy.
- #[default]
+ /// This is the default policy if the crate is built **without** the
+ /// `non-pedantic` feature.
+ #[cfg_attr(not(feature = "non-pedantic"), default)]
Fail,
}
@@ -356,13 +361,18 @@ impl UnknownAttributePolicy {
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Default)]
pub enum UnknownChildPolicy {
/// All unknown children are discarded.
+ ///
+ /// This is the default policy if the crate is built with the
+ /// `non-pedantic` feature.
+ #[cfg_attr(feature = "non-pedantic", default)]
Discard,
/// The first unknown child which is encountered generates a fatal
/// parsing error.
///
- /// This is the default policy.
- #[default]
+ /// This is the default policy if the crate is built **without** the
+ /// `non-pedantic` feature.
+ #[cfg_attr(not(feature = "non-pedantic"), default)]
Fail,
}