Validation consistency between format & validation modules

The best place to catch share problems is immediately during parsing from
`&str`, however, because `validate_shares` takes any type that implements the
`IsShare` trait, and there's nothing about that trait that guarantees that the
share id, threshold, and secret length will be valid, I thought it best to leave
those three tests in `validate_shares` as a defensive coding practice.
This commit is contained in:
Noah Vesely
2018-03-29 01:22:54 -06:00
committed by Romain Ruetschi
parent 88743caad8
commit 3f215cdb39
3 changed files with 18 additions and 12 deletions

View File

@ -40,6 +40,9 @@ pub(crate) fn validate_shares<S: IsShare>(shares: &Vec<S>) -> Result<(u8, usize)
share.get_data().len(), share.get_data().len(),
); );
// Public-facing `Share::share_from_string` performs these three tests, but in case another
// type which implements `IsShare` is implemented later that doesn't do that validation,
// we'll leave them.
if id < 1 { if id < 1 {
bail!(ErrorKind::ShareParsingInvalidShareId(id)) bail!(ErrorKind::ShareParsingInvalidShareId(id))
} else if threshold_ < 2 { } else if threshold_ < 2 {

View File

@ -49,12 +49,12 @@ pub(crate) fn share_from_string(s: &str, is_signed: bool) -> Result<Share> {
(k, i, p3) (k, i, p3)
}; };
if k < 1 || i < 1 { if i < 1 {
bail! { bail!(ErrorKind::ShareParsingInvalidShareId(i))
ErrorKind::ShareParsingError( } else if k < 2 {
format!("Found illegal share info: threshold = {}, identifier = {}.", k, i), bail!(ErrorKind::ShareParsingInvalidShareThreshold(k, i))
) } else if p3.is_empty() {
} bail!(ErrorKind::ShareParsingErrorEmptyShare(i))
} }
let raw_data = base64::decode_config(p3, BASE64_CONFIG).chain_err(|| { let raw_data = base64::decode_config(p3, BASE64_CONFIG).chain_err(|| {

View File

@ -11,6 +11,13 @@ fn test_recover_no_shares() {
} }
} }
#[test]
#[should_panic(expected = "ShareParsingErrorEmptyShare")]
fn test_share_parsing_error_empty_share() {
let shares = vec!["2-1-".to_string()];
recover_secret(&shares, false).unwrap();
}
#[test] #[test]
#[should_panic(expected = "ShareParsingError")] #[should_panic(expected = "ShareParsingError")]
fn test_recover_2_parts_share() { fn test_recover_2_parts_share() {
@ -34,13 +41,9 @@ fn test_recover_incorrect_share_num() {
} }
#[test] #[test]
#[should_panic(expected = "ShareParsingError")] #[should_panic(expected = "ShareParsingInvalidShareId")]
fn test_recover_0_share_num() { fn test_recover_0_share_num() {
let share1 = "2-0-1YAYwmOHqZ69jA".to_string(); let shares = vec!["2-0-1YAYwmOHqZ69jA".to_string()];
let share2 = "2-1-YJZQDGm22Y77Gw".to_string();
let shares = vec![share1, share2];
recover_secret(&shares, false).unwrap(); recover_secret(&shares, false).unwrap();
} }