Merge remote-tracking branch 'isis-github/bug24031_r5_squashed_033' into maint-0.3.3

changes/bug24031

@@ -0,0 +1,13 @@
+  o Major bugfixes (protover, voting):
+    - Revise Rust implementation of protover to use a more memory-efficient
+      voting algorithm and corresponding data structures, thus avoiding a
+      potential (but small impact) DoS attack where specially crafted protocol
+      strings would expand to several potential megabytes in memory.  In the
+      process, several portions of code were revised to be methods on new,
+      custom types, rather than functions taking interchangeable types, thus
+      increasing type safety of the module.  Custom error types and handling
+      were added as well, in order to facilitate better error dismissal/handling
+      in outside crates and avoid mistakenly passing an internal error string to
+      C over the FFI boundary.  Many tests were added, and some previous
+      differences between the C and Rust implementations have been
+      remedied. Fixes 24031; bugfix on 0.3.3.1-alpha.

src/or/protover.c

@@ -671,7 +671,9 @@ int
 protover_all_supported(const char *s, char **missing_out)
 {
   int all_supported = 1;
-  smartlist_t *missing;
+  smartlist_t *missing_some;
+  smartlist_t *missing_completely;
+  smartlist_t *missing_all;
 
   if (!s) {
     return 1;
@@ -684,7 +686,8 @@ protover_all_supported(const char *s, char **missing_out)
     return 1;
   }
 
-  missing = smartlist_new();
+  missing_some = smartlist_new();
+  missing_completely = smartlist_new();
 
   SMARTLIST_FOREACH_BEGIN(entries, const proto_entry_t *, ent) {
     protocol_type_t tp;
@@ -696,26 +699,86 @@ protover_all_supported(const char *s, char **missing_out)
     }
 
     SMARTLIST_FOREACH_BEGIN(ent->ranges, const proto_range_t *, range) {
+      proto_entry_t *unsupported = tor_malloc_zero(sizeof(proto_entry_t));
+      proto_range_t *versions = tor_malloc_zero(sizeof(proto_range_t));
       uint32_t i;
+
+      unsupported->name = tor_strdup(ent->name);
+      unsupported->ranges = smartlist_new();
+
       for (i = range->low; i <= range->high; ++i) {
         if (!protover_is_supported_here(tp, i)) {
-          goto unsupported;
+          if (versions->low == 0 && versions->high == 0) {
+            versions->low = i;
+            /* Pre-emptively add the high now, just in case we're in a single
+             * version range (e.g. "Link=999"). */
+            versions->high = i;
+          }
+          /* If the last one to be unsupported is one less than the current
+           * one, we're in a continous range, so set the high field. */
+          if ((versions->high && versions->high == i - 1) ||
+              /* Similarly, if the last high wasn't set and we're currently
+               * one higher than the low, add current index as the highest
+               * known high. */
+              (!versions->high && versions->low == i - 1)) {
+            versions->high = i;
+            continue;
+          }
+        } else {
+          /* If we hit a supported version, and we previously had a range,
+           * we've hit a non-continuity. Copy the previous range and add it to
+           * the unsupported->ranges list and zero-out the previous range for
+           * the next iteration. */
+          if (versions->low != 0 && versions->high != 0) {
+            proto_range_t *versions_to_add = tor_malloc(sizeof(proto_range_t));
+
+            versions_to_add->low = versions->low;
+            versions_to_add->high = versions->high;
+            smartlist_add(unsupported->ranges, versions_to_add);
+
+            versions->low = 0;
+            versions->high = 0;
+          }
         }
       }
+      /* Once we've run out of versions to check, see if we had any unsupported
+       * ones and, if so, add them to unsupported->ranges. */
+      if (versions->low != 0 && versions->high != 0) {
+        smartlist_add(unsupported->ranges, versions);
+      }
+      /* Finally, if we had something unsupported, add it to the list of
+       * missing_some things and mark that there was something missing. */
+      if (smartlist_len(unsupported->ranges) != 0) {
+        smartlist_add(missing_some, (void*) unsupported);
+        all_supported = 0;
+      } else {
+        proto_entry_free(unsupported);
+        tor_free(versions);
+      }
     } SMARTLIST_FOREACH_END(range);
 
     continue;
 
   unsupported:
     all_supported = 0;
-    smartlist_add(missing, (void*) ent);
+    smartlist_add(missing_completely, (void*) ent);
   } SMARTLIST_FOREACH_END(ent);
 
+  /* We keep the two smartlists separate so that we can free the proto_entry_t
+   * we created and put in missing_some, so here we add them together to build
+   * the string. */
+  missing_all = smartlist_new();
+  smartlist_add_all(missing_all, missing_some);
+  smartlist_add_all(missing_all, missing_completely);
+
   if (missing_out && !all_supported) {
-    tor_assert(0 != smartlist_len(missing));
-    *missing_out = encode_protocol_list(missing);
+    tor_assert(smartlist_len(missing_all) != 0);
+    *missing_out = encode_protocol_list(missing_all);
   }
-  smartlist_free(missing);
+  SMARTLIST_FOREACH(missing_some, proto_entry_t *, ent, proto_entry_free(ent));
+  smartlist_free(missing_some);
+  smartlist_free(missing_completely);
+  smartlist_free(missing_all);
 
   SMARTLIST_FOREACH(entries, proto_entry_t *, ent, proto_entry_free(ent));
   smartlist_free(entries);

src/rust/protover/errors.rs

@@ -0,0 +1,43 @@
+// Copyright (c) 2018, The Tor Project, Inc.
+// Copyright (c) 2018, isis agora lovecruft
+// See LICENSE for licensing information
+
+//! Various errors which may occur during protocol version parsing.
+
+use std::fmt;
+use std::fmt::Display;
+
+/// All errors which may occur during protover parsing routines.
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
+#[allow(missing_docs)] // See Display impl for error descriptions
+pub enum ProtoverError {
+    Overlap,
+    LowGreaterThanHigh,
+    Unparseable,
+    ExceedsMax,
+    ExceedsExpansionLimit,
+    UnknownProtocol,
+    ExceedsNameLimit,
+}
+
+/// Descriptive error messages for `ProtoverError` variants.
+impl Display for ProtoverError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            ProtoverError::Overlap
+                => write!(f, "Two or more (low, high) protover ranges would overlap once expanded."),
+            ProtoverError::LowGreaterThanHigh
+                => write!(f, "The low in a (low, high) protover range was greater than high."),
+            ProtoverError::Unparseable
+                => write!(f, "The protover string was unparseable."),
+            ProtoverError::ExceedsMax
+                => write!(f, "The high in a (low, high) protover range exceeds u32::MAX."),
+            ProtoverError::ExceedsExpansionLimit
+                => write!(f, "The protover string would exceed the maximum expansion limit."),
+            ProtoverError::UnknownProtocol
+                => write!(f, "A protocol in the protover string we attempted to parse is unknown."),
+            ProtoverError::ExceedsNameLimit
+                => write!(f, "An unrecognised protocol name was too long."),
+        }
+    }
+}

src/rust/protover/ffi.rs

@@ -9,30 +9,32 @@ use libc::{c_char, c_int, uint32_t};
 use std::ffi::CStr;
 use std::ffi::CString;
 
-use protover::*;
 use smartlist::*;
 use tor_allocate::allocate_and_copy_string;
 use tor_util::strings::byte_slice_is_c_like;
 use tor_util::strings::empty_static_cstr;
 
 
+use errors::ProtoverError;
+use protover::*;
+
 /// Translate C enums to Rust Proto enums, using the integer value of the C
