diff --git a/Cargo.lock b/Cargo.lock index f42c91306..5b7a16d3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -994,6 +994,17 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "displaydoc" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3bf95dc3f046b9da4f2d51833c0d3547d8564ef6910f5c1ed130306a75b92886" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "downcast-rs" version = "1.2.0" @@ -3142,6 +3153,15 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "42657b1a6f4d817cda8e7a0ace261fe0cc946cf3a80314390b22cc61ae080792" +[[package]] +name = "tinystr" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0949a998f5fdb6b70a86deb1c8b8e2119f981f59b1b050344dbd3c40d8894c59" +dependencies = [ + "displaydoc", +] + [[package]] name = "tinyvec" version = "1.6.0" @@ -3651,6 +3671,7 @@ dependencies = [ "signature", "thiserror", "time", + "tinystr", "tor-bytes", "tor-cert", "tor-checkable", diff --git a/crates/tor-netdoc/Cargo.toml b/crates/tor-netdoc/Cargo.toml index 6291904e0..8d7099015 100644 --- a/crates/tor-netdoc/Cargo.toml +++ b/crates/tor-netdoc/Cargo.toml @@ -60,6 +60,7 @@ once_cell = "1" phf = { version = "0.10.0", features = ["macros"] } serde = "1.0.103" signature = "1" +tinystr = "0.5" thiserror = "1" visible = { version = "0.0.1", optional = true } visibility = { version = "0.0.1", optional = true } diff --git a/crates/tor-netdoc/src/doc/netstatus/rs.rs b/crates/tor-netdoc/src/doc/netstatus/rs.rs index b985187ca..ab1e85f46 100644 --- a/crates/tor-netdoc/src/doc/netstatus/rs.rs +++ b/crates/tor-netdoc/src/doc/netstatus/rs.rs @@ -39,7 +39,7 @@ struct GenericRouterStatus { /// /// Nicknames can be used for convenience purpose, but no more: /// there is no mechanism to enforce their uniqueness. - nickname: String, + nickname: Nickname, /// Fingerprint of the old-style RSA identity for this relay. identity: RsaIdentity, /// A list of address:port values where this relay can be reached. @@ -115,8 +115,8 @@ macro_rules! implement_accessors { &self.rs.protos } /// Return the nickname of this routerstatus. - pub fn nickname(&self) -> &String { - &self.rs.nickname + pub fn nickname(&self) -> &str { + self.rs.nickname.as_str() } /// Return the relay flags of this routerstatus. pub fn flags(&self) -> &RelayFlags { @@ -177,7 +177,7 @@ where use NetstatusKwd::*; // R line let r_item = sec.required(RS_R)?; - let nickname = r_item.required_arg(0)?.to_string(); + let nickname = r_item.required_arg(0)?.parse()?; let ident = r_item.required_arg(1)?.parse::()?; let identity = RsaIdentity::from_bytes(ident.as_bytes()).ok_or_else(|| { EK::BadArgument diff --git a/crates/tor-netdoc/src/doc/netstatus/rs/build.rs b/crates/tor-netdoc/src/doc/netstatus/rs/build.rs index 6b6d84347..c76a8bf64 100644 --- a/crates/tor-netdoc/src/doc/netstatus/rs/build.rs +++ b/crates/tor-netdoc/src/doc/netstatus/rs/build.rs @@ -118,7 +118,7 @@ impl RouterStatusBuilder { } /// Try to build a GenericRouterStatus from this builder. fn finish(&self) -> Result> { - let nickname = self.nickname.clone().unwrap_or_else(|| "Unnamed".into()); + let nickname = self.nickname.as_deref().unwrap_or("Unnamed").parse()?; let identity = self .identity .ok_or(Error::CannotBuild("Missing RSA identity"))?; diff --git a/crates/tor-netdoc/src/doc/routerdesc.rs b/crates/tor-netdoc/src/doc/routerdesc.rs index c129c2335..4713c23d9 100644 --- a/crates/tor-netdoc/src/doc/routerdesc.rs +++ b/crates/tor-netdoc/src/doc/routerdesc.rs @@ -100,7 +100,7 @@ pub struct RouterDesc { /// Human-readable nickname for this relay. /// /// This is not secure, and not guaranteed to be unique. - nickname: String, + nickname: Nickname, /// IPv4 address for this relay. ipv4addr: Option, /// IPv4 ORPort for this relay. @@ -481,10 +481,7 @@ impl RouterDesc { let (nickname, ipv4addr, orport, dirport) = { let rtrline = header.required(ROUTER)?; ( - // Unwrap should be safe because above `required` should return - // an `Error::MissingToken` if `ROUTER` is not `Ok` - #[allow(clippy::unwrap_used)] - rtrline.arg(0).unwrap().to_string(), + rtrline.parse_arg::(0)?, Some(rtrline.parse_arg::(1)?), rtrline.parse_arg(2)?, // Skipping socksport. @@ -815,7 +812,7 @@ mod test { .check_signature()? .dangerously_assume_timely(); - assert_eq!(rd.nickname, "idun2"); + assert_eq!(rd.nickname.as_str(), "idun2"); assert_eq!(rd.orport, 9001); assert_eq!(rd.dirport, 0); diff --git a/crates/tor-netdoc/src/types.rs b/crates/tor-netdoc/src/types.rs index 26f1d1958..cdc0cbda7 100644 --- a/crates/tor-netdoc/src/types.rs +++ b/crates/tor-netdoc/src/types.rs @@ -7,3 +7,6 @@ pub mod family; pub(crate) mod misc; pub mod policy; pub mod version; + +#[cfg(feature = "dangerous-expose-struct-fields")] +pub use misc::Nickname; diff --git a/crates/tor-netdoc/src/types/misc.rs b/crates/tor-netdoc/src/types/misc.rs index c532bfe3e..58cb1b19f 100644 --- a/crates/tor-netdoc/src/types/misc.rs +++ b/crates/tor-netdoc/src/types/misc.rs @@ -15,6 +15,11 @@ pub(crate) use fingerprint::*; pub(crate) use rsa::*; pub(crate) use timeimpl::*; +#[cfg(feature = "dangerous-expose-struct-fields")] +pub use nickname::Nickname; +#[cfg(not(feature = "dangerous-expose-struct-fields"))] +pub(crate) use nickname::Nickname; + /// Describes a value that van be decoded from a bunch of bytes. /// /// Used for decoding the objects between BEGIN and END tags. @@ -390,6 +395,61 @@ mod fingerprint { } } +/// A type for relay nicknames +mod nickname { + use crate::{Error, ParseErrorKind as EK, Pos, Result}; + use tinystr::TinyAsciiStr; + + /// This is a strange limit, but it comes from Tor. + const MAX_NICKNAME_LEN: usize = 19; + + /// The nickname for a Tor relay. + /// + /// These nicknames are legacy mechanism that's occasionally useful in + /// debugging. They should *never* be used to uniquely identify relays; + /// nothing prevents two relays from having the same nickname. + /// + /// Nicknames are required to be ASCII, alphanumeric, and between 1 and 19 + /// characters inclusive. + #[cfg_attr(feature = "dangerous-expose-struct-fields", visibility::make(pub))] + #[derive(Clone, Debug)] + + pub(crate) struct Nickname(tinystr::TinyAsciiStr); + + impl Nickname { + /// Return a view of this nickname as a string slice. + pub(crate) fn as_str(&self) -> &str { + self.0.as_str() + } + } + + impl std::fmt::Display for Nickname { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.as_str().fmt(f) + } + } + + impl std::str::FromStr for Nickname { + type Err = Error; + + fn from_str(s: &str) -> Result { + let tiny = TinyAsciiStr::from_str(s).map_err(|_| { + EK::BadArgument + .at_pos(Pos::at(s)) + .with_msg("Invalid nickname") + })?; + + if tiny.is_ascii_alphanumeric() && !tiny.is_empty() { + Ok(Nickname(tiny)) + } else { + Err(EK::BadArgument + .at_pos(Pos::at(s)) + .with_msg("Invalid nickname")) + } + } + } +} + #[cfg(test)] mod test { #![allow(clippy::unwrap_used)] @@ -519,4 +579,28 @@ mod test { assert!("ffffffffff".parse::().is_err()); Ok(()) } + + #[test] + fn nickname() -> Result<()> { + let n: Nickname = "Foo".parse()?; + assert_eq!(n.as_str(), "Foo"); + assert_eq!(n.to_string(), "Foo"); + + let word = "Untr1gonometr1cally"; + assert_eq!(word.len(), 19); + let long: Nickname = word.parse()?; + assert_eq!(long.as_str(), word); + + let too_long = "abcdefghijklmnopqrstuvwxyz"; + let not_ascii = "Eyjafjallajökull"; + let too_short = ""; + let other_invalid = "contains space"; + assert!(not_ascii.len() <= 19); + assert!(too_long.parse::().is_err()); + assert!(not_ascii.parse::().is_err()); + assert!(too_short.parse::().is_err()); + assert!(other_invalid.parse::().is_err()); + + Ok(()) + } } diff --git a/doc/semver_status.md b/doc/semver_status.md index f45ed5443..bb0c04e8a 100644 --- a/doc/semver_status.md +++ b/doc/semver_status.md @@ -31,6 +31,11 @@ MODIFIED: New APIs for CachedDir, Error. BREAKING: Added new cache_trust element to DirMgrConfig. +### tor-netdoc + +BREAKING: Routerstatus::nickname() now returns &str, not &String. + ### tor-persist +BREAKING: Replaced from_path with from_path_and_mistrust +