From 5165fdf7e3afd20ae8315f5057f01174d1d81932 Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Fri, 20 Sep 2024 19:26:56 -0700 Subject: [PATCH] Use u8 for internal verbosity level calculation This avoids integer overflow / underflow errors when calculating the verbosity level. Prior to this change, constructing a `Verbosity` with either parameter equal to `u8::MAX` would result in incorrect verbosity level calculation. This change adds tests to ensure that the verbosity level is correctly calculated for each log level. These were run before and after the change to ensure that the change is correct. The tests using 255 as either input value were failing before the change and passing after the change. --- src/lib.rs | 193 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 177 insertions(+), 16 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9959aae..81937f6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -126,30 +126,32 @@ impl Verbosity { self.log_level().is_none() } - fn verbosity(&self) -> i8 { - level_value(L::default()) - (self.quiet as i8) + (self.verbose as i8) + fn verbosity(&self) -> u8 { + let default_verbosity = level_value(L::default()); + let verbosity = default_verbosity as i16 - self.quiet as i16 + self.verbose as i16; + verbosity.clamp(0, u8::MAX as i16) as u8 } } -fn level_value(level: Option) -> i8 { +fn level_value(level: Option) -> u8 { match level { - None => -1, - Some(Level::Error) => 0, - Some(Level::Warn) => 1, - Some(Level::Info) => 2, - Some(Level::Debug) => 3, - Some(Level::Trace) => 4, + None => 0, + Some(Level::Error) => 1, + Some(Level::Warn) => 2, + Some(Level::Info) => 3, + Some(Level::Debug) => 4, + Some(Level::Trace) => 5, } } -fn level_enum(verbosity: i8) -> Option { +fn level_enum(verbosity: u8) -> Option { match verbosity { - i8::MIN..=-1 => None, - 0 => Some(Level::Error), - 1 => Some(Level::Warn), - 2 => Some(Level::Info), - 3 => Some(Level::Debug), - 4..=i8::MAX => Some(Level::Trace), + 0 => None, + 1 => Some(Level::Error), + 2 => Some(Level::Warn), + 3 => Some(Level::Info), + 4 => Some(Level::Debug), + 5..=u8::MAX => Some(Level::Trace), } } @@ -235,4 +237,163 @@ mod test { use clap::CommandFactory; Cli::command().debug_assert(); } + + #[test] + fn verbosity_error_level() { + let v: Verbosity = Verbosity::new(0, 0); + assert_eq!(v.log_level(), Some(Level::Error)); + assert_eq!(v.log_level_filter(), LevelFilter::Error); + + let v: Verbosity = Verbosity::new(1, 0); + assert_eq!(v.log_level(), Some(Level::Warn)); + assert_eq!(v.log_level_filter(), LevelFilter::Warn); + + let v: Verbosity = Verbosity::new(2, 0); + assert_eq!(v.log_level(), Some(Level::Info)); + assert_eq!(v.log_level_filter(), LevelFilter::Info); + + let v: Verbosity = Verbosity::new(3, 0); + assert_eq!(v.log_level(), Some(Level::Debug)); + assert_eq!(v.log_level_filter(), LevelFilter::Debug); + + let v: Verbosity = Verbosity::new(4, 0); + assert_eq!(v.log_level(), Some(Level::Trace)); + assert_eq!(v.log_level_filter(), LevelFilter::Trace); + + // overflows to trace + let v: Verbosity = Verbosity::new(5, 0); + assert_eq!(v.log_level(), Some(Level::Trace)); + assert_eq!(v.log_level_filter(), LevelFilter::Trace); + + // max verbosity is trace + let v: Verbosity = Verbosity::new(255, 0); + assert_eq!(v.log_level(), Some(Level::Trace)); + assert_eq!(v.log_level_filter(), LevelFilter::Trace); + + let v: Verbosity = Verbosity::new(0, 1); + assert_eq!(v.log_level(), None); + assert_eq!(v.log_level_filter(), LevelFilter::Off); + + // underflows to off + let v: Verbosity = Verbosity::new(0, 2); + assert_eq!(v.log_level(), None); + assert_eq!(v.log_level_filter(), LevelFilter::Off); + + // max quiet is off + let v: Verbosity = Verbosity::new(0, 255); + assert_eq!(v.log_level(), None); + assert_eq!(v.log_level_filter(), LevelFilter::Off); + + // This can only happen when vebosity is manually constructed due to the args being marked + // as conflicting + let v: Verbosity = Verbosity::new(255, 255); + assert_eq!(v.log_level(), Some(Level::Error)); + assert_eq!(v.log_level_filter(), LevelFilter::Error); + } + + #[test] + fn verbosity_warn_level() { + let v: Verbosity = Verbosity::new(0, 0); + assert_eq!(v.log_level(), Some(Level::Warn)); + assert_eq!(v.log_level_filter(), LevelFilter::Warn); + + let v: Verbosity = Verbosity::new(1, 0); + assert_eq!(v.log_level(), Some(Level::Info)); + assert_eq!(v.log_level_filter(), LevelFilter::Info); + + let v: Verbosity = Verbosity::new(2, 0); + assert_eq!(v.log_level(), Some(Level::Debug)); + assert_eq!(v.log_level_filter(), LevelFilter::Debug); + + let v: Verbosity = Verbosity::new(3, 0); + assert_eq!(v.log_level(), Some(Level::Trace)); + assert_eq!(v.log_level_filter(), LevelFilter::Trace); + + // overflows to trace + let v: Verbosity = Verbosity::new(4, 0); + assert_eq!(v.log_level(), Some(Level::Trace)); + assert_eq!(v.log_level_filter(), LevelFilter::Trace); + + // max verbosity is trace + let v: Verbosity = Verbosity::new(255, 0); + assert_eq!(v.log_level(), Some(Level::Trace)); + assert_eq!(v.log_level_filter(), LevelFilter::Trace); + + let v: Verbosity = Verbosity::new(0, 1); + assert_eq!(v.log_level(), Some(Level::Error)); + assert_eq!(v.log_level_filter(), LevelFilter::Error); + + let v: Verbosity = Verbosity::new(0, 2); + assert_eq!(v.log_level(), None); + assert_eq!(v.log_level_filter(), LevelFilter::Off); + + // underflows to off + let v: Verbosity = Verbosity::new(0, 3); + assert_eq!(v.log_level(), None); + assert_eq!(v.log_level_filter(), LevelFilter::Off); + + // max quiet is off + let v: Verbosity = Verbosity::new(0, 255); + assert_eq!(v.log_level(), None); + assert_eq!(v.log_level_filter(), LevelFilter::Off); + + // This can only happen when vebosity is manually constructed due to the args being marked + // as conflicting + let v: Verbosity = Verbosity::new(255, 255); + assert_eq!(v.log_level(), Some(Level::Warn)); + assert_eq!(v.log_level_filter(), LevelFilter::Warn); + } + + #[test] + fn verbosity_info_level() { + let v: Verbosity = Verbosity::new(0, 0); + assert_eq!(v.log_level(), Some(Level::Info)); + assert_eq!(v.log_level_filter(), LevelFilter::Info); + + let v: Verbosity = Verbosity::new(1, 0); + assert_eq!(v.log_level(), Some(Level::Debug)); + assert_eq!(v.log_level_filter(), LevelFilter::Debug); + + let v: Verbosity = Verbosity::new(2, 0); + assert_eq!(v.log_level(), Some(Level::Trace)); + assert_eq!(v.log_level_filter(), LevelFilter::Trace); + + // overflows to trace + let v: Verbosity = Verbosity::new(3, 0); + assert_eq!(v.log_level(), Some(Level::Trace)); + assert_eq!(v.log_level_filter(), LevelFilter::Trace); + + // max verbosity is trace + let v: Verbosity = Verbosity::new(255, 0); + assert_eq!(v.log_level(), Some(Level::Trace)); + assert_eq!(v.log_level_filter(), LevelFilter::Trace); + + let v: Verbosity = Verbosity::new(0, 1); + assert_eq!(v.log_level(), Some(Level::Warn)); + assert_eq!(v.log_level_filter(), LevelFilter::Warn); + + let v: Verbosity = Verbosity::new(0, 2); + assert_eq!(v.log_level(), Some(Level::Error)); + assert_eq!(v.log_level_filter(), LevelFilter::Error); + + let v: Verbosity = Verbosity::new(0, 3); + assert_eq!(v.log_level(), None); + assert_eq!(v.log_level_filter(), LevelFilter::Off); + + // underflows to off + let v: Verbosity = Verbosity::new(0, 4); + assert_eq!(v.log_level(), None); + assert_eq!(v.log_level_filter(), LevelFilter::Off); + + // max quiet is off + let v: Verbosity = Verbosity::new(0, 255); + assert_eq!(v.log_level(), None); + assert_eq!(v.log_level_filter(), LevelFilter::Off); + + // This can only happen when vebosity is manually constructed due to the args being marked + // as conflicting + let v: Verbosity = Verbosity::new(255, 255); + assert_eq!(v.log_level(), Some(Level::Info)); + assert_eq!(v.log_level_filter(), LevelFilter::Info); + } }