mehcode / config-rs

⚙️ Layered configuration system for Rust applications (with strong support for 12-factor applications).
Apache License 2.0
2.57k stars 213 forks source link

Deserializing maps with integer keys fails silently #341

Closed emgrav closed 2 years ago

emgrav commented 2 years ago

Attempting to deserialize a map with an integer key fails, but does not produce an error. Putting quotes around the key makes it work as expected. This is using config 0.13.1.

#![allow(dead_code, unused)]
use std::collections::HashMap;

use config::Config;
use serde::Deserialize;

#[derive(Debug, Deserialize)]
struct Outer {
    inner_string: HashMap<String, Inner>,
    inner_int: HashMap<u32, Inner>,
}

#[derive(Debug, Deserialize)]
struct Inner {
    member: String,
}

fn main() {
    let config = Config::builder()
        .add_source(config::File::with_name("config.yaml"))
        .build()
        .unwrap();
    println!("{:?}", config.try_deserialize::<Outer>().unwrap());
}
# config.yaml
inner_int:
  "1":
    member: "Test Int 1"
  2:
    member: "Test Int 2"
inner_string:
  stuff:
    member: "Test String"

Output:

Outer { 
  inner_string: {
    "stuff": Inner { 
      member: "Test String" }
    }, 
  inner_int: {
    1: Inner { 
      member: "Test Int 1" 
    }
  }
}
YounessBird commented 2 years ago

This error occurs in the from_yaml_value function in the format/yaml.rs specifically in the yaml::Yaml::Hash(ref table) variant.

if let Some(k) = key.as_str() {
      m.insert(k.to_owned(), from_yaml_value(uri, value)?);
  }

Because the hash is nested it attempts to insert the keys in the Map as &str this conflicts with an integer key yaml::Yaml::Integer(k) and so a quick fix to this could be.

 match key{
       yaml::Yaml::String(k) =>{
       m.insert(k.to_owned(), from_yaml_value(uri, value)?);
       }
       yaml::Yaml::Integer(k) =>{
       m.insert(k.to_string(), from_yaml_value(uri, value)?); 
       },
       _ => unreachable!()
 }

I have tested this and it seems to solve the problem, @matthiasbeyer if you are happy with this quick fix I will be happy to send a pull request if requested.

matthiasbeyer commented 2 years ago

Yes, a patch would be nice.

Also: Does YAML allow other types as keys? For example floats? Would be the perfect opportunity to fix this as well :laughing: !

YounessBird commented 2 years ago

@matthiasbeyer I don't think so. It's evident from the yaml parser that it only supports Integer(i64), I have also looked online but I couldn't find anything to suggest that.