feat(rumqttc): allow empty client id

rumqttc/CHANGELOG.md

@@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Surfaced `AsyncClient`'s `from_senders` method to the `Client` as `from_sender`
 
 ### Changed
+- `MqttOptions::new` now accepts empty client id.
+- `MqttOptions::set_clean_session` now panics if client ID is empty and `clean_session` flag is set to false.
 - Synchronous client methods take `&self` instead of `&mut self` (#646)
 - Removed the `Key` enum: users do not need to specify the TLS key variant in the `TlsConfiguration` anymore, this is inferred automatically.
 To update your code simply remove `Key::ECC()` or `Key::RSA()` from the initialization.

rumqttc/src/lib.rs

@@ -491,25 +491,14 @@ impl MqttOptions {
     /// # use rumqttc::MqttOptions;
     /// let options = MqttOptions::new("123", "localhost", 1883);
     /// ```
-    /// NOTE: you are not allowed to use an id that starts with a whitespace or is empty.
-    /// for example, the following code would panic:
-    /// ```should_panic
-    /// # use rumqttc::MqttOptions;
-    /// let options = MqttOptions::new("", "localhost", 1883);
-    /// ```
     pub fn new<S: Into<String>, T: Into<String>>(id: S, host: T, port: u16) -> MqttOptions {
-        let id = id.into();
-        if id.starts_with(' ') || id.is_empty() {
-            panic!("Invalid client id");
-        }
-
         MqttOptions {
             broker_addr: host.into(),
             port,
             transport: Transport::tcp(),
             keep_alive: Duration::from_secs(60),
             clean_session: true,
-            client_id: id,
+            client_id: id.into(),
             credentials: None,
             max_incoming_packet_size: 10 * 1024,
             max_outgoing_packet_size: 10 * 1024,
@@ -624,7 +613,21 @@ impl MqttOptions {
     /// When set `false`, broker will hold the client state and performs pending
     /// operations on the client when reconnection with same `client_id`
     /// happens. Local queue state is also held to retransmit packets after reconnection.
+    ///
+    /// # Panic
+    ///
+    /// Panics if `clean_session` is false when `client_id` is empty.
+    ///
+    /// ```should_panic
+    /// # use rumqttc::MqttOptions;
+    /// let mut options = MqttOptions::new("", "localhost", 1883);
+    /// options.set_clean_session(false);
+    /// ```
     pub fn set_clean_session(&mut self, clean_session: bool) -> &mut Self {
+        assert!(
+            !self.client_id.is_empty() || clean_session,
+            "Cannot unset clean session when client id is empty"
+        );
         self.clean_session = clean_session;
         self
     }
@@ -918,12 +921,6 @@ impl Debug for MqttOptions {
 mod test {
     use super::*;
 
-    #[test]
-    #[should_panic]
-    fn client_id_startswith_space() {
-        let _mqtt_opts = MqttOptions::new(" client_a", "127.0.0.1", 1883).set_clean_session(true);
-    }
-
     #[test]
     #[cfg(all(feature = "use-rustls", feature = "websocket"))]
     fn no_scheme() {
@@ -1008,8 +1005,14 @@ mod test {
     }
 
     #[test]
-    #[should_panic]
-    fn no_client_id() {
+    fn accept_empty_client_id() {
         let _mqtt_opts = MqttOptions::new("", "127.0.0.1", 1883).set_clean_session(true);
     }
+
+    #[test]
+    fn set_clean_session_when_client_id_present() {
+        let mut options = MqttOptions::new("client_id", "127.0.0.1", 1883);
+        options.set_clean_session(false);
+        options.set_clean_session(true);
+    }
 }

rumqttc/src/v5/mod.rs

@@ -113,28 +113,17 @@ impl MqttOptions {
     /// - port: The port number on which broker must be listening for incoming connections
     ///
     /// ```
-    /// # use rumqttc::MqttOptions;
+    /// # use rumqttc::v5::MqttOptions;
     /// let options = MqttOptions::new("123", "localhost", 1883);
     /// ```
-    /// NOTE: you are not allowed to use an id that starts with a whitespace or is empty.
-    /// for example, the following code would panic:
-    /// ```should_panic
-    /// # use rumqttc::MqttOptions;
-    /// let options = MqttOptions::new("", "localhost", 1883);
-    /// ```
     pub fn new<S: Into<String>, T: Into<String>>(id: S, host: T, port: u16) -> MqttOptions {
-        let id = id.into();
-        if id.starts_with(' ') || id.is_empty() {
-            panic!("Invalid client id");
-        }
-
         MqttOptions {
             broker_addr: host.into(),
             port,
             transport: Transport::tcp(),
             keep_alive: Duration::from_secs(60),
             clean_start: true,
-            client_id: id,
+            client_id: id.into(),
             credentials: None,
             request_channel_capacity: 10,
             max_request_batch: 0,
@@ -730,12 +719,6 @@ impl Debug for MqttOptions {
 mod test {
     use super::*;
 
-    #[test]
-    #[should_panic]
-    fn client_id_startswith_space() {
-        let _mqtt_opts = MqttOptions::new(" client_a", "127.0.0.1", 1883).set_clean_start(true);
-    }
-
     #[test]
     #[cfg(all(feature = "use-rustls", feature = "websocket"))]
     fn no_scheme() {
@@ -821,8 +804,7 @@ mod test {
     }
 
     #[test]
-    #[should_panic]
-    fn no_client_id() {
+    fn allow_empty_client_id() {
         let _mqtt_opts = MqttOptions::new("", "127.0.0.1", 1883).set_clean_start(true);
     }
 }