search

borntyping / rust-simple_logger

A rust logger that prints all messages with a readable output format.
https://crates.io/crates/simple_logger
MIT License
220 stars 48 forks source link

Module levels not actually getting sorted outside of #[cfg(test)] #90

Closed nikarh closed 7 months ago

nikarh commented 7 months ago

Hi,

It seems that this commit https://github.com/borntyping/rust-simple_logger/commit/8cec833d054543759e7e359ccc294023cbd55e49 introduced a bug - only the cloned Vec is now sorted (and never used later), leaving the original self.module_levels pristine.

borntyping commented 7 months ago

Can you describe the unexpected behaviour you're seeing? I can see that the code is doing something useless, but I can't work out if sorting module_levels is ever actually important in the code.

nikarh commented 7 months ago

Honestly, I was pursuing an issue that turned out to be unrelated to this finding (I used indicatif_log_bridge which was overriding log::set_max_level to an incorrect value).

But theoretically, looking here, I would say it's possible to have something like:

    let log = simple_logger::SimpleLogger::new()
        .with_module_level("serde", log::LevelFilter::Error);
        .with_module_level("serde_json", log::LevelFilter::Trace);

And, without sorting, this would lead to all serde_json logs being treated as if they were configured to Error level instead of Trace (since to determine the logging level for target, the code finds first match in module_levels by a string prefix).

borntyping commented 7 months ago

Thanks - I ended up with a similar looking test debugging this so glad I understood correctly. It was a bit confusing to work out since there was some code that sorted the Vec in with_module_level but only when tests were running!

borntyping commented 7 months ago

Released in v4.3.3. Thanks!

nikarh commented 7 months ago

Thanks for the quick fix!