From 4853776010c9ac8df66faf9629d6324effa60648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=A4fer?= Date: Thu, 9 May 2024 10:35:43 +0200 Subject: [PATCH] data_forms: ignore incorrect `FORM_TYPE` fields as per XEP-0068 XEP-0068 is rather explicit that `FORM_TYPE` fields which are not `type='hidden'` MUST be ignored (in most cases, see comments inside the code for exceptions). The previous implementation returned an error instead (and aborted parsing with that), which is obviously not "ignoring". --- parsers/src/data_forms.rs | 134 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 130 insertions(+), 4 deletions(-) diff --git a/parsers/src/data_forms.rs b/parsers/src/data_forms.rs index 6e7572ee..8fa597e4 100644 --- a/parsers/src/data_forms.rs +++ b/parsers/src/data_forms.rs @@ -120,6 +120,47 @@ impl Field { fn is_list(&self) -> bool { self.type_ == FieldType::ListSingle || self.type_ == FieldType::ListMulti } + + /// Return true if this field is a valid form type specifier as per + /// [XEP-0068](https://xmpp.org/extensions/xep-0068.html). + /// + /// This function requires knowledge of the form's type attribute as the + /// criteria differ slighly among form types. + pub fn is_form_type(&self, ty: &DataFormType) -> bool { + // 1. A field must have the var FORM_TYPE + if self.var != "FORM_TYPE" { + return false; + } + + match ty { + // https://xmpp.org/extensions/xep-0068.html#usecases-incorrect + // > If the FORM_TYPE field is not hidden in a form with + // > type="form" or type="result", it MUST be ignored as a context + // > indicator. + DataFormType::Form | DataFormType::Result_ => self.type_ == FieldType::Hidden, + + // https://xmpp.org/extensions/xep-0068.html#impl + // > Data forms with the type "submit" are free to omit any + // > explicit field type declaration (as per Data Forms (XEP-0004) + // > § 3.2), as the type is implied by the corresponding + // > "form"-type data form. As consequence, implementations MUST + // > treat a FORM_TYPE field without an explicit type attribute, + // > in data forms of type "submit", as the FORM_TYPE field with + // > the special meaning defined herein. + DataFormType::Submit => match self.type_ { + FieldType::Hidden => true, + FieldType::TextSingle => true, + _ => false, + }, + + // XEP-0068 does not explicitly mention cancel type forms. + // However, XEP-0004 states: + // > a data form of type "cancel" SHOULD NOT contain any + // > elements. + // thus we ignore those. + DataFormType::Cancel => false, + } + } } impl TryFrom for Field { @@ -276,14 +317,11 @@ impl TryFrom for DataForm { form.instructions = Some(child.text()); } else if child.is("field", ns::DATA_FORMS) { let field = Field::try_from(child.clone())?; - if field.var == "FORM_TYPE" { + if field.is_form_type(&form.type_) { let mut field = field; if form.form_type.is_some() { return Err(Error::ParseError("More than one FORM_TYPE in a data form.")); } - if field.type_ != FieldType::Hidden { - return Err(Error::ParseError("Invalid field type for FORM_TYPE.")); - } if field.values.len() != 1 { return Err(Error::ParseError("Wrong number of values in FORM_TYPE.")); } @@ -418,4 +456,92 @@ mod tests { "Element option must not have more than one value child." ); } + + #[test] + fn test_ignore_form_type_field_if_field_type_mismatches_in_form_typed_forms() { + // https://xmpp.org/extensions/xep-0068.html#usecases-incorrect + // […] it MUST be ignored as a context indicator + let elem: Element = "foo".parse().unwrap(); + match DataForm::try_from(elem) { + Ok(form) => { + match form.form_type { + None => (), + other => panic!("unexpected extracted form type: {:?}", other), + }; + } + other => panic!("unexpected result: {:?}", other), + } + } + + #[test] + fn test_ignore_form_type_field_if_field_type_mismatches_in_result_typed_forms() { + // https://xmpp.org/extensions/xep-0068.html#usecases-incorrect + // […] it MUST be ignored as a context indicator + let elem: Element = "foo".parse().unwrap(); + match DataForm::try_from(elem) { + Ok(form) => { + match form.form_type { + None => (), + other => panic!("unexpected extracted form type: {:?}", other), + }; + } + other => panic!("unexpected result: {:?}", other), + } + } + + #[test] + fn test_accept_form_type_field_without_type_attribute_in_submit_typed_forms() { + let elem: Element = "foo".parse().unwrap(); + match DataForm::try_from(elem) { + Ok(form) => { + match form.form_type { + Some(ty) => assert_eq!(ty, "foo"), + other => panic!("unexpected extracted form type: {:?}", other), + }; + } + other => panic!("unexpected result: {:?}", other), + } + } + + #[test] + fn test_accept_form_type_field_with_type_hidden_in_submit_typed_forms() { + let elem: Element = "foo".parse().unwrap(); + match DataForm::try_from(elem) { + Ok(form) => { + match form.form_type { + Some(ty) => assert_eq!(ty, "foo"), + other => panic!("unexpected extracted form type: {:?}", other), + }; + } + other => panic!("unexpected result: {:?}", other), + } + } + + #[test] + fn test_accept_form_type_field_with_type_hidden_in_result_typed_forms() { + let elem: Element = "foo".parse().unwrap(); + match DataForm::try_from(elem) { + Ok(form) => { + match form.form_type { + Some(ty) => assert_eq!(ty, "foo"), + other => panic!("unexpected extracted form type: {:?}", other), + }; + } + other => panic!("unexpected result: {:?}", other), + } + } + + #[test] + fn test_accept_form_type_field_with_type_hidden_in_form_typed_forms() { + let elem: Element = "foo".parse().unwrap(); + match DataForm::try_from(elem) { + Ok(form) => { + match form.form_type { + Some(ty) => assert_eq!(ty, "foo"), + other => panic!("unexpected extracted form type: {:?}", other), + }; + } + other => panic!("unexpected result: {:?}", other), + } + } }