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

Revise rand_core::Error (alt #771) #800

Merged
merged 10 commits into from May 29, 2019
3 changes: 2 additions & 1 deletion rand_core/Cargo.toml
Expand Up @@ -18,10 +18,11 @@ travis-ci = { repository = "rust-random/rand" }
appveyor = { repository = "rust-random/rand" }

[features]
std = ["alloc"] # use std library; should be default but for above bug
std = ["alloc", "getrandom", "getrandom/std"] # use std library; should be default but for above bug
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this coupled to getrandom?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean we should support std without getrandom? We could try, but (1) getrandom supports (pretty-much) any std platform anyway and (2) I'm not sure Cargo allows us to imply getrandom/std only when both std and getrandom are enabled.

alloc = [] # enables Vec and Box support without std
serde1 = ["serde", "serde_derive"] # enables serde for BlockRng wrapper

[dependencies]
serde = { version = "1", optional = true }
serde_derive = { version = "^1.0.38", optional = true }
getrandom = { version = "0.1", optional = true }
213 changes: 85 additions & 128 deletions rand_core/src/error.rs
Expand Up @@ -9,169 +9,126 @@
//! Error types

use core::fmt;

#[cfg(feature="std")]
use std::error::Error as stdError;
#[cfg(feature="std")]
use std::io;

/// Error kind which can be matched over.
#[derive(PartialEq, Eq, Debug, Copy, Clone)]
pub enum ErrorKind {
/// Feature is not available; not recoverable.
///
/// This is the most permanent failure type and implies the error cannot be
/// resolved simply by retrying (e.g. the feature may not exist in this
/// build of the application or on the current platform).
Unavailable,
/// General failure; there may be a chance of recovery on retry.
///
/// This is the catch-all kind for errors from known and unknown sources
/// which do not have a more specific kind / handling method.
///
/// It is suggested to retry a couple of times or retry later when
/// handling; some error sources may be able to resolve themselves,
/// although this is not likely.
Unexpected,
/// A transient failure which likely can be resolved or worked around.
///
/// This error kind exists for a few specific cases where it is known that
/// the error likely can be resolved internally, but is reported anyway.
Transient,
/// Not ready yet: recommended to try again a little later.
///
/// This error kind implies the generator needs more time or needs some
/// other part of the application to do something else first before it is
/// ready for use; for example this may be used by external generators
/// which require time for initialization.
NotReady,
#[doc(hidden)]
__Nonexhaustive,
}

impl ErrorKind {
/// True if this kind of error may resolve itself on retry.
///
/// See also `should_wait()`.
pub fn should_retry(self) -> bool {
self != ErrorKind::Unavailable
}

/// True if we should retry but wait before retrying
///
/// This implies `should_retry()` is true.
pub fn should_wait(self) -> bool {
self == ErrorKind::NotReady
}

/// A description of this error kind
pub fn description(self) -> &'static str {
match self {
ErrorKind::Unavailable => "permanently unavailable",
ErrorKind::Unexpected => "unexpected failure",
ErrorKind::Transient => "transient failure",
ErrorKind::NotReady => "not ready yet",
ErrorKind::__Nonexhaustive => unreachable!(),
}
}
}
use core::num::NonZeroU32;


/// Error type of random number generators
///
/// This is a relatively simple error type, designed for compatibility with and
/// without the Rust `std` library. It embeds a "kind" code, a message (static
/// string only), and an optional chained cause (`std` only). The `kind` and
/// `msg` fields can be accessed directly; cause can be accessed via
/// `std::error::Error::cause` or `Error::take_cause`. Construction can only be
/// done via `Error::new` or `Error::with_cause`.
///
/// In order to be compatible with `std` and `no_std`, this type has two
/// possible implementations: with `std` a boxed `Error` trait object is stored,
/// while with `no_std` we merely store an error code.
#[derive(Debug)]
pub struct Error {
/// The error kind
pub kind: ErrorKind,
/// The error message
pub msg: &'static str,
#[cfg(feature="std")]
cause: Option<Box<stdError + Send + Sync>>,
inner: Box<dyn std::error::Error + Send + Sync + 'static>,
#[cfg(not(feature="std"))]
code: NonZeroU32,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this still have the issue that the error types are incompatible for different features, making the std feature non-additive?

Copy link
Member

Choose a reason for hiding this comment

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

No, with enabled std feature NonZeroU32 code gets wrapped into ErrorCode, so public API gets only extended with new methods and no_std part of API does not change in any way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Functionally this is the case (and the code method can no longer guarantee to have Some(NonZeroU32) result), but API changes are only additive.


impl Error {
/// Create a new instance, with specified kind and a message.
pub fn new(kind: ErrorKind, msg: &'static str) -> Self {
#[cfg(feature="std")] {
Error { kind, msg, cause: None }
}
#[cfg(not(feature="std"))] {
Error { kind, msg }
}
}

