Skip to content
Open
Changes from all commits
Commits
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
52 changes: 52 additions & 0 deletions tokio-boring/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,58 @@ impl<S> HandshakeError<S> {
_ => None,
}
}

/// Returns a reference to the inner SSL error, if this error was caused by
/// a SSL error.
pub fn as_ssl_error(&self) -> Option<&boring::error::ErrorStack> {
match &self.0 {
ssl::HandshakeError::Failure(s) => s.error().ssl_error(),
ssl::HandshakeError::SetupFailure(ref s) => Some(s),
_ => None,
Comment on lines +315 to +317
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this also handle HandshakeError::WouldBlock? Or is it impossible somehow for WouldBlock to be an SSL error?

Suggested change
ssl::HandshakeError::Failure(s) => s.error().ssl_error(),
ssl::HandshakeError::SetupFailure(ref s) => Some(s),
_ => None,
ssl::HandshakeError::Failure(s) | ssl::HandshakeError::WouldBlock(s) => s.error().ssl_error(),
ssl::HandshakeError::SetupFailure(ref s) => Some(s),

}
}

/// Consumes `self` and returns the inner I/O error by value, if this error
/// was caused by an I/O error.
///
/// If this error was not caused by an I/O error, returns `Err(self)` so
/// that the error value can be reused.
pub fn into_io_error(self) -> Result<io::Error, Self> {
match self.0 {
ssl::HandshakeError::Failure(s) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ssl::HandshakeError::Failure(s) => {
ssl::HandshakeError::Failure(s) | ssl::HandshakeError::WouldBlock(s) => {

if s.error().io_error().is_some() {
Ok(s.into_error().into_io_error().expect(
"if `s.error().io_error().is_some()`, `into_io_error()` must succeed",
))
Comment on lines +330 to +332
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could avoid the expect() here by adding a new MidHandshakeSslStream::from_parts function and using into_parts() above.

} else {
Err(Self(ssl::HandshakeError::Failure(s)))
}
}
_ => Err(self),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_ => Err(self),
ssl::HandshakeError::SetupFailure(_) => Err(self),

}
}

/// Consumes `self` and returns the inner SSL error by value, if this error
/// was caused by a SSL error.
///
/// If this error was not caused by a SSL error, returns `Err(self)` so
/// that the error value can be reused.
pub fn into_ssl_error(self) -> Result<boring::error::ErrorStack, Self> {
match self.0 {
// TODO(eliza): it's not great that we have to clone in this case,
// but `boring::ssl::Error` doesn't currently expose an
// `into_ssl_error` the way it does for `io::Error`s. We could add
// that in a separate PR, but adding that here would make
// `tokio-boring` depend on a new release of `boring`...
ssl::HandshakeError::Failure(s) => s
.error()
.ssl_error()
.cloned()
.ok_or_else(|| Self(ssl::HandshakeError::Failure(s))),
Comment on lines +348 to +357
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather add into_ssl_error for boring too, if you don't mind - you can change them both in the same PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I was trying to limit the API additions because I wasn't sure what would be preferred by the maintainers, but I'm happy to change that.

ssl::HandshakeError::SetupFailure(stack) => Ok(stack),
_ => Err(self),
}
}
}

impl<S> fmt::Debug for HandshakeError<S>
Expand Down