commit 3c47d31e1f29a016e2f0f21ca8445430ed7aad0a Author: Isis Lovecruft isis@torproject.org Date: Wed Mar 21 01:29:36 2018 +0000
rust: Add new protover::UnvalidatedProtoEntry type.
This adds a new protover::UnvalidatedProtoEntry type, which is the UnknownProtocol variant of a ProtoEntry, and refactors several functions which should operate on this type into methods.
This also fixes what was previously another difference to the C implementation: if you asked the C version of protovet_compute_vote() to compute a single vote containing "Fribble=", it would return NULL. However, the Rust version would return "Fribble=" since it didn't check if the versions were empty before constructing the string of differences. ("Fribble=" is technically a valid protover string.) This is now fixed, and the Rust version in that case will, analogous to (although safer than) C returning a NULL, return None.
* REMOVE internal `contains_only_supported_protocols()` function. * REMOVE `all_supported()` function and refactor it into `UnvalidatedProtoEntry::all_supported()`. * REMOVE `parse_protocols_from_string_with_no_validation()` and refactor it into the more rusty implementation of `impl FromStr for UnvalidatedProtoEntry`. * REMOVE `protover_string_supports_protocol()` and refactor it into `UnvalidatedProtoEntry::supports_protocol()`. * REMOVE `protover_string_supports_protocol_or_later()` and refactor it into `UnvalidatedProtoEntry::supports_protocol_or_later()`. * FIXES part of #24031: https://bugs.torproject.org/24031
rust: Fix another C/Rust different in compute_vote().
This fixes the unittest from the prior commit by checking if the versions are empty before adding a protocol to a vote. --- src/rust/protover/protover.rs | 388 ++++++++++++++++++++++-------------------- 1 file changed, 208 insertions(+), 180 deletions(-)
diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 1f62e70f1..847406ca2 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -226,207 +226,235 @@ impl FromStr for ProtoEntry { } }
-/// Parses a single subprotocol entry string into subprotocol and version -/// parts, and then checks whether any of those versions are unsupported. -/// Helper for protover::all_supported -/// -/// # Inputs -/// -/// Accepted data is in the string format as follows: -/// -/// "HSDir=1-1" -/// -/// # Returns -/// -/// Returns `true` if the protocol entry is well-formatted and only contains -/// versions that are also supported in tor. Otherwise, returns false -/// -fn contains_only_supported_protocols(proto_entry: &str) -> bool { - let (name, mut vers) = match get_proto_and_vers(proto_entry) { - Ok(n) => n, - Err(_) => return false, - }; - - let currently_supported = match SupportedProtocols::tor_supported() { - Ok(n) => n.0, - Err(_) => return false, - }; - - let supported_versions = match currently_supported.get(&name) { - Some(n) => n, - None => return false, - }; +/// A `ProtoEntry`, but whose `Protocols` can be any `UnknownProtocol`, not just +/// the supported ones enumerated in `Protocols`. The protocol versions are +/// validated, however. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UnvalidatedProtoEntry(HashMap<UnknownProtocol, ProtoSet>);
- vers.0.retain(|x| !supported_versions.0.contains(x)); - vers.0.is_empty() +impl Default for UnvalidatedProtoEntry { + fn default() -> UnvalidatedProtoEntry { + UnvalidatedProtoEntry( HashMap::new() ) + } }
-/// Determine if we support every protocol a client supports, and if not, -/// determine which protocols we do not have support for. -/// -/// # Inputs -/// -/// Accepted data is in the string format as follows: -/// -/// "HSDir=1-1 LinkAuth=1-2" -/// -/// # Returns -/// -/// Return `true` if every protocol version is one that we support. -/// Otherwise, return `false`. -/// Optionally, return parameters which the client supports but which we do not -/// -/// # Examples -/// ``` -/// use protover::all_supported; -/// -/// let (is_supported, unsupported) = all_supported("Link=1"); -/// assert_eq!(true, is_supported); -/// -/// let (is_supported, unsupported) = all_supported("Link=5-6"); -/// assert_eq!(false, is_supported); -/// assert_eq!("Link=5-6", unsupported); -/// -pub fn all_supported(protocols: &str) -> (bool, String) { - let unsupported = protocols - .split_whitespace() - .filter(|v| !contains_only_supported_protocols(v)) - .collect::<Vec<&str>>(); +impl UnvalidatedProtoEntry { + /// Get an iterator over the `Protocol`s and their `ProtoSet`s in this `ProtoEntry`. + pub fn iter(&self) -> hash_map::Iter<UnknownProtocol, ProtoSet> { + self.0.iter() + }
- (unsupported.is_empty(), unsupported.join(" ")) -} + pub fn get(&self, protocol: &UnknownProtocol) -> Option<&ProtoSet> { + self.0.get(protocol) + }
-/// Return true iff the provided protocol list includes support for the -/// indicated protocol and version. -/// Otherwise, return false -/// -/// # Inputs -/// -/// * `list`, a string representation of a list of protocol entries. -/// * `proto`, a `Proto` to test support for -/// * `vers`, a `Version` version which we will go on to determine whether the -/// specified protocol supports. -/// -/// # Examples -/// ``` -/// use protover::*; -/// -/// let is_supported = protover_string_supports_protocol("Link=3-4 Cons=1", -/// Proto::Cons,1); -/// assert_eq!(true, is_supported); -/// -/// let is_not_supported = protover_string_supports_protocol("Link=3-4 Cons=1", -/// Proto::Cons,5); -/// assert_eq!(false, is_not_supported) -/// ``` -pub fn protover_string_supports_protocol( - list: &str, - proto: Proto, - vers: Version, -) -> bool { - let supported = match SupportedProtocols::from_proto_entries_string(list) { - Ok(result) => result.0, - Err(_) => return false, - }; + pub fn insert(&mut self, key: UnknownProtocol, value: ProtoSet) { + self.0.insert(key, value); + }
- let supported_versions = match supported.get(&proto) { - Some(n) => n, - None => return false, - }; + pub fn remove(&mut self, key: &UnknownProtocol) -> Option<ProtoSet> { + self.0.remove(key) + }
- supported_versions.0.contains(&vers) -} + pub fn is_empty(&self) -> bool { + self.0.is_empty() + }
-/// As protover_string_supports_protocol(), but also returns True if -/// any later version of the protocol is supported. -/// -/// # Examples -/// ``` -/// use protover::*; -/// -/// let is_supported = protover_string_supports_protocol_or_later( -/// "Link=3-4 Cons=5", Proto::Cons, 5); -/// -/// assert_eq!(true, is_supported); -/// -/// let is_supported = protover_string_supports_protocol_or_later( -/// "Link=3-4 Cons=5", Proto::Cons, 4); -/// -/// assert_eq!(true, is_supported); -/// -/// let is_supported = protover_string_supports_protocol_or_later( -/// "Link=3-4 Cons=5", Proto::Cons, 6); -/// -/// assert_eq!(false, is_supported); -/// ``` -pub fn protover_string_supports_protocol_or_later( - list: &str, - proto: Proto, - vers: u32, -) -> bool { - let supported = match SupportedProtocols::from_proto_entries_string(list) { - Ok(result) => result.0, - Err(_) => return false, - }; + /// Determine if we support every protocol a client supports, and if not, + /// determine which protocols we do not have support for. + /// + /// # Returns + /// + /// Optionally, return parameters which the client supports but which we do not. + /// + /// # Examples + /// ``` + /// use protover::UnvalidatedProtoEntry; + /// + /// 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()); + /// + /// let protocols: UnvalidatedProtoEntry = "Link=1-2 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()); + /// ``` + pub fn all_supported(&self) -> Option<UnvalidatedProtoEntry> { + let mut unsupported: UnvalidatedProtoEntry = UnvalidatedProtoEntry::default(); + let supported: ProtoEntry = match ProtoEntry::supported() { + Ok(x) => x, + Err(_) => return None, + };
- let supported_versions = match supported.get(&proto) { - Some(n) => n, - None => return false, - }; + for (protocol, versions) in self.iter() { + let is_supported: Result<Protocol, ProtoverError> = protocol.0.parse(); + let supported_protocol: Protocol; + + // If the protocol wasn't even in the enum, then we definitely don't + // know about it and don't support any of its versions. + if is_supported.is_err() { + if !versions.is_empty() { + unsupported.insert(protocol.clone(), versions.clone()); + } + continue; + } else { + supported_protocol = is_supported.unwrap(); + }
- supported_versions.0.iter().any(|v| v >= &vers) -} + let maybe_supported_versions: Option<&ProtoSet> = supported.get(&supported_protocol); + let supported_versions: &ProtoSet; + let mut unsupported_versions: ProtoSet; + + // If the protocol wasn't in the map, then we don't know about it + // and don't support any of its versions. Add its versions to the + // map (if it has versions). + if maybe_supported_versions.is_none() { + if !versions.is_empty() { + unsupported.insert(protocol.clone(), versions.clone()); + } + continue; + } else { + supported_versions = maybe_supported_versions.unwrap(); + } + unsupported_versions = versions.clone(); + unsupported_versions.retain(|x| !supported_versions.contains(x));
-/// Parses a protocol list without validating the protocol names -/// -/// # Inputs -/// -/// * `protocol_string`, a string comprised of keys and values, both which are -/// strings. The keys are the protocol names while values are a string -/// representation of the supported versions. -/// -/// The input is _not_ expected to be a subset of the Proto types -/// -/// # Returns -/// -/// A `Result` whose `Ok` value is a `HashSet<Version>` holding all of the -/// unique version numbers. -/// -/// The returned `Result`'s `Err` value is an `&'static str` with a description -/// of the error. -/// -/// # Errors -/// -/// This function will error if: -/// -/// * The protocol string does not follow the "protocol_name=version_list" -/// expected format -/// * If the version string is malformed. See `Versions::from_version_string`. -/// -fn parse_protocols_from_string_with_no_validation<'a>( - protocol_string: &'a str, -) -> Result<HashMap<String, Versions>, &'static str> { - let mut parsed: HashMap<String, Versions> = HashMap::new(); + if !unsupported_versions.is_empty() { + unsupported.insert(protocol.clone(), unsupported_versions); + } + }
- for subproto in protocol_string.split(" ") { - let mut parts = subproto.splitn(2, "="); + if unsupported.is_empty() { + return None; + } + Some(unsupported) + }
- let name = match parts.next() { - Some("") => return Err("invalid protover entry"), + /// Determine if we have support for some protocol and version. + /// + /// # Inputs + /// + /// * `proto`, an `UnknownProtocol` to test support for + /// * `vers`, a `Version` which we will go on to determine whether the + /// specified protocol supports. + /// + /// # Return + /// + /// Returns `true` iff this `UnvalidatedProtoEntry` includes support for the + /// indicated protocol and version, and `false` otherwise. + /// + /// # Examples + /// + /// ``` + /// # use std::str::FromStr; + /// use protover::*; + /// # use protover::errors::ProtoverError; + /// + /// # fn do_test () -> Result<UnvalidatedProtoEntry, ProtoverError> { + /// let proto: UnvalidatedProtoEntry = "Link=3-4 Cons=1 Doggo=3-5".parse()?; + /// assert_eq!(true, proto.supports_protocol(&Protocol::Cons.into(), &1)); + /// assert_eq!(false, proto.supports_protocol(&Protocol::Cons.into(), &5)); + /// assert_eq!(true, proto.supports_protocol(&UnknownProtocol::from_str("Doggo")?, &4)); + /// # Ok(proto) + /// # } fn main () { do_test(); } + /// ``` + pub fn supports_protocol(&self, proto: &UnknownProtocol, vers: &Version) -> bool { + let supported_versions: &ProtoSet = match self.get(proto) { Some(n) => n, - None => return Err("invalid protover entry"), + None => return false, }; + supported_versions.contains(&vers) + }
- let vers = match parts.next() { + /// As `UnvalidatedProtoEntry::supports_protocol()`, but also returns `true` + /// if any later version of the protocol is supported. + /// + /// # Examples + /// ``` + /// use protover::*; + /// # use protover::errors::ProtoverError; + /// + /// # fn do_test () -> Result<UnvalidatedProtoEntry, ProtoverError> { + /// let proto: UnvalidatedProtoEntry = "Link=3-4 Cons=5".parse()?; + /// + /// assert_eq!(true, proto.supports_protocol_or_later(&Protocol::Cons.into(), &5)); + /// assert_eq!(true, proto.supports_protocol_or_later(&Protocol::Cons.into(), &4)); + /// assert_eq!(false, proto.supports_protocol_or_later(&Protocol::Cons.into(), &6)); + /// # Ok(proto) + /// # } fn main () { do_test(); } + /// ``` + pub fn supports_protocol_or_later(&self, proto: &UnknownProtocol, vers: &Version) -> bool { + let supported_versions: &ProtoSet = match self.get(&proto) { Some(n) => n, - None => return Err("invalid protover entry"), + None => return false, }; + supported_versions.iter().any(|v| v.1 >= *vers) + } +} + +impl FromStr for UnvalidatedProtoEntry { + type Err = ProtoverError; + + /// Parses a protocol list without validating the protocol names. + /// + /// # Inputs + /// + /// * `protocol_string`, a string comprised of keys and values, both which are + /// strings. The keys are the protocol names while values are a string + /// representation of the supported versions. + /// + /// The input is _not_ expected to be a subset of the Protocol types + /// + /// # 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` whose `Display` + /// impl has a description of the error. + /// + /// # Errors + /// + /// This function will error if: + /// + /// * The protocol string does not follow the "protocol_name=version_list" + /// expected format, or + /// * If the version string is malformed. See `impl FromStr for ProtoSet`. + fn from_str(protocol_string: &str) -> Result<UnvalidatedProtoEntry, ProtoverError> { + let mut parsed: UnvalidatedProtoEntry = UnvalidatedProtoEntry::default();
- let versions = Versions::from_version_string(vers)?; + for subproto in protocol_string.split(' ') { + let mut parts = subproto.splitn(2, '=');
- parsed.insert(String::from(name), versions); + let name = match parts.next() { + Some("") => return Err(ProtoverError::Unparseable), + Some(n) => n, + None => return Err(ProtoverError::Unparseable), + }; + let vers = match parts.next() { + Some(n) => n, + None => return Err(ProtoverError::Unparseable), + }; + let versions = ProtoSet::from_str(vers)?; + let protocol = UnknownProtocol::from_str(name)?; + + parsed.insert(protocol, versions); + } + Ok(parsed) + } +} + +/// Pretend a `ProtoEntry` is actually an `UnvalidatedProtoEntry`. +impl From<ProtoEntry> for UnvalidatedProtoEntry { + fn from(proto_entry: ProtoEntry) -> UnvalidatedProtoEntry { + let mut unvalidated: UnvalidatedProtoEntry = UnvalidatedProtoEntry::default(); + + for (protocol, versions) in proto_entry.iter() { + unvalidated.insert(UnknownProtocol::from(protocol.clone()), versions.clone()); + } + unvalidated } - Ok(parsed) }
/// Protocol voting implementation.