/// Create a new instance, with specified kind, message, and a
/// chained cause.
/// Construct from any type supporting `std::error::Error`
///
/// Note: `stdError` is an alias for `std::error::Error`.
/// Available only when configured with `std`.
///
/// If not targetting `std` (i.e. `no_std`), this function is replaced by
/// another with the same prototype, except that there are no bounds on the
/// type `E` (because both `Box` and `stdError` are unavailable), and the
/// `cause` is ignored.
/// See also `From<NonZeroU32>`, which is available with and without `std`.
#[cfg(feature="std")]
pub fn with_cause<E>(kind: ErrorKind, msg: &'static str, cause: E) -> Self
where E: Into<Box<stdError + Send + Sync>>
pub fn new<E>(err: E) -> Self
where E: Into<Box<dyn std::error::Error + Send + Sync + 'static>>
{
Error { kind, msg, cause: Some(cause.into()) }
Error { inner: err.into() }
}

/// Create a new instance, with specified kind, message, and a
/// chained cause.
/// Reference the inner error (`std` only)
///
/// In `no_std` mode the *cause* is ignored.
#[cfg(not(feature="std"))]
pub fn with_cause<E>(kind: ErrorKind, msg: &'static str, _cause: E) -> Self {
Error { kind, msg }
/// When configured with `std`, this is a trivial operation and never
/// panics. Without `std`, this method is simply unavailable.
#[cfg(feature="std")]
pub fn inner(&self) -> &(dyn std::error::Error + Send + Sync + 'static) {
&*self.inner
}

/// Take the cause, if any. This allows the embedded cause to be extracted.
/// This uses `Option::take`, leaving `self` with no cause.
/// Unwrap the inner error (`std` only)
///
/// When configured with `std`, this is a trivial operation and never
/// panics. Without `std`, this method is simply unavailable.
#[cfg(feature="std")]
pub fn take_cause(&mut self) -> Option<Box<stdError + Send + Sync>> {
self.cause.take()
pub fn take_inner(self) -> Box<dyn std::error::Error + Send + Sync + 'static> {
self.inner
}

/// Retrieve the error code, if any.
///
/// If this `Error` was constructed via `From<NonZeroU32>`, then this method
/// will return this `NonZeroU32` code (for `no_std` this is always the
/// case). Otherwise, this method will return `None`.
pub fn code(&self) -> Option<NonZeroU32> {
#[cfg(feature="std")] {
self.inner.downcast_ref::<ErrorCode>().map(|c| c.0)
}
#[cfg(not(feature="std"))] {
Some(self.code)
}
}
}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
#[cfg(feature="std")] {
if let Some(ref cause) = self.cause {
return write!(f, "{} ({}); cause: {}",
self.msg, self.kind.description(), cause);
}
write!(f, "{}", self.inner)
}
#[cfg(not(feature="std"))] {
write!(f, "error code {}", self.code)
}
write!(f, "{} ({})", self.msg, self.kind.description())
}
}

#[cfg(feature="std")]
impl stdError for Error {
fn description(&self) -> &str {
self.msg
impl From<NonZeroU32> for Error {
fn from(code: NonZeroU32) -> Self {
#[cfg(feature="std")] {
Error { inner: Box::new(ErrorCode(code)) }
}
#[cfg(not(feature="std"))] {
Error { code }
}
}
}

fn cause(&self) -> Option<&stdError> {
self.cause.as_ref().map(|e| e.as_ref() as &stdError)
#[cfg(feature="getrandom")]
impl From<getrandom::Error> for Error {
fn from(error: getrandom::Error) -> Self {
#[cfg(feature="std")] {
Error { inner: Box::new(error) }
}
#[cfg(not(feature="std"))] {
Error { code: error.code() }
}
}
}

#[cfg(feature="std")]
impl std::error::Error for Error {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
self.inner.source()
}
}

#[cfg(feature="std")]
impl From<Error> for io::Error {
impl From<Error> for std::io::Error {
fn from(error: Error) -> Self {
use std::io::ErrorKind::*;
match error.kind {
ErrorKind::Unavailable => io::Error::new(NotFound, error),
ErrorKind::Unexpected |
ErrorKind::Transient => io::Error::new(Other, error),
ErrorKind::NotReady => io::Error::new(WouldBlock, error),
ErrorKind::__Nonexhaustive => unreachable!(),
}
std::io::Error::new(std::io::ErrorKind::Other, error)
}
}

#[cfg(feature="std")]
#[derive(Debug, Copy, Clone)]
struct ErrorCode(NonZeroU32);

#[cfg(feature="std")]
impl fmt::Display for ErrorCode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "error code {}", self.0)
}
}

#[cfg(feature="std")]
impl std::error::Error for ErrorCode {}
86 changes: 51 additions & 35 deletions rand_core/src/lib.rs
Expand Up @@ -50,7 +50,7 @@ use core::ptr::copy_nonoverlapping;

#[cfg(all(feature="alloc", not(feature="std")))] use alloc::boxed::Box;

pub use error::{ErrorKind, Error};
pub use error::Error;