-/// enum to map to its associated Rust enum
+/// enum to map to its associated Rust enum.
 ///
 /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t`
-fn translate_to_rust(c_proto: uint32_t) -> Result<Proto, &'static str> {
+fn translate_to_rust(c_proto: uint32_t) -> Result<Protocol, ProtoverError> {
     match c_proto {
-        0 => Ok(Proto::Link),
-        1 => Ok(Proto::LinkAuth),
-        2 => Ok(Proto::Relay),
-        3 => Ok(Proto::DirCache),
-        4 => Ok(Proto::HSDir),
-        5 => Ok(Proto::HSIntro),
-        6 => Ok(Proto::HSRend),
-        7 => Ok(Proto::Desc),
-        8 => Ok(Proto::Microdesc),
-        9 => Ok(Proto::Cons),
-        _ => Err("Invalid protocol type"),
+        0 => Ok(Protocol::Link),
+        1 => Ok(Protocol::LinkAuth),
+        2 => Ok(Protocol::Relay),
+        3 => Ok(Protocol::DirCache),
+        4 => Ok(Protocol::HSDir),
+        5 => Ok(Protocol::HSIntro),
+        6 => Ok(Protocol::HSRend),
+        7 => Ok(Protocol::Desc),
+        8 => Ok(Protocol::Microdesc),
+        9 => Ok(Protocol::Cons),
+        _ => Err(ProtoverError::UnknownProtocol),
     }
 }
 
@@ -57,19 +59,26 @@ pub extern "C" fn protover_all_supported(
         Err(_) => return 1,
     };
 
-    let (is_supported, unsupported) = all_supported(relay_version);
+    let relay_proto_entry: UnvalidatedProtoEntry = match relay_version.parse() {
+        Ok(n)  => n,
+        Err(_) => return 1,
+    };
+    let maybe_unsupported: Option<UnvalidatedProtoEntry> = relay_proto_entry.all_supported();
 
-    if unsupported.len() > 0 {
-        let c_unsupported = match CString::new(unsupported) {
+    if maybe_unsupported.is_some() {
+        let unsupported: UnvalidatedProtoEntry = maybe_unsupported.unwrap();
+        let c_unsupported: CString = match CString::new(unsupported.to_string()) {
             Ok(n) => n,
             Err(_) => return 1,
         };
 
         let ptr = c_unsupported.into_raw();
         unsafe { *missing_out = ptr };
+
+        return 0;
     }
 
-    return if is_supported { 1 } else { 0 };
+    1
 }
 
 /// Provide an interface for C to translate arguments and return types for
@@ -92,16 +101,18 @@ pub extern "C" fn protocol_list_supports_protocol(
         Ok(n) => n,
         Err(_) => return 1,
     };
-
-    let protocol = match translate_to_rust(c_protocol) {
-        Ok(n) => n,
+    let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() {
+        Ok(n)  => n,
         Err(_) => return 0,
     };
-
-    let is_supported =
-        protover_string_supports_protocol(protocol_list, protocol, version);
-
-    return if is_supported { 1 } else { 0 };
+    let protocol: UnknownProtocol = match translate_to_rust(c_protocol) {
+        Ok(n) => n.into(),
+        Err(_) => return 0,
+    };
+    match proto_entry.supports_protocol(&protocol, &version) {
+        false => return 0,
+        true  => return 1,
+    }
 }
 
 /// Provide an interface for C to translate arguments and return types for
@@ -130,11 +141,15 @@ pub extern "C" fn protocol_list_supports_protocol_or_later(
         Err(_) => return 0,
     };
 
-    let is_supported =
-        protover_string_supports_protocol_or_later(
-            protocol_list, protocol, version);
+    let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() {
+        Ok(n)  => n,
+        Err(_) => return 1,
+    };
 
-    return if is_supported { 1 } else { 0 };
+    if proto_entry.supports_protocol_or_later(&protocol.into(), &version) {
+        return 1;
+    }
+    0
 }
 
 /// Provide an interface for C to translate arguments and return types for
@@ -160,6 +175,8 @@ pub extern "C" fn protover_get_supported_protocols() -> *const c_char {
 
 /// Provide an interface for C to translate arguments and return types for
 /// protover::compute_vote
+//
+// Why is the threshold a signed integer? —isis
 #[no_mangle]
 pub extern "C" fn protover_compute_vote(
     list: *const Stringlist,
@@ -174,10 +191,19 @@ pub extern "C" fn protover_compute_vote(
     // Dereference of raw pointer requires an unsafe block. The pointer is
     // checked above to ensure it is not null.
     let data: Vec<String> = unsafe { (*list).get_list() };
+    let hold: usize = threshold as usize;
+    let mut proto_entries: Vec<UnvalidatedProtoEntry> = Vec::new();
 
-    let vote = compute_vote(data, threshold);
+    for datum in data {
+        let entry: UnvalidatedProtoEntry = match datum.parse() {
+            Ok(x)  => x,
+            Err(_) => continue,
+        };
+        proto_entries.push(entry);
+    }
+    let vote: UnvalidatedProtoEntry = ProtoverVote::compute(&proto_entries, &hold);
 
-    allocate_and_copy_string(&vote)
+    allocate_and_copy_string(&vote.to_string())
 }
 
 /// Provide an interface for C to translate arguments and return types for
@@ -192,7 +218,7 @@ pub extern "C" fn protover_is_supported_here(
         Err(_) => return 0,
     };
 
-    let is_supported = is_supported_here(protocol, version);
+    let is_supported = is_supported_here(&protocol, &version);
 
     return if is_supported { 1 } else { 0 };
 }
@@ -220,7 +246,7 @@ pub extern "C" fn protover_compute_for_old_tor(version: *const c_char) -> *const
         Err(_) => return empty.as_ptr(),
     };
 
-    elder_protocols = compute_for_old_tor(&version);
+    elder_protocols = compute_for_old_tor_cstr(&version);
 
     // If we're going to pass it to C, there cannot be any intermediate NUL
     // bytes.  An assert is okay here, since changing the const byte slice

src/rust/protover/lib.rs

@@ -22,12 +22,16 @@
 //! protocols to develop independently, without having to claim compatibility
 //! with specific versions of Tor.
 
+#[deny(missing_docs)]
+
 extern crate libc;
 extern crate smartlist;
 extern crate external;
 extern crate tor_allocate;
 extern crate tor_util;
 
+pub mod errors;
+pub mod protoset;
 mod protover;
 pub mod ffi;
 

src/rust/protover/protoset.rs

@@ -0,0 +1,634 @@
+// Copyright (c) 2018, The Tor Project, Inc.
+// Copyright (c) 2018, isis agora lovecruft
+// See LICENSE for licensing information
+
+//! Sets for lazily storing ordered, non-overlapping ranges of integers.
+
+use std::slice;
+use std::str::FromStr;
+use std::u32;
+
+use errors::ProtoverError;
+
+/// A single version number.
+pub type Version = u32;
+
+/// A `ProtoSet` stores an ordered `Vec<T>` of `(low, high)` pairs of ranges of
+/// non-overlapping protocol versions.
+///
+/// # Examples
+///
+/// ```
+/// use std::str::FromStr;
+///
+/// use protover::errors::ProtoverError;
+/// use protover::protoset::ProtoSet;
+/// use protover::protoset::Version;
+///
+/// # fn do_test() -> Result<ProtoSet, ProtoverError> {
+/// let protoset: ProtoSet = ProtoSet::from_str("3-5,8")?;
+///
+/// // We could also equivalently call:
+/// let protoset: ProtoSet = "3-5,8".parse()?;
+///
+/// assert!(protoset.contains(&4));
+/// assert!(!protoset.contains(&7));
+///
+/// let expanded: Vec<Version> = protoset.clone().into();
+///
+/// assert_eq!(&expanded[..], &[3, 4, 5, 8]);
+///
+/// let contracted: String = protoset.clone().to_string();
+///
+/// assert_eq!(contracted, "3-5,8".to_string());
+/// # Ok(protoset)
+/// # }
+/// # fn main() { do_test(); }  // wrap the test so we can use the ? operator
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
+pub struct ProtoSet {
+    pub(crate) pairs: Vec<(Version, Version)>,
+}
+
+impl Default for ProtoSet {
+    fn default() -> Self {
+        let pairs: Vec<(Version, Version)> = Vec::new();
+
+        ProtoSet{ pairs }
+    }
+}
+
+impl<'a> ProtoSet {
+    /// Create a new `ProtoSet` from a slice of `(low, high)` pairs.
+    ///
+    /// # Inputs
+    ///
+    /// We do not assume the input pairs are deduplicated or ordered.
+    pub fn from_slice(low_high_pairs: &'a [(Version, Version)]) -> Result<Self, ProtoverError> {
+        let mut pairs: Vec<(Version, Version)> = Vec::with_capacity(low_high_pairs.len());
+
+        for &(low, high) in low_high_pairs {
+            pairs.push((low, high));
+        }
+        // Sort the pairs without reallocation and remove all duplicate pairs.
+        pairs.sort_unstable();
+        pairs.dedup();
+
+        ProtoSet{ pairs }.is_ok()
+    }
+}
+
+/// Expand this `ProtoSet` to a `Vec` of all its `Version`s.
+///
+/// # Examples
+///
+/// ```
+/// use std::str::FromStr;
+/// use protover::protoset::ProtoSet;
+/// use protover::protoset::Version;
+/// # use protover::errors::ProtoverError;
+///
+/// # fn do_test() -> Result<Vec<Version>, ProtoverError> {
+/// let protoset: ProtoSet = ProtoSet::from_str("3-5,21")?;
+/// let versions: Vec<Version> = protoset.into();
+///
+/// assert_eq!(&versions[..], &[3, 4, 5, 21]);
+/// #
+/// # Ok(versions)
+/// # }
+/// # fn main() { do_test(); }  // wrap the test so we can use the ? operator
+/// ```
+impl Into<Vec<Version>> for ProtoSet {
+    fn into(self) -> Vec<Version> {
+        let mut versions: Vec<Version> = Vec::new();
+
+        for &(low, high) in self.iter() {
+            versions.extend(low..high + 1);
+        }
+        versions
+    }
+}
+
+impl ProtoSet {
+    /// Get an iterator over the `(low, high)` `pairs` in this `ProtoSet`.
+    pub fn iter(&self) -> slice::Iter<(Version, Version)> {
+        self.pairs.iter()
+    }
+
+    /// Expand this `ProtoSet` into a `Vec` of all its `Version`s.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use protover::errors::ProtoverError;
+    /// use protover::protoset::ProtoSet;
+    ///
+    /// # fn do_test() -> Result<bool, ProtoverError> {
+    /// let protoset: ProtoSet = "3-5,9".parse()?;
+    ///
+    /// assert_eq!(protoset.expand(), vec![3, 4, 5, 9]);
+    ///
+    /// let protoset: ProtoSet = "1,3,5-7".parse()?;
+    ///
+    /// assert_eq!(protoset.expand(), vec![1, 3, 5, 6, 7]);
+    /// #
+    /// # Ok(true)
+    /// # }
+    /// # fn main() { do_test(); }  // wrap the test so we can use the ? operator
+    /// ```
+    pub fn expand(self) -> Vec<Version> {
+        self.into()
+    }
+
+    pub fn len(&self) -> usize {
+        let mut length: usize = 0;
+
+        for &(low, high) in self.iter() {
+            length += (high as usize - low as usize) + 1;
+        }
+
+        length
+    }
+
+    /// Check that this `ProtoSet` is well-formed.
+    ///
+    /// This is automatically called in `ProtoSet::from_str()`.
+    ///
+    /// # Errors
+    ///
+    /// * `ProtoverError::LowGreaterThanHigh`: if its `pairs` were not
+    ///   well-formed, i.e. a `low` in a `(low, high)` was higher than the
+    ///   previous `high`,
+    /// * `ProtoverError::Overlap`: if one or more of the `pairs` are
+    ///   overlapping,
+    /// * `ProtoverError::ExceedsMax`: if the number of versions when expanded
+    ///   would exceed `MAX_PROTOCOLS_TO_EXPAND`, and
+    ///
+    /// # Returns
+    ///
+    /// A `Result` whose `Ok` is this `Protoset`, and whose `Err` is one of the
+    /// errors enumerated in the Errors section above.
+    fn is_ok(self) -> Result<ProtoSet, ProtoverError> {
+        let mut last_high: Version = 0;
+
+        for &(low, high) in self.iter() {
+            if low == u32::MAX || high == u32::MAX {
+                return Err(ProtoverError::ExceedsMax);
+            }
+            if low < last_high {
+                return Err(ProtoverError::Overlap);
+            } else if low > high {
+                return Err(ProtoverError::LowGreaterThanHigh);
+            }
+            last_high = high;
+        }
+
+        Ok(self)
+    }
+
+    /// Determine if this `ProtoSet` contains no `Version`s.
+    ///
+    /// # Returns
+    ///
+    /// * `true` if this `ProtoSet`'s length is zero, and
+    /// * `false` otherwise.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use protover::protoset::ProtoSet;
+    ///
+    /// let protoset: ProtoSet = ProtoSet::default();
+    ///
+    /// assert!(protoset.is_empty());
+    /// ```
+    pub fn is_empty(&self) -> bool {
+        self.pairs.len() == 0
+    }
+
+    /// Determine if `version` is included within this `ProtoSet`.
+    ///
+    /// # Inputs
+    ///
+    /// * `version`: a `Version`.
+    ///
+    /// # Returns
+    ///
+    /// `true` if the `version` is contained within this set; `false` otherwise.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use protover::errors::ProtoverError;
+    /// use protover::protoset::ProtoSet;
+    ///
+    /// # fn do_test() -> Result<ProtoSet, ProtoverError> {
+    /// let protoset: ProtoSet = ProtoSet::from_slice(&[(0, 5), (7, 9), (13, 14)])?;
+    ///
+    /// assert!(protoset.contains(&5));
+    /// assert!(!protoset.contains(&10));
+    /// #
+    /// # Ok(protoset)
+    /// # }
+    /// # fn main() { do_test(); }  // wrap the test so we can use the ? operator
+    /// ```
+    pub fn contains(&self, version: &Version) -> bool {
+        for &(low, high) in self.iter() {
+            if low <= *version && *version <= high {
+                return true;
+            }
+        }
+        false
+    }
+
+    /// Retain only the `Version`s in this `ProtoSet` for which the predicate
+    /// `F` returns `true`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use protover::errors::ProtoverError;
+    /// use protover::protoset::ProtoSet;
+    ///
+    /// # fn do_test() -> Result<bool, ProtoverError> {
+    /// let mut protoset: ProtoSet = "1,3-5,9".parse()?;
+    ///
+    /// // Keep only versions less than or equal to 8:
+    /// protoset.retain(|x| x <= &8);
+    ///
+    /// assert_eq!(protoset.expand(), vec![1, 3, 4, 5]);
+    /// #
+    /// # Ok(true)
+    /// # }
+    /// # fn main() { do_test(); }  // wrap the test so we can use the ? operator
+    /// ```
+    // XXX we could probably do something more efficient here. —isis
+    pub fn retain<F>(&mut self, f: F)
+        where F: FnMut(&Version) -> bool
+    {
+        let mut expanded: Vec<Version> = self.clone().expand();
+        expanded.retain(f);
+        *self = expanded.into();
+    }
+}
+
+impl FromStr for ProtoSet {
+    type Err = ProtoverError;
+
+    /// Parse the unique version numbers supported by a subprotocol from a string.
+    ///
+    /// # Inputs
+    ///
+    /// * `version_string`, a string comprised of "[0-9,-]"
+    ///
+    /// # Returns
+    ///
+    /// A `Result` whose `Ok` value is a `ProtoSet` holding all of the unique
+    /// version numbers.
+    ///
+    /// The returned `Result`'s `Err` value is an `ProtoverError` appropriate to
+    /// the error.
+    ///
+    /// # Errors
+    ///
+    /// This function will error if:
+    ///
+    /// * the `version_string` is an equals (`"="`) sign,
+    /// * the expansion of a version range produces an error (see
+    ///   `expand_version_range`),
+    /// * any single version number is not parseable as an `u32` in radix 10, or
+    /// * there are greater than 2^16 version numbers to expand.
+    ///
+    /// # Examples
+    /// 
+    /// ```
+    /// use std::str::FromStr;
+    ///
+    /// use protover::errors::ProtoverError;
+    /// use protover::protoset::ProtoSet;
+    ///
+    /// # fn do_test() -> Result<ProtoSet, ProtoverError> {
+    /// let protoset: ProtoSet = ProtoSet::from_str("2-5,8")?;
+    ///
+    /// assert!(protoset.contains(&5));
+    /// assert!(!protoset.contains(&10));
+    ///
+    /// // We can also equivalently call `ProtoSet::from_str` by doing (all
+    /// // implementations of `FromStr` can be called this way, this one isn't
+    /// // special):
+    /// let protoset: ProtoSet = "4-6,12".parse()?;
+    ///
+    /// // Calling it (either way) can take really large ranges (up to `u32::MAX`):
+    /// let protoset: ProtoSet = "1-70000".parse()?;
+    /// let protoset: ProtoSet = "1-4294967296".parse()?;
+    ///
+    /// // There are lots of ways to get an `Err` from this function.  Here are
+    /// // a few:
+    /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("="));
+    /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("-"));
+    /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("not_an_int"));
+    /// 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`:
+    /// 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> {
+        let mut pairs: Vec<(Version, Version)> = Vec::new();
+        let pieces: ::std::str::Split<char> = version_string.trim().split(',');
+
+        for piece in pieces {
+            let p: &str = piece.trim();
+
+            if p.is_empty() {
+                continue;
+            } else if p.contains('-') {
+                let mut pair = p.split('-');
+
+                let low  = pair.next().ok_or(ProtoverError::Unparseable)?;
+                let high = pair.next().ok_or(ProtoverError::Unparseable)?;
+
+                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 a bunch of whitespace, 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[..])
+    }
+}
+
+impl ToString for ProtoSet {
+    /// Contracts a `ProtoSet` of versions into a string.
+    ///
+    /// # Returns
+    ///
+    /// A `String` representation of this `ProtoSet` in ascending order.
+    fn to_string(&self) -> String {
+        let mut final_output: Vec<String> = Vec::new();
+
+        for &(lo, hi) in self.iter() {
+            if lo != hi {
+                debug_assert!(lo < hi);
+                final_output.push(format!("{}-{}", lo, hi));
+            } else {
+                final_output.push(format!("{}", lo));
+            }
+        }
+        final_output.join(",")
+    }
+}
+
+/// Checks to see if there is a continuous range of integers, starting at the
+/// first in the list. Returns the last integer in the range if a range exists.
+///
+/// # Inputs
+///
+/// `list`, an ordered  vector of `u32` integers of "[0-9,-]" representing the
+/// supported versions for a single protocol.
+///
+/// # Returns
+///
+/// A `bool` indicating whether the list contains a range, starting at the first
+/// in the list, a`Version` of the last integer in the range, and a `usize` of
+/// the index of that version.
+///
+/// For example, if given vec![1, 2, 3, 5], find_range will return true,
+/// as there is a continuous range, and 3, which is the last number in the
+/// continuous range, and 2 which is the index of 3.
+fn find_range(list: &Vec<Version>) -> (bool, Version, usize) {
+    if list.len() == 0 {
+        return (false, 0, 0);
+    }
+
+    let mut index: usize = 0;
+    let mut iterable = list.iter().peekable();
+    let mut range_end = match iterable.next() {
+        Some(n) => *n,
+        None => return (false, 0, 0),
+    };
+
+    let mut has_range = false;
+
+    while iterable.peek().is_some() {
+        let n = *iterable.next().unwrap();
+        if n != range_end + 1 {
+            break;
+        }
+
+        has_range = true;
+        range_end = n;
+        index += 1;
+    }
+
+    (has_range, range_end, index)
+}
+
+impl From<Vec<Version>> for ProtoSet {
+    fn from(mut v: Vec<Version>) -> ProtoSet {
+        let mut version_pairs: Vec<(Version, Version)> = Vec::new();
+
+        v.sort_unstable();
+        v.dedup();
+
+        'vector: while !v.is_empty() {
+            let (has_range, end, index): (bool, Version, usize) = find_range(&v);
+
+            if has_range {
+                let first: Version = match v.first() {
+                    Some(x) => *x,
+                    None    => continue,
+                };
+                let last:  Version = match v.get(index) {
+                    Some(x) => *x,
+                    None    => continue,
+                };
+                debug_assert!(last == end, format!("last = {}, end = {}", last, end));
+
+                version_pairs.push((first, last));
+                v = v.split_off(index + 1);
+
+                if v.len() == 0 {
+                    break 'vector;
+                }
+            } else {
+                let last: Version = match v.get(index) {
+                    Some(x) => *x,
+                    None    => continue,
+                };
+                version_pairs.push((last, last));
+                v.remove(index);
+            }
+        }
+        ProtoSet::from_slice(&version_pairs[..]).unwrap_or(ProtoSet::default())
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_find_range() {
+        assert_eq!((false, 0, 0), find_range(&vec![]));
+        assert_eq!((false, 1, 0), find_range(&vec![1]));
+        assert_eq!((true, 2, 1), find_range(&vec![1, 2]));
+        assert_eq!((true, 3, 2), find_range(&vec![1, 2, 3]));
+        assert_eq!((true, 3, 2), find_range(&vec![1, 2, 3, 5]));
+    }
+
+    macro_rules! assert_contains_each {
+        ($protoset:expr, $versions:expr) => (
+            for version in $versions {
+                assert!($protoset.contains(version));
+            }
+        )
+    }
+
+    macro_rules! test_protoset_contains_versions {
+        ($list:expr, $str:expr) => (
+            let versions: &[Version] = $list;
+            let protoset: Result<ProtoSet, ProtoverError> = ProtoSet::from_str($str);
+
+            assert!(protoset.is_ok());
+            let p = protoset.unwrap();
+            assert_contains_each!(p, versions);
+        )
+    }
+
+    #[test]
+    fn test_versions_from_str() {
+        test_protoset_contains_versions!(&[], "");
+        test_protoset_contains_versions!(&[1], "1");
+        test_protoset_contains_versions!(&[1, 2], "1,2");
+        test_protoset_contains_versions!(&[1, 2, 3], "1-3");
+        test_protoset_contains_versions!(&[0, 1], "0-1");
+        test_protoset_contains_versions!(&[1, 2, 5], "1-2,5");
+        test_protoset_contains_versions!(&[1, 3, 4, 5], "1,3-5");
+        test_protoset_contains_versions!(&[42, 55, 56, 57, 58], "42,55-58");
+    }
+
+    #[test]
+    fn test_versions_from_str_ab() {
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("a,b"));
+    }
+
+    #[test]
+    fn test_versions_from_str_negative_1() {
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("-1"));
+    }
+
+    #[test]
+    fn test_versions_from_str_1exclam() {
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1,!"));
+    }
+
+    #[test]
+    fn test_versions_from_str_percent_equal() {
+        assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("%="));
+    }
+
+    #[test]
+    fn test_versions_from_str_overlap() {
+        assert_eq!(Err(ProtoverError::Overlap), ProtoSet::from_str("1-3,2-4"));
+    }
+
+    #[test]
+    fn test_versions_from_slice_overlap() {
+        assert_eq!(Err(ProtoverError::Overlap), ProtoSet::from_slice(&[(1, 3), (2, 4)]));
+    }
+
+    #[test]
+    fn test_versions_from_str_max() {
+        assert_eq!(Err(ProtoverError::ExceedsMax), ProtoSet::from_str("4294967295"));
+    }
+
+    #[test]
+    fn test_versions_from_slice_max() {
+        assert_eq!(Err(ProtoverError::ExceedsMax), ProtoSet::from_slice(&[(4294967295, 4294967295)]));
+    }
+
+    #[test]
+    fn test_protoset_contains() {
+        let protoset: ProtoSet = ProtoSet::from_slice(&[(0, 5), (7, 9), (13, 14)]).unwrap();
+
+        for x in 0..6   { assert!(protoset.contains(&x), format!("should contain {}", x)); }
+        for x in 7..10  { assert!(protoset.contains(&x), format!("should contain {}", x)); }
+        for x in 13..15 { assert!(protoset.contains(&x), format!("should contain {}", x)); }
+
+        for x in [6, 10, 11, 12, 15, 42, 43, 44, 45, 1234584].iter() {
+            assert!(!protoset.contains(&x), format!("should not contain {}", x));
+        }
+    }
+
+    #[test]
+    fn test_protoset_contains_0_3() {
+        let protoset: ProtoSet = ProtoSet::from_slice(&[(0, 3)]).unwrap();
+
+        for x in 0..4 { assert!(protoset.contains(&x), format!("should contain {}", x)); }
+    }
+
+    macro_rules! assert_protoset_from_vec_contains_all {
+        ($($x:expr),*) => (
+            let vec: Vec<Version> = vec!($($x),*);
+            let protoset: ProtoSet = vec.clone().into();
+
+            for x in vec.iter() {
+                assert!(protoset.contains(&x));
+            }
+        )
+    }
+
+    #[test]
+    fn test_protoset_from_vec_123() {
+        assert_protoset_from_vec_contains_all!(1, 2, 3);
+    }
+
+    #[test]
+    fn test_protoset_from_vec_0_315() {
+        assert_protoset_from_vec_contains_all!(0, 1, 2, 3, 15);
+    }
+
+    #[test]
+    fn test_protoset_from_vec_unordered() {
+        let v: Vec<Version> = vec!(2, 3, 8, 4, 3, 9, 7, 2);
+        let ps: ProtoSet = v.into();
+
+        assert_eq!(ps.to_string(), "2-4,7-9");
+    }
+
+    #[test]
+    fn test_protoset_into_vec() {
+        let ps: ProtoSet = "1-13,42,9001,4294967294".parse().unwrap();
+        let v: Vec<Version> = ps.into();
+
+        assert!(v.contains(&7));
+        assert!(v.contains(&9001));
+        assert!(v.contains(&4294967294));
+    }
+}
+
+#[cfg(all(test, feature = "bench"))]
+mod bench {
+    use super::*;
+}

