Skip to content
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.

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

crypto: lazy_static removed, light parser for address URI added #2250

Merged
merged 32 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5b233fc
crypto: lazy_static removed, light parser added
michalkucharczyk Nov 9, 2023
3ab3733
fuzzer added
michalkucharczyk Nov 9, 2023
7e6bdb3
fix
michalkucharczyk Nov 9, 2023
f92f50f
cleanup
michalkucharczyk Nov 9, 2023
1a52c98
license added
michalkucharczyk Nov 9, 2023
b0167c3
std gate added
michalkucharczyk Nov 9, 2023
b6dbf0d
address uri moved to src/
michalkucharczyk Nov 9, 2023
eaf5de0
Apply suggestions from code review
michalkucharczyk Nov 13, 2023
047eb83
parsing error added
michalkucharczyk Nov 13, 2023
b33de54
fix
michalkucharczyk Nov 13, 2023
ee7f179
Merge remote-tracking branch 'origin/master' into mku-crypto-address-…
michalkucharczyk Nov 13, 2023
a3b9ded
Update substrate/primitives/core/src/address_uri.rs
michalkucharczyk Nov 13, 2023
3aad2c5
Update substrate/primitives/core/src/address_uri.rs
michalkucharczyk Nov 13, 2023
d556091
review suggestions
michalkucharczyk Nov 13, 2023
40486a6
fuzzer fixed
michalkucharczyk Nov 13, 2023
77ad7f1
review suggestion
michalkucharczyk Nov 13, 2023
93bc851
naming
michalkucharczyk Nov 13, 2023
33dd1f3
missed review suggesions + fixes
michalkucharczyk Nov 13, 2023
0476638
Update substrate/primitives/core/src/address_uri.rs
michalkucharczyk Nov 13, 2023
0dddd34
fix
michalkucharczyk Nov 13, 2023
2500fe0
Merge remote-tracking branch 'origin/master' into mku-crypto-address-…
michalkucharczyk Nov 13, 2023
52f8aa2
Merge branch 'master' into mku-crypto-address-uri-parser
michalkucharczyk Nov 13, 2023
abff740
".git/.scripts/commands/update-ui/update-ui.sh"
Nov 13, 2023
b2bb315
test-pallet-ui: trybuild-overwrite
michalkucharczyk Nov 13, 2023
13582d3
Merge branch 'master' into mku-crypto-address-uri-parser
michalkucharczyk Nov 13, 2023
9fa5ff8
better error communication
michalkucharczyk Nov 16, 2023
80532e7
fuzzer added to workspace
michalkucharczyk Nov 16, 2023
69d7c7f
fix
michalkucharczyk Nov 16, 2023
ad71461
fix
michalkucharczyk Nov 16, 2023
ab059c3
fix indexing to be 0-based
michalkucharczyk Nov 16, 2023
f6c7bfd
Merge branch 'master' into mku-crypto-address-uri-parser
michalkucharczyk Nov 16, 2023
06dd2fd
Merge branch 'master' into mku-crypto-address-uri-parser
michalkucharczyk Nov 17, 2023
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
45 changes: 36 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ members = [
"substrate/primitives/consensus/sassafras",
"substrate/primitives/consensus/slots",
"substrate/primitives/core",
"substrate/primitives/core/fuzz",
"substrate/primitives/core/hashing",
"substrate/primitives/core/hashing/proc-macro",
"substrate/primitives/crypto/ec-utils",
Expand Down
7 changes: 0 additions & 7 deletions substrate/primitives/core/fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,6 @@ regex = "1.10.2"

sp-core = { path = ".." }

# Prevent this from interfering with workspaces
[workspace]
members = ["."]

[profile.release]
debug = 1

[[bin]]
name = "fuzz_address_uri"
path = "fuzz_targets/fuzz_address_uri.rs"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ fuzz_target!(|input: &str| {
let regex_result = SECRET_PHRASE_REGEX.captures(input);
let manual_result = AddressUri::parse(input);
assert_eq!(regex_result.is_some(), manual_result.is_ok());
if manual_result.is_err() {
let _ = format!("{}", manual_result.as_ref().err().unwrap());
}
if let (Some(regex_result), Ok(manual_result)) = (regex_result, manual_result) {
assert_eq!(regex_result.name("phrase").map(|p| p.as_str()), manual_result.phrase);

Expand Down
175 changes: 147 additions & 28 deletions substrate/primitives/core/src/address_uri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,82 @@ pub struct AddressUri<'a> {
/// Errors that are possible during parsing the address URI.
#[allow(missing_docs)]
#[cfg_attr(feature = "std", derive(thiserror::Error))]
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum Error {
#[cfg_attr(feature = "std", error("Invalid character in phrase"))]
InvalidCharacterInPhrase,
#[cfg_attr(feature = "std", error("Invalid character in password"))]
InvalidCharacterInPass,
#[cfg_attr(feature = "std", error("Invalid character in hard path"))]
InvalidCharacterInHardPath,
#[cfg_attr(feature = "std", error("Invalid character in soft path"))]
InvalidCharacterInSoftPath,
#[cfg_attr(feature = "std", error("Invalid character in phrase:\n{0}"))]
InvalidCharacterInPhrase(InvalidCharacterInfo),
#[cfg_attr(feature = "std", error("Invalid character in password:\n{0}"))]
InvalidCharacterInPass(InvalidCharacterInfo),
#[cfg_attr(feature = "std", error("Missing character in hard path:\n{0}"))]
MissingCharacterInHardPath(InvalidCharacterInfo),
#[cfg_attr(feature = "std", error("Missing character in soft path:\n{0}"))]
MissingCharacterInSoftPath(InvalidCharacterInfo),
}

impl Error {
/// Creates an instance of `Error::InvalidCharacterInPhrase` using given parameters.
pub fn in_phrase(input: &str, pos: usize) -> Self {
Self::InvalidCharacterInPhrase(InvalidCharacterInfo::new(input, pos))
}
/// Creates an instance of `Error::InvalidCharacterInPass` using given parameters.
pub fn in_pass(input: &str, pos: usize) -> Self {
Self::InvalidCharacterInPass(InvalidCharacterInfo::new(input, pos))
}
/// Creates an instance of `Error::MissingCharacterInHardPath` using given parameters.
pub fn in_hard_path(input: &str, pos: usize) -> Self {
Self::MissingCharacterInHardPath(InvalidCharacterInfo::new(input, pos))
}
/// Creates an instance of `Error::MissingCharacterInSoftPath` using given parameters.
pub fn in_soft_path(input: &str, pos: usize) -> Self {
Self::MissingCharacterInSoftPath(InvalidCharacterInfo::new(input, pos))
}
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub struct InvalidCharacterInfo(String, usize);

impl InvalidCharacterInfo {
fn new(info: &str, pos: usize) -> Self {
Self(info.to_string(), pos)
}
}

impl sp_std::fmt::Display for InvalidCharacterInfo {
fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
let (s, width) = escape_string(&self.0, self.1);
write!(f, "{s}\n{i: >width$}", i = '^')
}
}

/// Escapes the control characters in given string, and recomputes the position if some characters
/// were actually escaped.
fn escape_string(input: &str, pos: usize) -> (String, usize) {
let mut out = String::with_capacity(2 * input.len());
let mut out_pos = 0;
input
.chars()
.enumerate()
.map(|(i, c)| {
let esc = |c| (i, Some('\\'), c, 2);
match c {
'\t' => esc('t'),
'\n' => esc('n'),
'\r' => esc('r'),
'\x07' => esc('a'),
'\x08' => esc('b'),
'\x0b' => esc('v'),
'\x0c' => esc('f'),
_ => (i, None, c, 1),
}
})
.for_each(|(i, maybe_escape, c, increment)| {
maybe_escape.map(|e| out.push(e));
out.push(c);
if i < pos {
out_pos += increment;
}
});
(out, out_pos)
}

fn extract_prefix<'a>(input: &mut &'a str, is_allowed: &dyn Fn(char) -> bool) -> Option<&'a str> {
Expand All @@ -80,6 +146,8 @@ fn strip_prefix(input: &mut &str, prefix: &str) -> bool {
impl<'a> AddressUri<'a> {
/// Parses the given string.
pub fn parse(mut input: &'a str) -> Result<Self, Error> {
let initial_input = input;
let initial_input_len = input.len();
let phrase = extract_prefix(&mut input, &|ch: char| {
ch.is_ascii_digit() || ch.is_ascii_alphabetic() || ch == ' '
});
Expand All @@ -91,22 +159,22 @@ impl<'a> AddressUri<'a> {
if strip_prefix(&mut input, "///") {
pass = Some(extract_prefix(&mut input, &|ch: char| ch != '\n').unwrap_or(""));
} else if strip_prefix(&mut input, "//") {
let path = extract_prefix(&mut input, &|ch: char| ch != '/')
.ok_or(Error::InvalidCharacterInHardPath)?;
let path = extract_prefix(&mut input, &|ch: char| ch != '/').ok_or(
Error::in_hard_path(initial_input, initial_input_len - input.len() + 1),
)?;
assert!(path.len() > 0);
// hard path shall contain leading '/', so take it from unstripped input.
paths.push(&unstripped_input[1..path.len() + 2]);
} else if strip_prefix(&mut input, "/") {
paths.push(
extract_prefix(&mut input, &|ch: char| ch != '/')
.ok_or(Error::InvalidCharacterInSoftPath)?,
);
paths.push(extract_prefix(&mut input, &|ch: char| ch != '/').ok_or(
Error::in_soft_path(initial_input, initial_input_len - input.len() + 1),
)?);
} else {
return if pass.is_some() {
Err(Error::InvalidCharacterInPass)
return Err(if pass.is_some() {
Error::in_pass(initial_input, initial_input_len - input.len() + 1)
} else {
Err(Error::InvalidCharacterInPhrase)
};
Error::in_phrase(initial_input, initial_input_len - input.len() + 1)
});
}
}

Expand Down Expand Up @@ -188,7 +256,8 @@ mod tests {

#[test]
fn test05() {
check("sdasd//", Err(Error::InvalidCharacterInHardPath));
let input = "sdasd//";
check(input, Err(Error::in_hard_path(input, 8)));
}

#[test]
Expand Down Expand Up @@ -221,7 +290,8 @@ mod tests {

#[test]
fn test09() {
check("sdasd/xx//", Err(Error::InvalidCharacterInHardPath));
let input = "sdasd/xx//";
check(input, Err(Error::in_hard_path(input, 11)));
}

#[test]
Expand All @@ -247,7 +317,8 @@ mod tests {

#[test]
fn test13() {
check("sdasd/", Err(Error::InvalidCharacterInSoftPath));
let input = "sdasd/";
check(input, Err(Error::in_soft_path(input, 7)));
}

#[test]
Expand All @@ -257,17 +328,20 @@ mod tests {

#[test]
fn test15() {
check("sd.asd", Err(Error::InvalidCharacterInPhrase));
let input = "sdasd.";
check(input, Err(Error::in_phrase(input, 6)));
}

#[test]
fn test16() {
check("sd.asd/asd.a", Err(Error::InvalidCharacterInPhrase));
let input = "sd.asd/asd.a";
check(input, Err(Error::in_phrase(input, 3)));
}

#[test]
fn test17() {
check("sd.asd//asd.a", Err(Error::InvalidCharacterInPhrase));
let input = "sd.asd//asd.a";
check(input, Err(Error::in_phrase(input, 3)));
}

#[test]
Expand All @@ -288,16 +362,61 @@ mod tests {

#[test]
fn test20() {
check("///\n", Err(Error::InvalidCharacterInPass));
let input = "///\n";
check(input, Err(Error::in_pass(input, 4)));
}

#[test]
fn test21() {
check("///a\n", Err(Error::InvalidCharacterInPass));
let input = "///a\n";
check(input, Err(Error::in_pass(input, 5)));
}

#[test]
fn test22() {
check("sd.asd///asd.a\n", Err(Error::InvalidCharacterInPhrase));
let input = "sd asd///asd.a\n";
check(input, Err(Error::in_pass(input, 15)));
}

#[test]
fn test_invalid_char_info_1() {
let expected = "12345\n^";
let f = format!("{}", InvalidCharacterInfo::new("12345", 1));
assert_eq!(expected, f);
}

#[test]
fn test_invalid_char_info_2() {
let expected = "1\n^";
let f = format!("{}", InvalidCharacterInfo::new("1", 1));
assert_eq!(expected, f);
}

#[test]
fn test_invalid_char_info_3() {
let expected = "12345\n ^";
let f = format!("{}", InvalidCharacterInfo::new("12345", 3));
assert_eq!(expected, f);
}

#[test]
fn test_invalid_char_info_4() {
let expected = "123\\n567\n ^";
let f = format!("{}", InvalidCharacterInfo::new("123\n567", 4));
assert_eq!(expected, f);
}

#[test]
fn test_invalid_char_info_5() {
let expected = "123\\n567\n ^";
Copy link
Member

Choose a reason for hiding this comment

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

Nice! ❤️

let f = format!("{}", InvalidCharacterInfo::new("123\n567", 7));
assert_eq!(expected, f);
}

#[test]
fn test_invalid_char_info_6() {
let expected = "123\\f567\\t90\n ^";
let f = format!("{}", InvalidCharacterInfo::new("123\x0c567\t90", 10));
assert_eq!(expected, f);
}
}
2 changes: 1 addition & 1 deletion substrate/primitives/core/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ impl<T: AsRef<str>> From<T> for DeriveJunction {
/// An error type for SS58 decoding.
#[cfg_attr(feature = "std", derive(thiserror::Error))]
#[cfg_attr(not(feature = "std"), derive(Debug))]
#[derive(Clone, Copy, Eq, PartialEq)]
#[derive(Clone, Eq, PartialEq)]
#[allow(missing_docs)]
#[cfg(any(feature = "full_crypto", feature = "serde"))]
pub enum PublicError {
Expand Down