mod error;
Expand Down Expand Up @@ -213,10 +213,6 @@ pub trait CryptoRng {}
/// This trait encapsulates the low-level functionality common to all
/// pseudo-random number generators (PRNGs, or algorithmic generators).
///
/// The `FromEntropy` trait from the [`rand`] crate is automatically
/// implemented for every type implementing `SeedableRng`, providing
/// a convenient `from_entropy()` constructor.
///
/// [`rand`]: https://docs.rs/rand
pub trait SeedableRng: Sized {
/// Seed type, which is restricted to types mutably-dereferencable as `u8`
Expand Down Expand Up @@ -270,14 +266,18 @@ pub trait SeedableRng: Sized {
///
/// PRNG implementations are allowed to assume that bits in the seed are
/// well distributed. That means usually that the number of one and zero
/// bits are about equal, and values like 0, 1 and (size - 1) are unlikely.
///
/// PRNG implementations are recommended to be reproducible. A PRNG seeded
/// using this function with a fixed seed should produce the same sequence
/// of output in the future and on different architectures (with for example
/// different endianness).
///
/// It is however not required that this function yield the same state as a
/// bits are roughly equal, and values like 0, 1 and (size - 1) are unlikely.
/// Note that many non-cryptographic PRNGs will show poor quality output
/// if this is not adhered to. If you wish to seed from simple numbers, use
/// `seed_from_u64` instead.
///
/// All PRNG implementations should be reproducible unless otherwise noted:
/// given a fixed `seed`, the same sequence of output should be produced
/// on all runs, library versions and architectures (e.g. check endianness).
/// Any "value-breaking" changes to the generator should require bumping at
/// least the minor version and documentation of the change.
///
/// It is not required that this function yield the same state as a
/// reference implementation of the PRNG given equivalent seed; if necessary
/// another constructor replicating behaviour from a reference
/// implementation can be added.
Expand Down Expand Up @@ -330,33 +330,26 @@ pub trait SeedableRng: Sized {

/// Create a new PRNG seeded from another `Rng`.
///
/// This is the recommended way to initialize PRNGs with fresh entropy. The
/// `FromEntropy` trait from the [`rand`] crate provides a convenient
/// `from_entropy` method based on `from_rng`.
///
/// Usage of this method is not recommended when reproducibility is required
/// since implementing PRNGs are not required to fix Endianness and are
/// allowed to modify implementations in new releases.
/// This may be useful when needing to rapidly seed many PRNGs from a master
/// PRNG, and to allow forking of PRNGs. It may be considered deterministic.
///
/// It is important to use a good source of randomness to initialize the
/// PRNG. Cryptographic PRNG may be rendered insecure when seeded from a
/// non-cryptographic PRNG or with insufficient entropy.
/// Many non-cryptographic PRNGs will show statistical bias in their first
/// results if their seed numbers are small or if there is a simple pattern
/// between them.
/// The master PRNG should be at least as high quality as the child PRNGs.
/// When seeding non-cryptographic child PRNGs, we recommend using a
/// different algorithm for the master PRNG (ideally a CSPRNG) to avoid
/// correlations between the child PRNGs. If this is not possible (e.g.
/// forking using small non-crypto PRNGs) ensure that your PRNG has a good
/// mixing function on the output or consider use of a hash function with
/// `from_seed`.
///
/// Prefer to seed from a strong external entropy source like `OsRng` from
/// the [`rand_os`] crate or from a cryptographic PRNG; if creating a new
/// generator for cryptographic uses you *must* seed from a strong source.
///
/// Seeding a small PRNG from another small PRNG is possible, but
/// something to be careful with. An extreme example of how this can go
/// wrong is seeding an Xorshift RNG from another Xorshift RNG, which
/// will effectively clone the generator. In general seeding from a
/// generator which is hard to predict is probably okay.
/// Note that seeding `XorShiftRng` from another `XorShiftRng` provides an
/// extreme example of what can go wrong: the new PRNG will be a clone
/// of the parent.
///
/// PRNG implementations are allowed to assume that a good RNG is provided
/// for seeding, and that it is cryptographically secure when appropriate.
/// As of `rand` 0.7 / `rand_core` 0.5, implementations overriding this
/// method should ensure the implementation satisfies reproducibility
/// (in prior versions this was not required).
///
/// [`rand`]: https://docs.rs/rand
/// [`rand_os`]: https://docs.rs/rand_os
Expand All @@ -365,6 +358,29 @@ pub trait SeedableRng: Sized {
rng.try_fill_bytes(seed.as_mut())?;
Ok(Self::from_seed(seed))
}

/// Creates a new instance of the RNG seeded via [`getrandom`].
///
/// This method is the recommended way to construct non-deterministic PRNGs
/// since it is convenient and secure.
///
/// In case the overhead of using [`getrandom`] to seed *many* PRNGs is an
/// issue, one may prefer to seed from a local PRNG, e.g.
/// `from_rng(thread_rng()).unwrap()`.
///
/// # Panics
///
/// If [`getrandom`] is unable to provide secure entropy this method will panic.
///
/// [`getrandom`]: https://docs.rs/getrandom
#[cfg(feature="getrandom")]
fn from_entropy() -> Self {
let mut seed = Self::Seed::default();
if let Err(err) = getrandom::getrandom(seed.as_mut()) {
panic!("from_entropy failed: {}", err);
}
Self::from_seed(seed)
}
}

// Implement `RngCore` for references to an `RngCore`.
Expand Down