data_forms: Stop duplicating FORM_TYPE in memory.

Emmanuel Gil Peyrot created 7 years ago

The FORM_TYPE is now only present once, as the form_type member of the
DataForm struct, it isn’t duplicated in fields anymore.

This removes the need to ignore this special field in every single
protocol built on XEP-0128.

Change summary

src/caps.rs       |  4 +---
src/data_forms.rs | 16 +++++++++++++---
src/ecaps2.rs     | 31 ++++++++++++++++++++-----------
src/ibr.rs        | 12 ------------
4 files changed, 34 insertions(+), 29 deletions(-)

Detailed changes

src/caps.rs

@@ -329,9 +329,7 @@ mod tests {
 "#
         .parse()
         .unwrap();
-        let data = b"client/pc/el/\xce\xa8 0.11<client/pc/en/Psi 0.11<http://jabber.org/protocol/caps<http://jabber.org/protocol/disco#info<http://jabber.org/protocol/disco#items<http://jabber.org/protocol/muc<urn:xmpp:dataforms:softwareinfo<ip_version<ipv4<ipv6<os<Mac<os_version<10.5.1<software<Psi<software_version<0.11<";
-        let mut expected = Vec::with_capacity(data.len());
-        expected.extend_from_slice(data);
+        let expected = b"client/pc/el/\xce\xa8 0.11<client/pc/en/Psi 0.11<http://jabber.org/protocol/caps<http://jabber.org/protocol/disco#info<http://jabber.org/protocol/disco#items<http://jabber.org/protocol/muc<urn:xmpp:dataforms:softwareinfo<ip_version<ipv4<ipv6<os<Mac<os_version<10.5.1<software<Psi<software_version<0.11<".to_vec();
         let disco = DiscoInfoResult::try_from(elem).unwrap();
         let caps = caps::compute_disco(&disco);
         assert_eq!(caps, expected);

src/data_forms.rs

@@ -247,16 +247,21 @@ impl TryFrom<Element> 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" && field.type_ == FieldType::Hidden {
+                if field.var == "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."));
                     }
-                    form.form_type = Some(field.values[0].clone());
+                    form.form_type = field.values.pop();
+                } else {
+                    form.fields.push(field);
                 }
-                form.fields.push(field);
             } else {
                 return Err(Error::ParseError("Unknown child in data form element."));
             }
@@ -279,6 +284,11 @@ impl From<DataForm> for Element {
                     .ns(ns::DATA_FORMS)
                     .append(text)
             }))
+            .append(if let Some(form_type) = form.form_type {
+                vec![Element::builder("field").ns(ns::DATA_FORMS).attr("var", "FORM_TYPE").attr("type", "hidden").append(Element::builder("value").ns(ns::DATA_FORMS).append(form_type).build()).build()]
+            } else {
+                vec![]
+            })
             .append(form.fields)
             .build()
     }

src/ecaps2.rs

@@ -69,31 +69,40 @@ fn compute_identities(identities: &[Identity]) -> Vec<u8> {
     })
 }
 
-fn compute_extensions(extensions: &[DataForm]) -> Vec<u8> {
-    compute_items(extensions, 0x1c, |extension| {
-        compute_items(&extension.fields, 0x1d, |field| {
+fn compute_extensions(extensions: &[DataForm]) -> Result<Vec<u8>, ()> {
+    for extension in extensions {
+        if extension.form_type.is_none() {
+            return Err(());
+        }
+    }
+    Ok(compute_items(extensions, 0x1c, |extension| {
+        let mut bytes = compute_item("FORM_TYPE");
+        bytes.append(&mut compute_item(if let Some(ref form_type) = extension.form_type { form_type } else { unreachable!() }));
+        bytes.push(0x1e);
+        bytes.append(&mut compute_items(&extension.fields, 0x1d, |field| {
             let mut bytes = compute_item(&field.var);
             bytes.append(&mut compute_items(&field.values, 0x1e, |value| {
                 compute_item(value)
             }));
             bytes
-        })
-    })
+        }));
+        bytes
+    }))
 }
 
 /// Applies the [algorithm from
 /// XEP-0390](https://xmpp.org/extensions/xep-0390.html#algorithm-input) on a
 /// [disco#info query element](../disco/struct.DiscoInfoResult.html).
-pub fn compute_disco(disco: &DiscoInfoResult) -> Vec<u8> {
+pub fn compute_disco(disco: &DiscoInfoResult) -> Result<Vec<u8>, ()> {
     let features_string = compute_features(&disco.features);
     let identities_string = compute_identities(&disco.identities);
-    let extensions_string = compute_extensions(&disco.extensions);
+    let extensions_string = compute_extensions(&disco.extensions)?;
 
     let mut final_string = vec![];
     final_string.extend(features_string);
     final_string.extend(identities_string);
     final_string.extend(extensions_string);
-    final_string
+    Ok(final_string)
 }
 
 fn get_hash_vec(hash: &[u8]) -> Vec<u8> {
@@ -204,7 +213,7 @@ mod tests {
     fn test_simple() {
         let elem: Element = "<query xmlns='http://jabber.org/protocol/disco#info'><identity category='client' type='pc'/><feature var='http://jabber.org/protocol/disco#info'/></query>".parse().unwrap();
         let disco = DiscoInfoResult::try_from(elem).unwrap();
-        let ecaps2 = compute_disco(&disco);
+        let ecaps2 = compute_disco(&disco).unwrap();
         assert_eq!(ecaps2.len(), 54);
     }
 
@@ -263,7 +272,7 @@ mod tests {
             117, 115, 77, 111, 100, 31, 30, 28, 28,
         ];
         let disco = DiscoInfoResult::try_from(elem).unwrap();
-        let ecaps2 = compute_disco(&disco);
+        let ecaps2 = compute_disco(&disco).unwrap();
         assert_eq!(ecaps2.len(), 0x1d9);
         assert_eq!(ecaps2, expected);
 
@@ -424,7 +433,7 @@ mod tests {
             98, 50, 41, 31, 30, 29, 28,
         ];
         let disco = DiscoInfoResult::try_from(elem).unwrap();
-        let ecaps2 = compute_disco(&disco);
+        let ecaps2 = compute_disco(&disco).unwrap();
         assert_eq!(ecaps2.len(), 0x543);
         assert_eq!(ecaps2, expected);
 

src/ibr.rs

@@ -207,18 +207,6 @@ mod tests {
         assert!(!query.fields["instructions"].is_empty());
         let form = query.form.clone().unwrap();
         assert!(!form.instructions.unwrap().is_empty());
-        assert!(form
-            .fields
-            .binary_search_by(|field| field.var.cmp(&String::from("first")))
-            .is_ok());
-        assert!(form
-            .fields
-            .binary_search_by(|field| field.var.cmp(&String::from("x-gender")))
-            .is_ok());
-        assert!(form
-            .fields
-            .binary_search_by(|field| field.var.cmp(&String::from("coucou")))
-            .is_err());
         let elem2 = query.into();
         assert!(elem1.compare_to(&elem2));
     }