From cd72f26be1c88013945bbe7181e7d5275e078d60 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Wed, 31 Jan 2018 18:22:45 +0000 Subject: [PATCH 1/2] Log error details via Display instead of Debug --- src/error.rs | 10 +++++++++- src/lib.rs | 4 ++-- src/os.rs | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/error.rs b/src/error.rs index f1eb27ef780..0edb7106f6e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -52,7 +52,7 @@ impl ErrorKind { /// A description of this error kind pub fn description(self) -> &'static str { match self { - ErrorKind::Unavailable => "permanent failure or unavailable", + ErrorKind::Unavailable => "permanent failure", ErrorKind::Transient => "transient failure", ErrorKind::NotReady => "not ready yet", ErrorKind::Other => "uncategorised", @@ -123,6 +123,14 @@ impl Error { 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, "RNG error [{}]: {}; cause: {}", + self.kind.description(), + self.msg(), + cause); + } + } write!(f, "RNG error [{}]: {}", self.kind.description(), self.msg()) } } diff --git a/src/lib.rs b/src/lib.rs index fbec03b23f5..578534d8820 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -856,9 +856,9 @@ impl NewRng for R { trace!("Seeding new RNG"); new_os().or_else(|e1| { - warn!("OsRng failed [falling back to JitterRng]: {:?}", e1); + warn!("OsRng failed [falling back to JitterRng]: {}", e1); new_jitter().map_err(|_e2| { - warn!("JitterRng failed: {:?}", _e2); + warn!("JitterRng failed: {}", _e2); // TODO: can we somehow return both error sources? Error::with_cause( ErrorKind::Unavailable, diff --git a/src/os.rs b/src/os.rs index fb622f9d3e2..a7453a90432 100644 --- a/src/os.rs +++ b/src/os.rs @@ -74,7 +74,7 @@ impl Rng for OsRng { loop { if let Err(e) = self.try_fill_bytes(dest) { if log_err == 0 { - warn!("OsRng failed: {:?}", e); + warn!("OsRng failed: {}", e); } if e.kind().should_retry() { From 489319818bca2585ccd91c437d31ab83440bfd13 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Mon, 5 Feb 2018 14:12:03 +0000 Subject: [PATCH 2/2] Revise error logging; add EntropyRng test --- src/error.rs | 6 +++--- src/lib.rs | 21 ++++++++++++++++----- src/os.rs | 12 ++++++------ src/reseeding.rs | 3 ++- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/error.rs b/src/error.rs index 0edb7106f6e..266bb0c2946 100644 --- a/src/error.rs +++ b/src/error.rs @@ -125,13 +125,13 @@ 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, "RNG error [{}]: {}; cause: {}", - self.kind.description(), + return write!(f, "{} ({}); cause: {}", self.msg(), + self.kind.description(), cause); } } - write!(f, "RNG error [{}]: {}", self.kind.description(), self.msg()) + write!(f, "{} ({})", self.msg(), self.kind.description()) } } diff --git a/src/lib.rs b/src/lib.rs index c1fdf53f654..5f039a9c75e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -950,8 +950,8 @@ thread_local!( static THREAD_RNG_KEY: Rc>> = { const THREAD_RNG_RESEED_THRESHOLD: u64 = 32_768; let mut entropy_source = EntropyRng::new(); - let r = StdRng::from_rng(&mut entropy_source) - .expect("could not initialize thread_rng"); + let r = StdRng::from_rng(&mut entropy_source).unwrap_or_else(|err| + panic!("could not initialize thread_rng: {}", err)); let rng = ReseedingRng::new(r, THREAD_RNG_RESEED_THRESHOLD, entropy_source); @@ -1041,7 +1041,8 @@ impl Rng for EntropyRng { } fn fill_bytes(&mut self, dest: &mut [u8]) { - self.try_fill_bytes(dest).unwrap(); + self.try_fill_bytes(dest).unwrap_or_else(|err| + panic!("all entropy sources failed; first error: {}", err)) } fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { @@ -1065,6 +1066,7 @@ impl Rng for EntropyRng { let os_rng_result = try_os_new(dest); match os_rng_result { Ok(os_rng) => { + debug!("EntropyRng: using OsRng"); switch_rng = Some(EntropySource::Os(os_rng)); } Err(os_rng_error) => { @@ -1072,6 +1074,7 @@ impl Rng for EntropyRng { os_rng_error); match try_jitter_new(dest) { Ok(jitter_rng) => { + debug!("EntropyRng: using JitterRng"); switch_rng = Some(EntropySource::Jitter(jitter_rng)); } Err(_jitter_error) => { @@ -1090,6 +1093,7 @@ impl Rng for EntropyRng { os_rng_error); match try_jitter_new(dest) { Ok(jitter_rng) => { + debug!("EntropyRng: using JitterRng"); switch_rng = Some(EntropySource::Jitter(jitter_rng)); } Err(_jitter_error) => { @@ -1102,7 +1106,7 @@ impl Rng for EntropyRng { } EntropySource::Jitter(ref mut rng) => { if let Ok(os_rng) = try_os_new(dest) { - info!("EntropyRng: OsRng available [switching back from JitterRng]"); + debug!("EntropyRng: using OsRng"); switch_rng = Some(EntropySource::Os(os_rng)); } else { return rng.try_fill_bytes(dest); // use JitterRng @@ -1194,7 +1198,7 @@ pub fn sample(rng: &mut R, iterable: I, amount: usize) -> Vec mod test { use impls; #[cfg(feature="std")] - use super::{random, thread_rng}; + use super::{random, thread_rng, EntropyRng}; use super::{Rng, SeedableRng, StdRng}; #[cfg(feature="alloc")] use alloc::boxed::Box; @@ -1240,6 +1244,13 @@ mod test { impls::fill_bytes_via_u64(self, dest) } } + + #[test] + #[cfg(feature="std")] + fn test_entropy() { + let mut rng = EntropyRng::new(); + rng.next_u32(); + } #[test] fn test_fill_bytes_default() { diff --git a/src/os.rs b/src/os.rs index 5a497cdd418..168b6295755 100644 --- a/src/os.rs +++ b/src/os.rs @@ -78,14 +78,14 @@ impl Rng for OsRng { loop { if let Err(e) = self.try_fill_bytes(dest) { if err_count >= RETRY_LIMIT { - error!("OsRng failed too many times; last error: {:?}", e); - panic!("OsRng failed too many times; last error: {:?}", e); + error!("OsRng failed too many times; last error: {}", e); + panic!("OsRng failed too many times; last error: {}", e); } match e.kind() { ErrorKind::Transient => { if !error_logged { - warn!("OsRng failed; retrying up to {} times. Error: {:?}", + warn!("OsRng failed; retrying up to {} times. Error: {}", TRANSIENT_RETRIES, e); error_logged = true; } @@ -95,7 +95,7 @@ impl Rng for OsRng { } ErrorKind::NotReady => { if !error_logged { - warn!("OsRng failed; waiting up to {}s and retrying. Error: {:?}", + warn!("OsRng failed; waiting up to {}s and retrying. Error: {}", MAX_RETRY_PERIOD, e); error_logged = true; } @@ -104,8 +104,8 @@ impl Rng for OsRng { continue; } _ => { - error!("OsRng failed: {:?}", e); - panic!("OsRng fatal error: {:?}", e); + error!("OsRng failed: {}", e); + panic!("OsRng fatal error: {}", e); } } } diff --git a/src/reseeding.rs b/src/reseeding.rs index aab46f44b91..cf47b3fea8d 100644 --- a/src/reseeding.rs +++ b/src/reseeding.rs @@ -46,7 +46,8 @@ impl ReseedingRng { pub fn reseed_if_necessary(&mut self) { if self.bytes_generated >= self.generation_threshold { trace!("Reseeding RNG after {} bytes", self.bytes_generated); - R::from_rng(&mut self.reseeder).map(|result| self.rng = result).unwrap(); + R::from_rng(&mut self.reseeder).map(|result| self.rng = result) + .unwrap_or_else(|err| panic!("reseeding failed: {}", err)); self.bytes_generated = 0; } }