xmpp-parsers: Simplify StreamFeatures::sasl_mechanisms

This was already just a wrapper around a Vec<String>, but we can do away
with the wrapper thanks to #[xml(extract)].

I’ve also replaced Vec with BTreeSet, since that corresponds better to
how the mechanisms are.

parsers/ChangeLog

@@ -9,6 +9,8 @@ XXXX-YY-ZZ RELEASER <admin@example.com>
         - The ibr::Query struct has been split into FieldsQuery, RemoveQuery,
           LegacyQuery and FormsQuery, to make it reflect more how it gets
           used in XMPP.
+        - Remove the unnecessary SaslMechanisms struct, to directly extract
+          SASL mechanisms in a Vec<String> in StreamFeatures.
     * Improvements:
         - Make Priority’s inner i8 pub, which had been broken since the
           conversion to xso. (!632)

parsers/src/stream_features.rs

@@ -4,6 +4,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+use alloc::collections::BTreeSet;
 use minidom::Element;
 use xso::{AsXml, FromXml};
 
@@ -28,8 +29,10 @@ pub struct StreamFeatures {
     pub bind: Option<BindFeature>,
 
     /// List of supported SASL mechanisms
-    #[xml(child(default))]
+    #[xml(extract(default, namespace = ns::SASL, name = "mechanisms", fields(
-    pub sasl_mechanisms: SaslMechanisms,
+        extract(n = .., name = "mechanism", fields(text(type_ = String)))
+    )))]
+    pub sasl_mechanisms: BTreeSet<String>,
 
     /// Limits advertised by the server.
     #[xml(child(default))]
@@ -59,15 +62,6 @@ pub struct StreamFeatures {
     pub others: Vec<Element>,
 }
 
-/// List of supported SASL mechanisms
-#[derive(FromXml, AsXml, PartialEq, Debug, Clone, Default)]
-#[xml(namespace = ns::SASL, name = "mechanisms")]
-pub struct SaslMechanisms {
-    /// List of information elements describing this SASL mechanism.
-    #[xml(extract(n = .., name = "mechanism", fields(text(type_ = String))))]
-    pub mechanisms: Vec<String>,
-}
-
 impl StreamFeatures {
     /// Can initiate TLS session with this server?
     pub fn can_starttls(&self) -> bool {
@@ -88,14 +82,12 @@ mod tests {
     #[cfg(target_pointer_width = "32")]
     #[test]
     fn test_size() {
-        assert_size!(SaslMechanisms, 12);
         assert_size!(StreamFeatures, 92);
     }
 
     #[cfg(target_pointer_width = "64")]
     #[test]
     fn test_size() {
-        assert_size!(SaslMechanisms, 24);
         assert_size!(StreamFeatures, 168);
     }
 
@@ -112,10 +104,10 @@ mod tests {
             .unwrap();
 
         let features = StreamFeatures::try_from(elem).unwrap();
-        assert_eq!(
+        assert!(features.sasl_mechanisms.contains("PLAIN"));
-            features.sasl_mechanisms.mechanisms,
+        assert!(features.sasl_mechanisms.contains("SCRAM-SHA-1"));
-            ["PLAIN", "SCRAM-SHA-1", "SCRAM-SHA-1-PLUS"]
+        assert!(features.sasl_mechanisms.contains("SCRAM-SHA-1-PLUS"));
-        );
+        assert!(!features.sasl_mechanisms.contains("CRAM-MD5"));
     }
 
     #[test]
@@ -131,7 +123,7 @@ mod tests {
         let features = StreamFeatures::try_from(elem).unwrap();
 
         assert_eq!(features.can_bind(), false);
-        assert_eq!(features.sasl_mechanisms.mechanisms.len(), 0);
+        assert_eq!(features.sasl_mechanisms.len(), 0);
         assert_eq!(features.can_starttls(), true);
         assert_eq!(features.starttls.unwrap().required, true);
     }
@@ -151,7 +143,7 @@ mod tests {
         let features = StreamFeatures::try_from(elem).unwrap();
 
         assert_eq!(features.can_bind(), true);
-        assert_eq!(features.sasl_mechanisms.mechanisms.len(), 0);
+        assert_eq!(features.sasl_mechanisms.len(), 0);
         assert_eq!(features.can_starttls(), false);
         assert_eq!(features.others.len(), 1);
 
@@ -181,7 +173,7 @@ mod tests {
         let features = StreamFeatures::try_from(elem).unwrap();
 
         assert_eq!(features.can_bind(), false);
-        assert_eq!(features.sasl_mechanisms.mechanisms.len(), 0);
+        assert_eq!(features.sasl_mechanisms.len(), 0);
         assert_eq!(features.can_starttls(), false);
     }
 }

tokio-xmpp/src/client/login.rs

@@ -5,14 +5,14 @@ use sasl::client::mechanisms::{Anonymous, Plain, Scram};
 use sasl::client::Mechanism;
 use sasl::common::scram::{Sha1, Sha256};
 use sasl::common::Credentials;
-use std::collections::HashSet;
+use std::collections::BTreeSet;
 use std::io;
 use tokio::io::{AsyncBufRead, AsyncWrite};
 use xmpp_parsers::{
     jid::Jid,
     ns,
     sasl::{Auth, Mechanism as XMPPMechanism, Nonza, Response},
-    stream_features::{SaslMechanisms, StreamFeatures},
+    stream_features::StreamFeatures,
 };
 
 use crate::{
@@ -30,7 +30,7 @@ use crate::{
 /// this returns the `stream` as [`InitiatingStream`] on success.
 pub async fn auth<S: AsyncBufRead + AsyncWrite + Unpin>(
     mut stream: XmppStream<S>,
-    sasl_mechanisms: &SaslMechanisms,
+    sasl_mechanisms: BTreeSet<String>,
     creds: Credentials,
 ) -> Result<InitiatingStream<S>, Error> {
     let local_mechs: Vec<Box<dyn Fn() -> Box<dyn Mechanism + Send + Sync> + Send>> = vec![
@@ -40,11 +40,9 @@ pub async fn auth<S: AsyncBufRead + AsyncWrite + Unpin>(
         Box::new(|| Box::new(Anonymous::new())),
     ];
 
-    let remote_mechs: HashSet<String> = sasl_mechanisms.mechanisms.iter().cloned().collect();
-
     for local_mech in local_mechs {
         let mut mechanism = local_mech();
-        if remote_mechs.contains(mechanism.name()) {
+        if sasl_mechanisms.contains(mechanism.name()) {
             let initial = mechanism.initial();
             let mechanism_name =
                 XMPPMechanism::from_str(mechanism.name()).map_err(ProtocolError::Parsers)?;
@@ -124,7 +122,7 @@ pub async fn client_auth<C: ServerConnector>(
         .with_password(password)
         .with_channel_binding(channel_binding);
     // Authenticated (unspecified) stream
-    let stream = auth(xmpp_stream, &features.sasl_mechanisms, creds).await?;
+    let stream = auth(xmpp_stream, features.sasl_mechanisms, creds).await?;
     let stream = stream
         .send_header(StreamHeader {
             to: Some(Cow::Borrowed(jid.domain().as_str())),