milksad
/
rust-bitcoin-unsafe-fast
14
0
Fork 0
Code Issues Pull Requests Activity

enforce strict SI(treat capital of m, u, n, p as invalid) in parsing amount denomiation. add disallow_unknown_denomination test

This commit is contained in:
KaFai Choi 2022-01-11 21:49:58 +07:00
parent e80de8b1ee
commit 40f38b3edc
No known key found for this signature in database
GPG Key ID: C93944C459E21542
1 changed files with 33 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
src/util/amount.rs
View File

@ -76,22 +76,26 @@ impl FromStr for Denomination {
/// Convert from a str to Denomination. /// Convert from a str to Denomination.
/// ///
/// Any combination of upper and/or lower case, excluding uppercase 'M', 'P' is considered valid. /// Any combination of upper and/or lower case, excluding uppercase of SI(m, u, n, p) is considered valid.
/// - Singular: BTC, mBTC, uBTC, nBTC, pBTC /// - Singular: BTC, mBTC, uBTC, nBTC, pBTC
/// - Plural or singular: sat, satoshi, bit, msat /// - Plural or singular: sat, satoshi, bit, msat
/// ///
/// Due to ambiguity between mega and milli we prohibit usage of leading capital 'M'. /// Due to ambiguity between mega and milli, pico and peta we prohibit usage of leading capital 'M', 'P'.
fn from_str(s: &str) -> Result<Self, Self::Err> { fn from_str(s: &str) -> Result<Self, Self::Err> {
use self::ParseAmountError::*; use self::ParseAmountError::*;
use self::Denomination as D;
if s.starts_with(|ch| ch == 'M' || ch == 'P') { let starts_with_uppercase = || s.starts_with(|ch: char| ch.is_uppercase());
return Err(denomination_from_str(s).map_or_else( match denomination_from_str(s) {
|| UnknownDenomination(s.to_owned()), None => Err(UnknownDenomination(s.to_owned())),
|_| PossiblyConfusingDenomination(s.to_owned()) Some(D::MilliBitcoin) | Some(D::PicoBitcoin) | Some(D::MilliSatoshi) if starts_with_uppercase() => {
)); Err(PossiblyConfusingDenomination(s.to_owned()))
}
Some(D::NanoBitcoin) | Some(D::MicroBitcoin) if starts_with_uppercase() => {
Err(UnknownDenomination(s.to_owned()))
}
Some(d) => Ok(d),
} }
denomination_from_str(s).ok_or_else(|| UnknownDenomination(s.to_owned()))
} }
} }
@ -169,7 +173,13 @@ impl fmt::Display for ParseAmountError {
ParseAmountError::InvalidCharacter(c) => write!(f, "invalid character in input: {}", c), ParseAmountError::InvalidCharacter(c) => write!(f, "invalid character in input: {}", c),
ParseAmountError::UnknownDenomination(ref d) => write!(f, "unknown denomination: {}", d), ParseAmountError::UnknownDenomination(ref d) => write!(f, "unknown denomination: {}", d),
ParseAmountError::PossiblyConfusingDenomination(ref d) => { ParseAmountError::PossiblyConfusingDenomination(ref d) => {
write!(f, "the 'M' at the beginning of {} should technically mean 'Mega' but that denomination is uncommon and maybe 'milli' was intended", d) let (letter, upper, lower) = match d.chars().next() {
Some('M') => ('M', "Mega", "milli"),
Some('P') => ('P',"Peta", "pico"),
// This panic could be avoided by adding enum ConfusingDenomination { Mega, Peta } but is it worth it?
_ => panic!("invalid error information"),
};
write!(f, "the '{}' at the beginning of {} should technically mean '{}' but that denomination is uncommon and maybe '{}' was intended", letter, d, upper, lower)
} }
} }
} }
@ -1744,5 +1754,18 @@ mod tests {
} }
} }
} }
#[test]
fn disallow_unknown_denomination() {
// Non-exhaustive list of unknown forms.
let unknown = vec!["NBTC", "UBTC", "ABC", "abc"];
for denom in unknown.iter() {
match Denomination::from_str(denom) {
Ok(_) => panic!("from_str should error for {}", denom),
Err(ParseAmountError::UnknownDenomination(_)) => {},
Err(e) => panic!("unexpected error: {}", e),
}
}
}
} }