diff --git a/src/env.rs b/src/env.rs index 564fdd69..b1e65f68 100644 --- a/src/env.rs +++ b/src/env.rs @@ -1,4 +1,5 @@ use std::env; +use std::ffi::OsString; #[cfg(feature = "convert-case")] use convert_case::{Case, Casing}; @@ -7,6 +8,7 @@ use crate::error::Result; use crate::map::Map; use crate::source::Source; use crate::value::{Value, ValueKind}; +use crate::ConfigError; /// An environment source collects a dictionary of environment variables values into a hierarchical /// config Value type. We have to be aware how the config tree is created from the environment @@ -243,10 +245,16 @@ impl Source for Environment { .as_ref() .map(|prefix| format!("{prefix}{prefix_separator}").to_lowercase()); - let collector = |(key, value): (String, String)| { + let collector = |(key, value): (OsString, OsString)| { + let key = match key.into_string() { + Ok(key) => key, + // Key is not valid unicode, skip it + Err(_) => return Ok(()), + }; + // Treat empty environment variables as unset if self.ignore_empty && value.is_empty() { - return; + return Ok(()); } let mut key = key.to_lowercase(); @@ -260,10 +268,18 @@ impl Source for Environment { } } else { // Skip this key - return; + return Ok(()); } } + // At this point, we don't know if the key is required or not. + // Therefore if the value is not a valid unicode string, we error out. + let value = value.into_string().map_err(|os_string| { + ConfigError::Message(format!( + "env variable {key:?} contains non-Unicode data: {os_string:?}" + )) + })?; + // If separator is given replace with `.` if !separator.is_empty() { key = key.replace(separator, "."); @@ -308,12 +324,18 @@ impl Source for Environment { }; m.insert(key, Value::new(Some(&uri), value)); + + Ok(()) }; match &self.source { - Some(source) => source.clone().into_iter().for_each(collector), - None => env::vars().for_each(collector), - } + Some(source) => source + .clone() + .into_iter() + .map(|(key, value)| (key.into(), value.into())) + .try_for_each(collector), + None => env::vars_os().try_for_each(collector), + }?; Ok(m) } diff --git a/tests/testsuite/env.rs b/tests/testsuite/env.rs index 00a59e8e..f8ef2b23 100644 --- a/tests/testsuite/env.rs +++ b/tests/testsuite/env.rs @@ -721,3 +721,86 @@ fn test_parse_uint_default() { let config: TestUint = config.try_deserialize().unwrap(); assert_eq!(config.int_val, 42); } + +#[cfg(any(unix, windows))] +#[cfg(test)] +mod unicode_tests { + use std::ffi::OsString; + + use super::*; + + fn make_invalid_unicode_os_string() -> OsString { + let string = { + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + + OsString::from_vec(vec![0xff]) + } + #[cfg(windows)] + { + use std::os::windows::ffi::OsStringExt; + + OsString::from_wide(&[0xd800]) // unpaired high surrogate + } + }; + + assert!(string.to_str().is_none()); + + string + } + + #[test] + fn test_invalid_unicode_key_ignored() { + temp_env::with_vars( + vec![ + (make_invalid_unicode_os_string(), Some("abc")), + ("A_B_C".into(), Some("abc")), + ], + || { + let vars = Environment::default().collect().unwrap(); + + assert!(vars.contains_key("a_b_c")); + }, + ); + } + + #[test] + fn test_invalid_unicode_value_filtered() { + temp_env::with_vars( + vec![ + ("invalid_value1", Some(make_invalid_unicode_os_string())), + ("valid_value2", Some("valid".into())), + ], + || { + let vars = Environment::with_prefix("valid") + .keep_prefix(true) + .collect() + .unwrap(); + + assert!(!vars.contains_key("invalid_value1")); + assert!(vars.contains_key("valid_value2")); + }, + ); + } + + #[test] + fn test_invalid_unicode_value_not_filtered() { + temp_env::with_vars( + vec![("invalid_value1", Some(make_invalid_unicode_os_string()))], + || { + let result = Environment::default().collect(); + + #[cfg(unix)] + let expected = + str![[r#"env variable "invalid_value1" contains non-Unicode data: "/xFF""#]]; + #[cfg(windows)] + let expected = str![[ + r#"env variable "invalid_value1" contains non-Unicode data: "/u{d800}""# + ]]; + + assert_data_eq!(result.unwrap_err().to_string(), expected); + }, + ); + } +}