-
Notifications
You must be signed in to change notification settings - Fork 139
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
Avoid panics #392
Avoid panics #392
Changes from all commits
b0b1e20
af69022
48cc6bf
ea9a85a
ca3f3a9
b28c5c4
53aac78
ae493c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,7 +105,7 @@ pub fn headers() -> HeaderMap { | |
headers | ||
} | ||
|
||
pub fn signature<S: Signer>(signer: &S, headers: &HeaderMap) -> HeaderValue { | ||
pub fn signature<S: Signer>(signer: &S, headers: &HeaderMap) -> Result<HeaderValue, ()> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you briefly explain this return signature? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It says that it can either success with a |
||
let signed_string = headers | ||
.iter() | ||
.map(|(h, v)| { | ||
|
@@ -125,13 +125,13 @@ pub fn signature<S: Signer>(signer: &S, headers: &HeaderMap) -> HeaderValue { | |
.join(" ") | ||
.to_lowercase(); | ||
|
||
let data = signer.sign(&signed_string); | ||
let data = signer.sign(&signed_string).map_err(|_| ())?; | ||
let sign = base64::encode(&data); | ||
|
||
HeaderValue::from_str(&format!( | ||
"keyId=\"{key_id}\",algorithm=\"rsa-sha256\",headers=\"{signed_headers}\",signature=\"{signature}\"", | ||
key_id = signer.get_key_id(), | ||
signed_headers = signed_headers, | ||
signature = sign | ||
)).expect("request::signature: signature header error") | ||
)).map_err(|_| ()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i thought we're trying to avoid panics?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahah, right… It actually doesn't add a panic, we just panic "outside" of the function instead of "inside" because it wouldn't make much sense to return a
Result
frombroadcast
as it is always the main function of its thread.