netdoc: add a new type for Nicknames

Relay nicknames are always between 1 and 19 characters long, and
they're always ASCII: That means that storing them in a [u8;19] will
always be possible, and always use less resources than storing them
in a String.

Fortunately, the tinystr crate already helps us with this kind of
thing.
This commit is contained in:
Nick Mathewson 2022-03-10 16:38:23 -05:00
parent 4262e9d0ec
commit 3b0336e841
8 changed files with 122 additions and 11 deletions

21
Cargo.lock generated
View File

@ -994,6 +994,17 @@ dependencies = [
"winapi 0.3.9", "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]] [[package]]
name = "downcast-rs" name = "downcast-rs"
version = "1.2.0" version = "1.2.0"
@ -3142,6 +3153,15 @@ version = "0.2.4"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "42657b1a6f4d817cda8e7a0ace261fe0cc946cf3a80314390b22cc61ae080792" checksum = "42657b1a6f4d817cda8e7a0ace261fe0cc946cf3a80314390b22cc61ae080792"
[[package]]
name = "tinystr"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0949a998f5fdb6b70a86deb1c8b8e2119f981f59b1b050344dbd3c40d8894c59"
dependencies = [
"displaydoc",
]
[[package]] [[package]]
name = "tinyvec" name = "tinyvec"
version = "1.6.0" version = "1.6.0"
@ -3651,6 +3671,7 @@ dependencies = [
"signature", "signature",
"thiserror", "thiserror",
"time", "time",
"tinystr",
"tor-bytes", "tor-bytes",
"tor-cert", "tor-cert",
"tor-checkable", "tor-checkable",

View File

@ -60,6 +60,7 @@ once_cell = "1"
phf = { version = "0.10.0", features = ["macros"] } phf = { version = "0.10.0", features = ["macros"] }
serde = "1.0.103" serde = "1.0.103"
signature = "1" signature = "1"
tinystr = "0.5"
thiserror = "1" thiserror = "1"
visible = { version = "0.0.1", optional = true } visible = { version = "0.0.1", optional = true }
visibility = { version = "0.0.1", optional = true } visibility = { version = "0.0.1", optional = true }

View File

@ -39,7 +39,7 @@ struct GenericRouterStatus<D> {
/// ///
/// Nicknames can be used for convenience purpose, but no more: /// Nicknames can be used for convenience purpose, but no more:
/// there is no mechanism to enforce their uniqueness. /// there is no mechanism to enforce their uniqueness.
nickname: String, nickname: Nickname,
/// Fingerprint of the old-style RSA identity for this relay. /// Fingerprint of the old-style RSA identity for this relay.
identity: RsaIdentity, identity: RsaIdentity,
/// A list of address:port values where this relay can be reached. /// A list of address:port values where this relay can be reached.
@ -115,8 +115,8 @@ macro_rules! implement_accessors {
&self.rs.protos &self.rs.protos
} }
/// Return the nickname of this routerstatus. /// Return the nickname of this routerstatus.
pub fn nickname(&self) -> &String { pub fn nickname(&self) -> &str {
&self.rs.nickname self.rs.nickname.as_str()
} }
/// Return the relay flags of this routerstatus. /// Return the relay flags of this routerstatus.
pub fn flags(&self) -> &RelayFlags { pub fn flags(&self) -> &RelayFlags {
@ -177,7 +177,7 @@ where
use NetstatusKwd::*; use NetstatusKwd::*;
// R line // R line
let r_item = sec.required(RS_R)?; 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::<B64>()?; let ident = r_item.required_arg(1)?.parse::<B64>()?;
let identity = RsaIdentity::from_bytes(ident.as_bytes()).ok_or_else(|| { let identity = RsaIdentity::from_bytes(ident.as_bytes()).ok_or_else(|| {
EK::BadArgument EK::BadArgument

View File

@ -118,7 +118,7 @@ impl<D: Clone> RouterStatusBuilder<D> {
} }
/// Try to build a GenericRouterStatus from this builder. /// Try to build a GenericRouterStatus from this builder.
fn finish(&self) -> Result<GenericRouterStatus<D>> { fn finish(&self) -> Result<GenericRouterStatus<D>> {
let nickname = self.nickname.clone().unwrap_or_else(|| "Unnamed".into()); let nickname = self.nickname.as_deref().unwrap_or("Unnamed").parse()?;
let identity = self let identity = self
.identity .identity
.ok_or(Error::CannotBuild("Missing RSA identity"))?; .ok_or(Error::CannotBuild("Missing RSA identity"))?;

View File

@ -100,7 +100,7 @@ pub struct RouterDesc {
/// Human-readable nickname for this relay. /// Human-readable nickname for this relay.
/// ///
/// This is not secure, and not guaranteed to be unique. /// This is not secure, and not guaranteed to be unique.
nickname: String, nickname: Nickname,
/// IPv4 address for this relay. /// IPv4 address for this relay.
ipv4addr: Option<net::Ipv4Addr>, ipv4addr: Option<net::Ipv4Addr>,
/// IPv4 ORPort for this relay. /// IPv4 ORPort for this relay.
@ -481,10 +481,7 @@ impl RouterDesc {
let (nickname, ipv4addr, orport, dirport) = { let (nickname, ipv4addr, orport, dirport) = {
let rtrline = header.required(ROUTER)?; let rtrline = header.required(ROUTER)?;
( (
// Unwrap should be safe because above `required` should return rtrline.parse_arg::<Nickname>(0)?,
// an `Error::MissingToken` if `ROUTER` is not `Ok`
#[allow(clippy::unwrap_used)]
rtrline.arg(0).unwrap().to_string(),
Some(rtrline.parse_arg::<net::Ipv4Addr>(1)?), Some(rtrline.parse_arg::<net::Ipv4Addr>(1)?),
rtrline.parse_arg(2)?, rtrline.parse_arg(2)?,
// Skipping socksport. // Skipping socksport.
@ -815,7 +812,7 @@ mod test {
.check_signature()? .check_signature()?
.dangerously_assume_timely(); .dangerously_assume_timely();
assert_eq!(rd.nickname, "idun2"); assert_eq!(rd.nickname.as_str(), "idun2");
assert_eq!(rd.orport, 9001); assert_eq!(rd.orport, 9001);
assert_eq!(rd.dirport, 0); assert_eq!(rd.dirport, 0);

View File

@ -7,3 +7,6 @@ pub mod family;
pub(crate) mod misc; pub(crate) mod misc;
pub mod policy; pub mod policy;
pub mod version; pub mod version;
#[cfg(feature = "dangerous-expose-struct-fields")]
pub use misc::Nickname;

View File

@ -15,6 +15,11 @@ pub(crate) use fingerprint::*;
pub(crate) use rsa::*; pub(crate) use rsa::*;
pub(crate) use timeimpl::*; 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. /// Describes a value that van be decoded from a bunch of bytes.
/// ///
/// Used for decoding the objects between BEGIN and END tags. /// 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<MAX_NICKNAME_LEN>);
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<Self> {
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)] #[cfg(test)]
mod test { mod test {
#![allow(clippy::unwrap_used)] #![allow(clippy::unwrap_used)]
@ -519,4 +579,28 @@ mod test {
assert!("ffffffffff".parse::<Fingerprint>().is_err()); assert!("ffffffffff".parse::<Fingerprint>().is_err());
Ok(()) 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::<Nickname>().is_err());
assert!(not_ascii.parse::<Nickname>().is_err());
assert!(too_short.parse::<Nickname>().is_err());
assert!(other_invalid.parse::<Nickname>().is_err());
Ok(())
}
} }

View File

@ -31,6 +31,11 @@ MODIFIED: New APIs for CachedDir, Error.
BREAKING: Added new cache_trust element to DirMgrConfig. BREAKING: Added new cache_trust element to DirMgrConfig.
### tor-netdoc
BREAKING: Routerstatus::nickname() now returns &str, not &String.
### tor-persist ### tor-persist
+BREAKING: Replaced from_path with from_path_and_mistrust +BREAKING: Replaced from_path with from_path_and_mistrust