src/rust/protover/tests/protover.rs

@@ -3,289 +3,392 @@
 
 extern crate protover;
 
+use protover::ProtoEntry;
+use protover::ProtoverVote;
+use protover::UnvalidatedProtoEntry;
+use protover::errors::ProtoverError;
+
 #[test]
-fn parse_protocol_list_with_single_proto_and_single_version() {
-    let protocol = "Cons=1";
-    let (is_supported, unsupported) = protover::all_supported(protocol);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+fn parse_protocol_with_single_proto_and_single_version() {
+    let _: ProtoEntry = "Cons=1".parse().unwrap();
 }
 
 #[test]
-fn parse_protocol_list_with_single_protocol_and_multiple_versions() {
-    let protocol = "Cons=1-2";
-    let (is_supported, unsupported) = protover::all_supported(protocol);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+fn parse_protocol_with_single_protocol_and_multiple_versions() {
+    let _: ProtoEntry = "Cons=1-2".parse().unwrap();
 }
 
 #[test]
-fn parse_protocol_list_with_different_single_protocol_and_single_version() {
-    let protocol = "HSDir=1";
-    let (is_supported, unsupported) = protover::all_supported(protocol);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+fn parse_protocol_with_different_single_protocol_and_single_version() {
+    let _: ProtoEntry = "HSDir=1".parse().unwrap();
 }
 
 #[test]
-fn parse_protocol_list_with_single_protocol_and_supported_version() {
-    let protocol = "Desc=2";
-    let (is_supported, unsupported) = protover::all_supported(protocol);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+fn parse_protocol_with_single_protocol_and_supported_version() {
+    let _: ProtoEntry = "Desc=2".parse().unwrap();
 }
 
 #[test]
-fn parse_protocol_list_with_two_protocols_and_single_version() {
-    let protocols = "Cons=1 HSDir=1";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+fn parse_protocol_with_two_protocols_and_single_version() {
+    let _: ProtoEntry = "Cons=1 HSDir=1".parse().unwrap();
 }
 
-
 #[test]
-fn parse_protocol_list_with_single_protocol_and_two_nonsequential_versions() {
-    let protocol = "Desc=1,2";
-    let (is_supported, unsupported) = protover::all_supported(protocol);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+fn parse_protocol_with_single_protocol_and_two_sequential_versions() {
+    let _: ProtoEntry = "Desc=1-2".parse().unwrap();
 }
 
+#[test]
+fn parse_protocol_with_single_protocol_and_protocol_range() {
+    let _: ProtoEntry = "Link=1-4".parse().unwrap();
+}
 
 #[test]
-fn parse_protocol_list_with_single_protocol_and_two_sequential_versions() {
-    let protocol = "Desc=1-2";
-    let (is_supported, unsupported) = protover::all_supported(protocol);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+fn parse_protocol_with_single_protocol_and_protocol_set() {
+    let _: ProtoEntry = "Link=3-4 Desc=2".parse().unwrap();
 }
 
 #[test]
-fn parse_protocol_list_with_single_protocol_and_protocol_range_returns_set() {
-    let protocol = "Link=1-4";
-    let (is_supported, unsupported) = protover::all_supported(protocol);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+fn protocol_all_supported_with_single_protocol_and_protocol_set() {
+    let protocols: UnvalidatedProtoEntry = "Link=3-4 Desc=2".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_none());
 }
 
 #[test]
-fn parse_protocol_list_with_single_protocol_and_protocol_set() {
-    let protocols = "Link=3-4 Desc=2";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+fn protocol_all_supported_with_two_values() {
+    let protocols: UnvalidatedProtoEntry = "Microdesc=1-2 Relay=2".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_none());
 }
 
 #[test]
-fn protover_all_supported_with_two_values() {
-    let protocols = "Microdesc=1-2 Relay=2";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!("", &unsupported);
-    assert_eq!(true, is_supported);
+fn protocol_all_supported_with_one_value() {
+    let protocols: UnvalidatedProtoEntry = "Microdesc=1-2".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_none());
 }
 
 #[test]
-fn protover_all_supported_with_one_value() {
-    let protocols = "Microdesc=1-2";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!("", &unsupported);
-    assert_eq!(true, is_supported);
+#[should_panic]
+fn parse_protocol_unvalidated_with_empty() {
+    let _: UnvalidatedProtoEntry = "".parse().unwrap();
 }
 
 #[test]
-fn protover_all_supported_with_empty() {
-    let protocols = "";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!(true, is_supported);
-    assert_eq!("", &unsupported);
+#[should_panic]
+fn parse_protocol_validated_with_empty() {
+    let _: UnvalidatedProtoEntry = "".parse().unwrap();
 }
 
 #[test]
-fn protover_all_supported_with_three_values() {
-    let protocols = "LinkAuth=1 Microdesc=1-2 Relay=2";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!("", &unsupported);
-    assert_eq!(true, is_supported);
+fn protocol_all_supported_with_three_values() {
+    let protocols: UnvalidatedProtoEntry = "LinkAuth=1 Microdesc=1-2 Relay=2".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_none());
 }
 
 #[test]
-fn protover_all_supported_with_unsupported_protocol() {
-    let protocols = "Wombat=9";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!(false, is_supported);
-    assert_eq!("Wombat=9", &unsupported);
+fn protocol_all_supported_with_unsupported_protocol() {
+    let protocols: UnvalidatedProtoEntry = "Wombat=9".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_some());
+    assert_eq!("Wombat=9", &unsupported.unwrap().to_string());
 }
 
 #[test]
-fn protover_all_supported_with_unsupported_versions() {
-    let protocols = "Link=3-999";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!(false, is_supported);
-    assert_eq!("Link=3-999", &unsupported);
+fn protocol_all_supported_with_unsupported_versions() {
+    let protocols: UnvalidatedProtoEntry = "Link=3-999".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_some());
+    assert_eq!("Link=6-999", &unsupported.unwrap().to_string());
 }
 
 #[test]
-fn protover_all_supported_with_unsupported_low_version() {
-    let protocols = "Cons=0-1";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!(false, is_supported);
-    assert_eq!("Cons=0-1", &unsupported);
+fn protocol_all_supported_with_unsupported_low_version() {
+    let protocols: UnvalidatedProtoEntry = "Cons=0-1".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_some());
+    assert_eq!("Cons=0", &unsupported.unwrap().to_string());
 }
 
 #[test]
-fn protover_all_supported_with_unsupported_high_version() {
-    let protocols = "Cons=1-3";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!(false, is_supported);
-    assert_eq!("Cons=1-3", &unsupported);
+fn protocol_all_supported_with_unsupported_high_version() {
+    let protocols: UnvalidatedProtoEntry = "Cons=1-2,999".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_some());
+    assert_eq!("Cons=999", &unsupported.unwrap().to_string());
 }
 
 #[test]
-fn protover_all_supported_with_mix_of_supported_and_unsupproted() {
-    let protocols = "Link=3-4 Wombat=9";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!(false, is_supported);
-    assert_eq!("Wombat=9", &unsupported);
+fn protocol_all_supported_with_mix_of_supported_and_unsupproted() {
+    let protocols: UnvalidatedProtoEntry = "Link=3-4 Wombat=9".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_some());
+    assert_eq!("Wombat=9", &unsupported.unwrap().to_string());
 }
 
 #[test]
 fn protover_string_supports_protocol_returns_true_for_single_supported() {
-    let protocols = "Link=3-4 Cons=1";
-    let is_supported = protover::protover_string_supports_protocol(
-        protocols,
-        protover::Proto::Cons,
-        1,
-    );
+    let protocols: UnvalidatedProtoEntry = "Link=3-4 Cons=1".parse().unwrap();
+    let is_supported = protocols.supports_protocol(&protover::Protocol::Cons.into(), &1);
     assert_eq!(true, is_supported);
 }
 
 #[test]
 fn protover_string_supports_protocol_returns_false_for_single_unsupported() {
-    let protocols = "Link=3-4 Cons=1";
-    let is_supported = protover::protover_string_supports_protocol(
-        protocols,
-        protover::Proto::Cons,
-        2,
-    );
+    let protocols: UnvalidatedProtoEntry = "Link=3-4 Cons=1".parse().unwrap();
+    let is_supported = protocols.supports_protocol(&protover::Protocol::Cons.into(), &2);
     assert_eq!(false, is_supported);
 }
 
 #[test]
 fn protover_string_supports_protocol_returns_false_for_unsupported() {
-    let protocols = "Link=3-4";
-    let is_supported = protover::protover_string_supports_protocol(
-        protocols,
-        protover::Proto::Cons,
-        2,
-    );
+    let protocols: UnvalidatedProtoEntry = "Link=3-4".parse().unwrap();
+    let is_supported = protocols.supports_protocol(&protover::Protocol::Cons.into(), &2);
     assert_eq!(false, is_supported);
 }
 
 #[test]
-fn protover_all_supported_with_unexpected_characters() {
-    let protocols = "Cons=*-%";
-    let (is_supported, unsupported) = protover::all_supported(protocols);
-    assert_eq!(false, is_supported);
-    assert_eq!("Cons=*-%", &unsupported);
+#[should_panic]
+fn parse_protocol_with_unexpected_characters() {
+    let _: UnvalidatedProtoEntry = "Cons=*-%".parse().unwrap();
 }
 
 #[test]
+#[should_panic]
 fn protover_compute_vote_returns_empty_for_empty_string() {
-    let protocols = vec![String::from("")];
-    let listed = protover::compute_vote(protocols, 1);
-    assert_eq!("", listed);
+    let protocols: &[UnvalidatedProtoEntry] = &["".parse().unwrap()];
+    let listed = ProtoverVote::compute(protocols, &1);
+    assert_eq!("", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_returns_single_protocol_for_matching() {
-    let protocols = vec![String::from("Cons=1")];
-    let listed = protover::compute_vote(protocols, 1);
-    assert_eq!("Cons=1", listed);
+    let protocols: &[UnvalidatedProtoEntry] = &["Cons=1".parse().unwrap()];
+    let listed = ProtoverVote::compute(protocols, &1);
+    assert_eq!("Cons=1", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_returns_two_protocols_for_two_matching() {
-    let protocols = vec![String::from("Link=1 Cons=1")];
-    let listed = protover::compute_vote(protocols, 1);
-    assert_eq!("Cons=1 Link=1", listed);
+    let protocols: &[UnvalidatedProtoEntry] = &["Link=1 Cons=1".parse().unwrap()];
+    let listed = ProtoverVote::compute(protocols, &1);
+    assert_eq!("Cons=1 Link=1", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_returns_one_protocol_when_one_out_of_two_matches() {
-    let protocols = vec![String::from("Cons=1 Link=2"), String::from("Cons=1")];
-    let listed = protover::compute_vote(protocols, 2);
-    assert_eq!("Cons=1", listed);
+    let protocols: &[UnvalidatedProtoEntry] = &["Cons=1 Link=2".parse().unwrap(), "Cons=1".parse().unwrap()];
+    let listed = ProtoverVote::compute(protocols, &2);
+    assert_eq!("Cons=1", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_returns_protocols_that_it_doesnt_currently_support() {
-    let protocols = vec![String::from("Foo=1 Cons=2"), String::from("Bar=1")];
-    let listed = protover::compute_vote(protocols, 1);
-    assert_eq!("Bar=1 Cons=2 Foo=1", listed);
+    let protocols: &[UnvalidatedProtoEntry] = &["Foo=1 Cons=2".parse().unwrap(), "Bar=1".parse().unwrap()];
+    let listed = ProtoverVote::compute(protocols, &1);
+    assert_eq!("Bar=1 Cons=2 Foo=1", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_returns_matching_for_mix() {
-    let protocols = vec![String::from("Link=1-10,500 Cons=1,3-7,8")];
-    let listed = protover::compute_vote(protocols, 1);
-    assert_eq!("Cons=1,3-8 Link=1-10,500", listed);
+    let protocols: &[UnvalidatedProtoEntry] = &["Link=1-10,500 Cons=1,3-7,8".parse().unwrap()];
+    let listed = ProtoverVote::compute(protocols, &1);
+    assert_eq!("Cons=1,3-8 Link=1-10,500", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_returns_matching_for_longer_mix() {
-    let protocols = vec![
-        String::from("Desc=1-10,500 Cons=1,3-7,8"),
-        String::from("Link=123-456,78 Cons=2-6,8 Desc=9"),
+    let protocols: &[UnvalidatedProtoEntry] = &[
+        "Desc=1-10,500 Cons=1,3-7,8".parse().unwrap(),
+        "Link=123-456,78 Cons=2-6,8 Desc=9".parse().unwrap(),
     ];
 
-    let listed = protover::compute_vote(protocols, 1);
-    assert_eq!("Cons=1-8 Desc=1-10,500 Link=78,123-456", listed);
+    let listed = ProtoverVote::compute(protocols, &1);
+    assert_eq!("Cons=1-8 Desc=1-10,500 Link=78,123-456", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_returns_matching_for_longer_mix_with_threshold_two() {
-    let protocols = vec![
-        String::from("Desc=1-10,500 Cons=1,3-7,8"),
-        String::from("Link=123-456,78 Cons=2-6,8 Desc=9"),
+    let protocols: &[UnvalidatedProtoEntry] = &[
+        "Desc=1-10,500 Cons=1,3-7,8".parse().unwrap(),
+        "Link=123-456,78 Cons=2-6,8 Desc=9".parse().unwrap(),
     ];
 
-    let listed = protover::compute_vote(protocols, 2);
-    assert_eq!("Cons=3-6,8 Desc=9", listed);
+    let listed = ProtoverVote::compute(protocols, &2);
+    assert_eq!("Cons=3-6,8 Desc=9", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_handles_duplicated_versions() {
-    let protocols = vec![String::from("Cons=1"), String::from("Cons=1")];
-    assert_eq!("Cons=1", protover::compute_vote(protocols, 2));
+    let protocols: &[UnvalidatedProtoEntry] = &["Cons=1".parse().unwrap(), "Cons=1".parse().unwrap()];
+    assert_eq!("Cons=1", ProtoverVote::compute(protocols, &2).to_string());
 
-    let protocols = vec![String::from("Cons=1-2"), String::from("Cons=1-2")];
-    assert_eq!("Cons=1-2", protover::compute_vote(protocols, 2));
+    let protocols: &[UnvalidatedProtoEntry] = &["Cons=1-2".parse().unwrap(), "Cons=1-2".parse().unwrap()];
+    assert_eq!("Cons=1-2", ProtoverVote::compute(protocols, &2).to_string());
 }
 
 #[test]
 fn protover_compute_vote_handles_invalid_proto_entries() {
-    let protocols = vec![
-        String::from("Cons=1"),
-        String::from("Cons=1"),
-        String::from("Link=a"),
+    let protocols: &[UnvalidatedProtoEntry] = &[
+        "Cons=1".parse().unwrap(),
+        "Cons=1".parse().unwrap(),
+        "Dinosaur=1".parse().unwrap(),
     ];
-    assert_eq!("Cons=1", protover::compute_vote(protocols, 2));
+    assert_eq!("Cons=1", ProtoverVote::compute(protocols, &2).to_string());
+}
 
-    let protocols = vec![
-        String::from("Cons=1"),
-        String::from("Cons=1"),
-        String::from("Link=1-%"),
-    ];
-    assert_eq!("Cons=1", protover::compute_vote(protocols, 2));
+#[test]
+fn parse_protocol_with_single_protocol_and_two_nonsequential_versions() {
+    let _: ProtoEntry = "Desc=1,2".parse().unwrap();
 }
 
 #[test]
 fn protover_is_supported_here_returns_true_for_supported_protocol() {
-    assert_eq!(true, protover::is_supported_here(protover::Proto::Cons, 1));
+    assert_eq!(true, protover::is_supported_here(&protover::Protocol::Cons, &1));
 }
 
 #[test]
 fn protover_is_supported_here_returns_false_for_unsupported_protocol() {
-    assert_eq!(false, protover::is_supported_here(protover::Proto::Cons, 5));
+    assert_eq!(false, protover::is_supported_here(&protover::Protocol::Cons, &5));
+}
+
+#[test]
+fn protocol_all_supported_with_single_proto_and_single_version() {
+    let protocol: UnvalidatedProtoEntry = "Cons=1".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocol.all_supported();
+    assert_eq!(true, unsupported.is_none());
+}
+
+#[test]
+fn protocol_all_supported_with_single_protocol_and_multiple_versions() {
+    let protocol: UnvalidatedProtoEntry = "Cons=1-2".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocol.all_supported();
+    assert_eq!(true, unsupported.is_none());
+}
+
+#[test]
+fn protocol_all_supported_with_different_single_protocol_and_single_version() {
+    let protocol: UnvalidatedProtoEntry = "HSDir=1".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocol.all_supported();
+    assert_eq!(true, unsupported.is_none());
+}
+
+#[test]
+fn protocol_all_supported_with_single_protocol_and_supported_version() {
+    let protocol: UnvalidatedProtoEntry = "Desc=2".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocol.all_supported();
+    assert_eq!(true, unsupported.is_none());
+}
+
+#[test]
+fn protocol_all_supported_with_two_protocols_and_single_version() {
+    let protocols: UnvalidatedProtoEntry = "Cons=1 HSDir=1".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
+    assert_eq!(true, unsupported.is_none());
+}
+
+#[test]
+fn protocol_all_supported_with_single_protocol_and_two_nonsequential_versions() {
+    let protocol: UnvalidatedProtoEntry = "Desc=1,2".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocol.all_supported();
+    assert_eq!(true, unsupported.is_none());
+}
+
+#[test]
+fn protocol_all_supported_with_single_protocol_and_two_sequential_versions() {
+    let protocol: UnvalidatedProtoEntry = "Desc=1-2".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocol.all_supported();
+    assert_eq!(true, unsupported.is_none());
+}
+
+#[test]
+fn protocol_all_supported_with_single_protocol_and_protocol_range() {
+    let protocol: UnvalidatedProtoEntry = "Link=1-4".parse().unwrap();
+    let unsupported: Option<UnvalidatedProtoEntry> = protocol.all_supported();
+    assert_eq!(true, unsupported.is_none());
+}
+
+// By allowing us to add to votes, the C implementation allows us to
+// exceed the limit.
+#[test]
+fn protover_compute_vote_may_exceed_limit() {
+    let proto1: UnvalidatedProtoEntry = "Sleen=1-65535".parse().unwrap();
+    let proto2: UnvalidatedProtoEntry = "Sleen=100000".parse().unwrap();
+
+    let _result: UnvalidatedProtoEntry = ProtoverVote::compute(&[proto1, proto2], &1);
+}
+
+#[test]
+fn protover_all_supported_should_exclude_versions_we_actually_do_support() {
+    let proto: UnvalidatedProtoEntry = "Link=3-999".parse().unwrap();
+    let result: String = proto.all_supported().unwrap().to_string();
+
+    assert_eq!(result, "Link=6-999".to_string());
+}
+
+#[test]
+fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex1() {
+    let proto: UnvalidatedProtoEntry = "Link=1-3,345-666".parse().unwrap();
+    let result: String = proto.all_supported().unwrap().to_string();
+
+    assert_eq!(result, "Link=345-666".to_string());
+}
+
+#[test]
+fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex2() {
+    let proto: UnvalidatedProtoEntry = "Link=1-3,5-12".parse().unwrap();
+    let result: String = proto.all_supported().unwrap().to_string();
+
+    assert_eq!(result, "Link=6-12".to_string());
+}
+
+#[test]
+fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() {
+    let proto: UnvalidatedProtoEntry = "Link=1-3,5-12 Quokka=9000-9001".parse().unwrap();
+    let result: String = proto.all_supported().unwrap().to_string();
+
+    assert_eq!(result, "Link=6-12 Quokka=9000-9001".to_string());
+}
+
+#[test]
+fn protover_all_supported_should_not_dos_anyones_computer() {
+    let proto: UnvalidatedProtoEntry = "Sleen=0-2147483648".parse().unwrap();
+    let result: String = proto.all_supported().unwrap().to_string();
+
+    assert_eq!(result, "Sleen=0-2147483648".to_string());
+}
+
+#[test]
+fn protover_all_supported_should_not_dos_anyones_computer_max_versions() {
+    let proto: UnvalidatedProtoEntry = "Sleen=0-4294967294".parse().unwrap();
+    let result: String = proto.all_supported().unwrap().to_string();
+
+    assert_eq!(result, "Sleen=0-4294967294".to_string());
+}
+
+#[test]
+// C_RUST_DIFFERS: The C will return true (e.g. saying "yes, that's supported")
+// but set the msg to NULL (??? seems maybe potentially bad).  The Rust will
+// simply return a None.
+fn protover_all_supported_should_return_empty_string_for_weird_thing() {
+    let proto: UnvalidatedProtoEntry = "Fribble=".parse().unwrap();
+    let result: Option<UnvalidatedProtoEntry> = proto.all_supported();
+
+    assert!(result.is_none());
+}
+
+#[test]
+fn protover_unvalidatedprotoentry_should_err_entirely_unparseable_things() {
+    let proto: Result<UnvalidatedProtoEntry, ProtoverError> = "Fribble".parse();
+
+    assert_eq!(Err(ProtoverError::Unparseable), proto);
+}
+
+#[test]
+fn protover_all_supported_over_maximum_limit() {
+    let proto: Result<UnvalidatedProtoEntry, ProtoverError> = "Sleen=0-4294967295".parse();
+
+    assert_eq!(Err(ProtoverError::ExceedsMax), proto);
 }

src/test/test_protover.c

@@ -254,11 +254,26 @@ test_protover_all_supported(void *arg)
   tt_assert(! protover_all_supported("Link=3-4 Wombat=9", &msg));
   tt_str_op(msg, OP_EQ, "Wombat=9");
   tor_free(msg);
+
+  /* Mix of things we support and don't support within a single protocol
+   * which we do support */
   tt_assert(! protover_all_supported("Link=3-999", &msg));
-  tt_str_op(msg, OP_EQ, "Link=3-999");
+  tt_str_op(msg, OP_EQ, "Link=6-999");
+  tor_free(msg);
+  tt_assert(! protover_all_supported("Link=1-3,345-666", &msg));
+  tt_str_op(msg, OP_EQ, "Link=345-666");
+  tor_free(msg);
+  tt_assert(! protover_all_supported("Link=1-3,5-12", &msg));
+  tt_str_op(msg, OP_EQ, "Link=6-12");
   tor_free(msg);
 
-  /* CPU/RAM DoS loop: Rust only */
+  /* Mix of protocols we do support and some we don't, where the protocols
+   * we do support have some versions we don't support. */
+  tt_assert(! protover_all_supported("Link=1-3,5-12 Quokka=9000-9001", &msg));
+  tt_str_op(msg, OP_EQ, "Link=6-12 Quokka=9000-9001");
+  tor_free(msg);
+
+  /* We shouldn't be able to DoS ourselves parsing a large range. */
   tt_assert(! protover_all_supported("Sleen=0-2147483648", &msg));
   tt_str_op(msg, OP_EQ, "Sleen=0-2147483648");
   tor_free(msg);
@@ -268,23 +283,22 @@ test_protover_all_supported(void *arg)
   tt_str_op(msg, OP_EQ, "Sleen=0-4294967294");
   tor_free(msg);
 
-  /* If we get an unparseable list, we say "yes, that's supported." */
-#ifndef HAVE_RUST
-  // XXXX let's make this section unconditional: rust should behave the
-  // XXXX same as C here!
+  /* If we get a (barely) valid (but unsupported list, we say "yes, that's
+   * supported." */
+  tt_assert(protover_all_supported("Fribble=", &msg));
+  tt_ptr_op(msg, OP_EQ, NULL);
+
+  /* If we get a completely unparseable list, protover_all_supported should
+   * hit a fatal assertion for BUG(entries == NULL). */
   tor_capture_bugs_(1);
   tt_assert(protover_all_supported("Fribble", &msg));
-  tt_ptr_op(msg, OP_EQ, NULL);
   tor_end_capture_bugs_();
 
-  /* This case is forbidden. Since it came from a protover_all_supported,
-   * it can trigger a bug message.  */
+  /* If we get a completely unparseable list, protover_all_supported should
+   * hit a fatal assertion for BUG(entries == NULL). */
   tor_capture_bugs_(1);
   tt_assert(protover_all_supported("Sleen=0-4294967295", &msg));
-  tt_ptr_op(msg, OP_EQ, NULL);
-  tor_free(msg);
   tor_end_capture_bugs_();
-#endif
 
  done:
   tor_end_capture_bugs_();
@@ -531,8 +545,6 @@ test_protover_vote_roundtrip(void *args)
     { "Link=1,9-8,3", NULL },
     { "Faux=-0", NULL },
     { "Faux=0--0", NULL },
-    // "These fail at the splitting stage in Rust, but the number parsing
-    // stage in C."
     { "Faux=-1", NULL },
     { "Faux=-1-3", NULL },
     { "Faux=1--1", NULL },
@@ -541,9 +553,9 @@ test_protover_vote_roundtrip(void *args)
     /* Large range */
     { "Sleen=1-501", "Sleen=1-501" },
     { "Sleen=1-65537", NULL },
-    /* CPU/RAM DoS Loop: Rust only. */
+    /* Both C/Rust implementations should be able to handle this mild DoS. */
     { "Sleen=0-2147483648", NULL },
-    /* Rust seems to experience an internal error here. */
+    /* Rust tests are built in debug mode, so ints are bounds-checked. */
     { "Sleen=0-4294967295", NULL },
   };
   unsigned u;