rust-lang / log

Logging implementation for Rust
https://docs.rs/log
Apache License 2.0
2.2k stars 254 forks source link

LevelFilter deserialization case-sensitive when deserializing from a config::Config #532

Closed cdhowie closed 7 months ago

cdhowie commented 2 years ago

When deserializing from a config::Config, log::LevelFilter deserialization is case-sensitive, even though it appears it should not be.

Cargo.toml:

[package]
name = "rust-tinker"
version = "0.1.0"
edition = "2021"

[dependencies]
log = { version = "0.4", features = ["serde"] }
serde = { version = "1", features = ["derive"] }
config = "0.13"

main.rs:

use std::error::Error;

use log::LevelFilter;

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
struct Example {
    level: log::LevelFilter,
}

fn main() -> Result<(), Box<dyn Error>> {
    let config = config::Config::builder()
        .set_default("level", "debug")?
        .build()?;

    assert_eq!(
        Example { level: LevelFilter::Debug },
        config.try_deserialize()?
    );

    Ok(())
}

No output is expected.

Actual output:

Error: enum LevelFilter does not have variant constructor debug

The only casing that appears to be accepted is all uppercase -- using "DEBUG" works.

log::Level suffers from the same problem, and I'd expect the fix to be similar.

KodrAus commented 1 year ago

Hmm, did you end up getting to the bottom of this @cdhowie? There may be some invocation of deserializer calls that we're not handling in our serde impls, but as far as I can tell we should be case-insensitive and do have some test coverage for it.

cdhowie commented 1 year ago

@KodrAus After investigating several logging libraries we decided to use slog, so I haven't been back to this issue yet. At the time, I worked around it using a custom deserializer function with #[serde(deserialize_with = ...)].

I may have some spare cycles this weekend to dig into the issue further.

Thomasdezeeuw commented 7 months ago

We use eq_ignore_ascii_case in our FromStr implementation (which the Deserialize uses`), so I can't really find the issue.

Closing due to lack of activity, @cdhowie if you find the cause please reopen.