3 files changed, 46 insertions, 42 deletions

diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs
index 0c28d032c6..f17f30127d 100644
--- a/src/rust/protover/ffi.rs
+++ b/src/rust/protover/ffi.rs
@@ -62,6 +62,9 @@ pub extern "C" fn protover_all_supported(
         };
 
     if let Some(unsupported) = relay_proto_entry.all_supported() {
+        if missing_out.is_null() {
+            return 0;
+        }
         let c_unsupported: CString = match CString::new(unsupported.to_string()) {
             Ok(n) => n,
             Err(_) => return 1,
diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs
index c447186098..aa8d243bad 100644
--- a/src/rust/protover/protoset.rs
+++ b/src/rust/protover/protoset.rs
@@ -290,7 +290,7 @@ impl ProtoSet {
         });
 
         let pairs = pairs.collect();
-        ProtoSet::is_ok(ProtoSet{ pairs }).expect("should be already sorted")
+        ProtoSet::is_ok(ProtoSet { pairs }).expect("should be already sorted")
     }
 }
 
@@ -352,23 +352,25 @@ impl FromStr for ProtoSet {
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("3-"));
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1-,4"));
     ///
-    /// // Things which would get parsed into an _empty_ `ProtoSet` are,
-    /// // however, legal, and result in an empty `ProtoSet`:
+    /// // An empty string is, however, legal, and results in an
+    /// // empty `ProtoSet`:
     /// assert_eq!(Ok(ProtoSet::default()), ProtoSet::from_str(""));
-    /// assert_eq!(Ok(ProtoSet::default()), ProtoSet::from_str(",,,"));
     /// #
     /// # Ok(protoset)
     /// # }
     /// # fn main() { do_test(); }  // wrap the test so we can use the ? operator
     /// ```
     fn from_str(version_string: &str) -> Result<Self, Self::Err> {
+        // If we were passed in an empty string, then return an empty ProtoSet.
+        if version_string.is_empty() {
+            return Ok(Self::default());
+        }
+
         let mut pairs: Vec<(Version, Version)> = Vec::new();
         let pieces: ::std::str::Split<char> = version_string.split(',');
 
         for p in pieces {
-            if p.is_empty() {
-                continue;
-            } else if p.contains('-') {
+            if p.contains('-') {
                 let mut pair = p.splitn(2, '-');
 
                 let low = pair.next().ok_or(ProtoverError::Unparseable)?;
@@ -377,24 +379,14 @@ impl FromStr for ProtoSet {
                 let lo: Version = low.parse().or(Err(ProtoverError::Unparseable))?;
                 let hi: Version = high.parse().or(Err(ProtoverError::Unparseable))?;
 
-                if lo == u32::MAX || hi == u32::MAX {
-                    return Err(ProtoverError::ExceedsMax);
-                }
                 pairs.push((lo, hi));
             } else {
                 let v: u32 = p.parse().or(Err(ProtoverError::Unparseable))?;
 
-                if v == u32::MAX {
-                    return Err(ProtoverError::ExceedsMax);
-                }
                 pairs.push((v, v));
             }
         }
-        // If we were passed in an empty string, or
-        // simply a comma, or a pile of commas, then return an empty ProtoSet.
-        if pairs.len() == 0 {
-            return Ok(ProtoSet::default());
-        }
+
         ProtoSet::from_slice(&pairs[..])
     }
 }
@@ -559,6 +551,13 @@ mod test {
     }
 
     #[test]
+    fn test_versions_from_str_commas() {
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str(","));
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1,,2"));
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1,2,"));
+    }
+
+    #[test]
     fn test_versions_from_str_hyphens() {
         assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("--1"));
         assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("-1-2"));
diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs
index 43822a3f15..8624afeafa 100644
--- a/src/rust/protover/protover.rs
+++ b/src/rust/protover/protover.rs
@@ -124,15 +124,15 @@ impl From<Protocol> for UnknownProtocol {
     }
 }
 
-#[cfg(feature="test_linking_hack")]
+#[cfg(feature = "test_linking_hack")]
 fn have_linkauth_v1() -> bool {
     true
 }
 
-#[cfg(not(feature="test_linking_hack"))]
+#[cfg(not(feature = "test_linking_hack"))]
 fn have_linkauth_v1() -> bool {
     use external::c_tor_is_using_nss;
-    ! c_tor_is_using_nss()
+    !c_tor_is_using_nss()
 }
 
 /// Get a CStr representation of current supported protocols, for
@@ -152,29 +152,31 @@ fn have_linkauth_v1() -> bool {
 ///
 //  C_RUST_COUPLED: protover.c `protover_get_supported_protocols`
 pub(crate) fn get_supported_protocols_cstr() -> &'static CStr {
-    if ! have_linkauth_v1() {
-        cstr!("Cons=1-2 \
-               Desc=1-2 \
-               DirCache=1-2 \
-               HSDir=1-2 \
-               HSIntro=3-4 \
-               HSRend=1-2 \
-               Link=1-5 \
-               LinkAuth=3 \
-               Microdesc=1-2 \
-               Relay=1-2"
+    if !have_linkauth_v1() {
+        cstr!(
+            "Cons=1-2 \
+             Desc=1-2 \
+             DirCache=1-2 \
+             HSDir=1-2 \
+             HSIntro=3-4 \
+             HSRend=1-2 \
+             Link=1-5 \
+             LinkAuth=3 \
+             Microdesc=1-2 \
+             Relay=1-2"
         )
     } else {
-        cstr!("Cons=1-2 \
-               Desc=1-2 \
-               DirCache=1-2 \
-               HSDir=1-2 \
-               HSIntro=3-4 \
-               HSRend=1-2 \
-               Link=1-5 \
-               LinkAuth=1,3 \
-               Microdesc=1-2 \
-               Relay=1-2"
+        cstr!(
+            "Cons=1-2 \
+             Desc=1-2 \
+             DirCache=1-2 \
+             HSDir=1-2 \
+             HSIntro=3-4 \
+             HSRend=1-2 \
+             Link=1-5 \
+             LinkAuth=1,3 \
+             Microdesc=1-2 \
+             Relay=1-2"
         )
     }
 }