Open
Description
When using yaml
as a config format, the deserialization incorrectly stops at periods in keys. I do not observe this behavior when manually deserializing the string with serde_yaml
or other deserializers (such as ruamel.yaml
or pyyaml
in Python). As a result, I am getting errors about keys not being present in the mapped value even though they definitely exist.
MCVE:
192.168.1.1:
an_arr:
- 1
- 2
- 3
another_value: 10
#[derive(Debug, serde::Deserialize)]
struct Conf {
an_arr: Vec<u32>,
another_value: u32,
}
fn main() {
let settings = config::Config::builder()
.add_source(config::File::from("config.yml"))
.build()
.unwrap();
println!(
"{:?}",
settings
.try_deserialize::<HashMap<String, Conf>>()
.unwrap()
);
}
Result:
The application panicked (crashed).
Message: called `Result::unwrap()` on an `Err` value: missing field `another_value`
Location: src/main.rs:51
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
⋮ 9 frames hidden ⋮
10: core::result::Result<T,E>::unwrap::hdf9dbc5c9c26ec5d
at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/result.rs:1113
11: ui::main::he2f965d0d6a736c9
at /home/redacted/src/main.rs:49
47 │ println!(
48 │ "{:?}",
49 > settings
50 │ .try_deserialize::<HashMap<String, Conf>>()
51 │ .unwrap()
Activity
matthiasbeyer commentedon Jan 19, 2023
Hi! Thanks for your report. Would you mind filing a patch showcasing this? I would be especially interested in a comparison of
config-rs
and the plain use of the yaml crate in use byconfig-rs
(serde_yaml
IIRC).Can all be in one testcase, in one file in
/tests
.If you cannot spare the time for this, just ping me and I'll do it myself.
Halkcyon commentedon Jan 19, 2023
I expected
serde_yaml
, too, but it appearsconfig-rs
is usingyaml-rust
@matthiasbeyerhttps://github.com/mehcode/config-rs/blob/f12b93fb949557dd41cbe870179eb9afbb4c3769/Cargo.toml#L20-L35
matthiasbeyer commentedon Jan 19, 2023
Ah yeah, then that crate of course.
Sorry, I maintain so many projects that I lose track sometimes and I didn't look it up. My bad.
Halkcyon commentedon Jan 19, 2023
Digging into this deeper, it also appears
yaml-rust
has been unmaintained since 2020, so it's unlikely to be fixed upstream. I'll submit a patch test a bit later today.matthiasbeyer commentedon Jan 19, 2023
Awesome!
So it seems that we should switch YAML implementations at some point. Maybe it is worth doing this even though there's a rewrite ongoing (although slowly, #376) where we could use
serde_yaml
from the get-go.... just thinking loud here.
adding failing test for repro of rust-cli#417 showcasing failure to p…
Halkcyon commentedon Jan 19, 2023
Alright, repro added to tests with
rust-yaml
vs.serde_yaml
@matthiasbeyerpolarathene commentedon Oct 19, 2023
For reference, the source of the problem has been identified: #418 (comment)
Switching YAML parser won't resolve this.
UPDATE: You can use keys with dots/periods if they're not defined at top-level. Although as referenced below, it's considered a bug (even though it'd seem necessary for the serde rename feature to work correctly to avoid
config-rs
internally splitting the key into nested tables).This issue is also a duplicate where the cause was explained:
Input:
Outputs: