8000 Add new hex parse error variant by tcharding · Pull Request #1584 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add new hex parse error variant #1584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 30, 2023
Merged

Add new hex parse error variant #1584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion bitcoin/src/bip32.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,8 @@ pub enum Error {
Base58(base58::Error),
/// Hexadecimal decoding error
Hex(hex::Error),
/// `PublicKey` hex should be 66 or 130 digits long.
InvalidPublicKeyHexLength(usize),
}

impl fmt::Display for Error {
Expand All @@ -473,6 +475,7 @@ impl fmt::Display for Error {
write!(f, "encoded extended key data has wrong length {}", len),
Error::Base58(ref e) => write_err!(f, "base58 encoding error"; e),
Error::Hex(ref e) => write_err!(f, "Hexadecimal decoding error"; e),
Error::InvalidPublicKeyHexLength(got) => write!(f, "PublicKey hex should be 66 or 130 digits long, got: {}", got),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use user friendly descriptions rather than type names in error messages but I guess there are other instances, so we may want to do complete audit and cleanup.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are on the topic, error messages should not be capitialised, right? I always forget and we have so many of both forms.

Copy link
Member Author
@tcharding tcharding Jan 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted however not to use types in error messages. Will leave it as it is for now since upcoming error work is imminent.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error messages should not be capitialised, right?

I think so.

}
}
}
Expand All @@ -492,7 +495,8 @@ impl std::error::Error for Error {
| InvalidChildNumberFormat
| InvalidDerivationPathFormat
| UnknownVersion(_)
| WrongExtendedKeyLength(_) => None,
| WrongExtendedKeyLength(_)
| InvalidPublicKeyHexLength(_) => None,
}
}
}
Expand All @@ -504,6 +508,7 @@ impl From<key::Error> for Error {
key::Error::Secp256k1(e) => Error::Secp256k1(e),
key::Error::InvalidKeyPrefix(_) => Error::Secp256k1(secp256k1::Error::InvalidPublicKey),
key::Error::Hex(e) => Error::Hex(e),
key::Error::InvalidHexLength(got) => Error::InvalidPublicKeyHexLength(got),
}
}
}
Expand Down
32 changes: 27 additions & 5 deletions bitcoin/src/crypto/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ pub enum Error {
/// Invalid key prefix error
InvalidKeyPrefix(u8),
/// Hex decoding error
Hex(hex::Error)
Hex(hex::Error),
/// `PublicKey` hex should be 66 or 130 digits long.
InvalidHexLength(usize),
}

impl fmt::Display for Error {
Expand All @@ -39,7 +41,8 @@ impl fmt::Display for Error {
Error::Base58(ref e) => write_err!(f, "key base58 error"; e),
Error::Secp256k1(ref e) => write_err!(f, "key secp256k1 error"; e),
Error::InvalidKeyPrefix(ref b) => write!(f, "key prefix invalid: {}", b),
Error::Hex(ref e) => write_err!(f, "key hex decoding error"; e)
Error::Hex(ref e) => write_err!(f, "key hex decoding error"; e),
Error::InvalidHexLength(got) => write!(f, "PublicKey hex should be 66 or 130 digits long, got: {}", got),
}
}
}
Expand All @@ -53,8 +56,8 @@ impl std::error::Error for Error {
match self {
Base58(e) => Some(e),
Secp256k1(e) => Some(e),
InvalidKeyPrefix(_) => None,
Hex(e) => Some(e),
InvalidKeyPrefix(_) | InvalidHexLength(_) => None,
}
}
}
Expand Down Expand Up @@ -159,7 +162,8 @@ impl PublicKey {
Error::Base58(_) => "base58 error",
Error::Secp256k1(_) => "secp256k1 error",
Error::InvalidKeyPrefix(_) => "invalid key prefix",
Error::Hex(_) => "hex decoding error"
Error::Hex(_) => "hex decoding error",
Error::InvalidHexLength(_) => "invalid hex string length",
};
io::Error::new(io::ErrorKind::InvalidData, reason)
})
Expand Down Expand Up @@ -287,7 +291,7 @@ impl FromStr for PublicKey {
match s.len() {
66 => PublicKey::from_slice(&<[u8; 33]>::from_hex(s)?),
130 => PublicKey::from_slice(&<[u8; 65]>::from_hex(s)?),
len => Err(Error::Hex(hex::Error::InvalidLength(66, len))),
len => Err(Error::InvalidHexLength(len)),
}
}
}
Expand Down Expand Up @@ -851,4 +855,22 @@ mod tests {
let _ = PublicKey::new(kp);
let _ = PublicKey::new_uncompressed(kp);
}

#[test]
fn public_key_from_str_wrong_length() {
// Sanity checks, we accept string length 130 digits.
let s = "042e58afe51f9ed8ad3cc7897f634d881fdbe49a81564629ded8156bebd2ffd1af191923a2964c177f5b5923ae500fca49e99492d534aa3759d6b25a8bc971b133";
assert_eq!(s.len(), 130);
assert!(PublicKey::from_str(s).is_ok());
// And 66 digits.
let s = "032e58afe51f9ed8ad3cc7897f634d881fdbe49a81564629ded8156bebd2ffd1af";
assert_eq!(s.len(), 66);
assert!(PublicKey::from_str(s).is_ok());

let s = "aoeusthb";
assert_eq!(s.len(), 8);
let res = PublicKey::from_str(s);
assert!(res.is_err());
assert_eq!(res.unwrap_err(), Error::InvalidHexLength(8));
}
}
0