feat(rumqttc): allow empty client id

rumqttc/CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Added
+- `MqttOptions::try_set_clean_session` to check if `clean_session` is `true` when `client_id` is an empty string.
 - Expose `EventLoop::clean` to allow triggering shutdown and subsequent storage of pending requests
 - Support for all variants of TLS key formats currently supported by Rustls: `PKCS#1`, `PKCS#8`, `RFC5915`. In practice we should now support all RSA keys and ECC keys in `DER` and `SEC1` encoding. Previously only `PKCS#1` and `PKCS#8` where supported.
 - TLS Error variants: `NoValidClientCertInChain`, `NoValidKeyInChain`.
@@ -16,6 +17,7 @@ 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` does not panic if the `client_id` is an empty string or starts with a whitespace.
 - 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,11 +613,44 @@ 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.
+    ///
+    /// NOTE: It panicks if the `client_id` is a zero byte string and `clean_session` is
+    /// not `true`.
+    ///
+    /// ```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 {
+        if self.client_id.is_empty() {
+            assert!(
+                clean_session,
+                "Cannot unset clean session when client id is empty"
+            );
+        }
         self.clean_session = clean_session;
         self
     }
 
+    /// Does the same thing as [`Self::set_clean_session`], however it returns an error
+    ///  if `clean_session = false` and `client_id` is a zero byte string.
+    ///
+    /// ```
+    /// # use rumqttc::{MqttOptions, OptionError};
+    /// let mut options = MqttOptions::new("", "localhost", 1883);
+    /// let error = options.try_set_clean_session(false).unwrap_err();
+    /// assert_eq!(error, OptionError::CleanSession);
+    /// ```
+    #[cfg(feature = "url")]
+    pub fn try_set_clean_session(&mut self, clean_session: bool) -> Result<&mut Self, OptionError> {
+        if self.client_id.is_empty() && !clean_session {
+            return Err(OptionError::CleanSession);
+        }
+        self.clean_session = clean_session;
+        Ok(self)
+    }
+
     /// Clean session
     pub fn clean_session(&self) -> bool {
         self.clean_session
@@ -918,12 +940,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() {
@@ -1006,10 +1022,4 @@ mod test {
             OptionError::Inflight
         );
     }
-
-    #[test]
-    #[should_panic]
-    fn no_client_id() {
-        let _mqtt_opts = MqttOptions::new("", "127.0.0.1", 1883).set_clean_session(true);
-    }
 }