itsscb/cargo
1
0
Fork 0
mirror of https://github.com/rust-lang/cargo.git synced 2025-10-01 11:30:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix config env vars that are prefix of another with underscore.

Browse Source
This commit is contained in:
Eric Huss 2019-12-23 18:13:50 -08:00
parent 2f529296cb
commit 829ddf0dc8
5 changed files with 179 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

111
src/cargo/util/config/de.rs
View File

@ -4,15 +4,22 @@ use crate::util::config::value;
use crate::util::config::{Config, ConfigError, ConfigKey};
use crate::util::config::{ConfigValue as CV, Definition, Value};
use serde::{de, de::IntoDeserializer};
use std::collections::HashSet;
use std::vec;
/// Serde deserializer used to convert config values to a target type using
/// `Config::get`.
#[derive(Clone)]
pub(crate) struct Deserializer<'config> {
pub(crate) config: &'config Config,
pub(crate) key: ConfigKey,
pub(super) struct Deserializer<'config> {
pub(super) config: &'config Config,
/// The current key being deserialized.
pub(super) key: ConfigKey,
/// Whether or not this key part is allowed to be an inner table. For
/// example, `profile.dev.build-override` needs to check if
/// CARGO_PROFILE_DEV_BUILD_OVERRIDE_ prefixes exist. But
/// CARGO_BUILD_TARGET should not check for prefixes because it would
/// collide with CARGO_BUILD_TARGET_DIR. See `ConfigMapAccess` for
/// details.
pub(super) env_prefix_ok: bool,
}
macro_rules! deserialize_method {
@ -109,7 +116,7 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
where
V: de::Visitor<'de>,
{
if self.config.has_key(&self.key) {
if self.config.has_key(&self.key, self.env_prefix_ok) {
visitor.visit_some(self)
} else {
// Treat missing values as `None`.
@ -179,8 +186,10 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
struct ConfigMapAccess<'config> {
de: Deserializer<'config>,
set_iter: <HashSet<KeyKind> as IntoIterator>::IntoIter,
next: Option<KeyKind>,
/// The fields that this map should deserialize.
fields: Vec<KeyKind>,
/// Current field being deserialized.
field_index: usize,
}
#[derive(Debug, PartialEq, Eq, Hash)]
@ -191,31 +200,31 @@ enum KeyKind {
impl<'config> ConfigMapAccess<'config> {
fn new_map(de: Deserializer<'config>) -> Result<ConfigMapAccess<'config>, ConfigError> {
let mut set = HashSet::new();
let mut fields = Vec::new();
if let Some(mut v) = de.config.get_table(&de.key)? {
// `v: Value<HashMap<String, CV>>`
for (key, _value) in v.val.drain() {
set.insert(KeyKind::CaseSensitive(key));
fields.push(KeyKind::CaseSensitive(key));
}
}
if de.config.cli_unstable().advanced_env {
// `CARGO_PROFILE_DEV_PACKAGE_`
let env_pattern = format!("{}_", de.key.as_env_key());
let env_prefix = format!("{}_", de.key.as_env_key());
for env_key in de.config.env.keys() {
if env_key.starts_with(&env_pattern) {
if env_key.starts_with(&env_prefix) {
// `CARGO_PROFILE_DEV_PACKAGE_bar_OPT_LEVEL = 3`
let rest = &env_key[env_pattern.len()..];
let rest = &env_key[env_prefix.len()..];
// `rest = bar_OPT_LEVEL`
let part = rest.splitn(2, '_').next().unwrap();
// `part = "bar"`
set.insert(KeyKind::CaseSensitive(part.to_string()));
fields.push(KeyKind::CaseSensitive(part.to_string()));
}
}
}
Ok(ConfigMapAccess {
de,
set_iter: set.into_iter(),
next: None,
fields,
field_index: 0,
})
}
@ -223,10 +232,10 @@ impl<'config> ConfigMapAccess<'config> {
de: Deserializer<'config>,
fields: &'static [&'static str],
) -> Result<ConfigMapAccess<'config>, ConfigError> {
let mut set = HashSet::new();
for field in fields {
set.insert(KeyKind::Normal(field.to_string()));
}
let fields: Vec<KeyKind> = fields
.iter()
.map(|field| KeyKind::Normal(field.to_string()))
.collect();
// Assume that if we're deserializing a struct it exhaustively lists all
// possible fields on this key that we're *supposed* to use, so take
@ -234,7 +243,10 @@ impl<'config> ConfigMapAccess<'config> {
// fields and warn about them.
if let Some(mut v) = de.config.get_table(&de.key)? {
for (t_key, value) in v.val.drain() {
if set.contains(&KeyKind::Normal(t_key.to_string())) {
if fields.iter().any(|k| match k {
KeyKind::Normal(s) => s == &t_key,
KeyKind::CaseSensitive(s) => s == &t_key,
}) {
continue;
}
de.config.shell().warn(format!(
@ -248,8 +260,8 @@ impl<'config> ConfigMapAccess<'config> {
Ok(ConfigMapAccess {
de,
set_iter: set.into_iter(),
next: None,
fields,
field_index: 0,
})
}
}
@ -261,30 +273,61 @@ impl<'de, 'config> de::MapAccess<'de> for ConfigMapAccess<'config> {
where
K: de::DeserializeSeed<'de>,
{
match self.set_iter.next() {
Some(key) => {
let name = match &key {
if self.field_index >= self.fields.len() {
return Ok(None);
}
let field = match &self.fields[self.field_index] {
KeyKind::Normal(s) | KeyKind::CaseSensitive(s) => s.as_str(),
};
let result = seed.deserialize(name.into_deserializer()).map(Some);
self.next = Some(key);
result
}
None => Ok(None),
}
seed.deserialize(field.into_deserializer()).map(Some)
}
fn next_value_seed<V>(&mut self, seed: V) -> Result<V::Value, Self::Error>
where
V: de::DeserializeSeed<'de>,
{
match self.next.take().expect("next field missing") {
KeyKind::Normal(key) => self.de.key.push(&key),
KeyKind::CaseSensitive(key) => self.de.key.push_sensitive(&key),
let field = &self.fields[self.field_index];
self.field_index += 1;
// Set this as the current key in the deserializer.
let field = match field {
KeyKind::Normal(field) => {
self.de.key.push(&field);
field
}
KeyKind::CaseSensitive(field) => {
self.de.key.push_sensitive(&field);
field
}
};
// Env vars that are a prefix of another with a dash/underscore cannot
// be supported by our serde implementation, so check for them here.
// Example:
// CARGO_BUILD_TARGET
// CARGO_BUILD_TARGET_DIR
// or
// CARGO_PROFILE_DEV_DEBUG
// CARGO_PROFILE_DEV_DEBUG_ASSERTIONS
// The `deserialize_option` method does not know the type of the field.
// If the type is an Option<struct> (like
// `profile.dev.build-override`), then it needs to check for env vars
// starting with CARGO_FOO_BAR_. This is a problem for keys like
// CARGO_BUILD_TARGET because checking for a prefix would incorrectly
// match CARGO_BUILD_TARGET_DIR. `deserialize_option` would have no
// choice but to call `visit_some()` which would then fail if
// CARGO_BUILD_TARGET isn't set. So we check for these prefixes and
// disallow them here.
let env_prefix = format!("{}_", field).replace('-', "_");
let env_prefix_ok = !self.fields.iter().any(|field| {
let field = match field {
KeyKind::Normal(s) | KeyKind::CaseSensitive(s) => s.as_str(),
};
field.replace('-', "_").starts_with(&env_prefix)
});
let result = seed.deserialize(Deserializer {
config: self.de.config,
key: self.de.key.clone(),
env_prefix_ok,
});
self.de.key.pop();
result

18
src/cargo/util/config/mod.rs
View File

@ -35,6 +35,12 @@
//! tables (because Cargo must fetch all of them), so those do not support
//! environment variables.
//!
//! Try to avoid keys that are a prefix of another with a dash/underscore. For
//! example `build.target` and `build.target-dir`. This is OK if these are not
//! structs/maps, but if it is a struct or map, then it will not be able to
//! read the environment variable due to ambiguity. (See `ConfigMapAccess` for
//! more details.)
//!
//! ## Internal API
//!
//! Internally config values are stored with the `ConfigValue` type after they
@ -520,14 +526,17 @@ impl Config {
}
}
fn has_key(&self, key: &ConfigKey) -> bool {
if self.env.get(key.as_env_key()).is_some() {
fn has_key(&self, key: &ConfigKey, env_prefix_ok: bool) -> bool {
if self.env.contains_key(key.as_env_key()) {
return true;
}
let env_pattern = format!("{}_", key.as_env_key());
if self.env.keys().any(|k| k.starts_with(&env_pattern)) {
// See ConfigMapAccess for a description of this.
if env_prefix_ok {
let env_prefix = format!("{}_", key.as_env_key());
if self.env.keys().any(|k| k.starts_with(&env_prefix)) {
return true;
}
}
if let Ok(o_cv) = self.get_cv(key) {
if o_cv.is_some() {
return true;
@ -1101,6 +1110,7 @@ impl Config {
let d = Deserializer {
config: self,
key: ConfigKey::from_str(key),
env_prefix_ok: true,
};
T::deserialize(d).map_err(|e| e.into())
}

4
src/cargo/util/config/value.rs
View File

@ -51,10 +51,14 @@ pub(crate) const DEFINITION_FIELD: &str = "$__cargo_private_definition";
pub(crate) const NAME: &str = "$__cargo_private_Value";
pub(crate) static FIELDS: [&str; 2] = [VALUE_FIELD, DEFINITION_FIELD];
/// Location where a config value is defined.
#[derive(Clone, Debug, Eq)]
pub enum Definition {
/// Defined in a `.cargo/config`, includes the path to the file.
Path(PathBuf),
/// Defined in an environment variable, includes the environment key.
Environment(String),
/// Passed in on the command line.
Cli,
}

5
tests/testsuite/build.rs
View File

@ -2806,6 +2806,11 @@ fn custom_target_dir_env() {
assert!(p.root().join("foo/target/debug").join(&exe_name).is_file());
assert!(p.root().join("target/debug").join(&exe_name).is_file());
p.cargo("build")
.env("CARGO_BUILD_TARGET_DIR", "foo2/target")
.run();
assert!(p.root().join("foo2/target/debug").join(&exe_name).is_file());
fs::create_dir(p.root().join(".cargo")).unwrap();
File::create(p.root().join(".cargo/config"))
.unwrap()

76
tests/testsuite/config.rs
View File

@ -424,6 +424,32 @@ lto = false
);
}
#[cargo_test]
fn profile_env_var_prefix() {
// Check for a bug with collision on DEBUG vs DEBUG_ASSERTIONS.
let config = ConfigBuilder::new()
.env("CARGO_PROFILE_DEV_DEBUG_ASSERTIONS", "false")
.build();
let p: toml::TomlProfile = config.get("profile.dev").unwrap();
assert_eq!(p.debug_assertions, Some(false));
assert_eq!(p.debug, None);
let config = ConfigBuilder::new()
.env("CARGO_PROFILE_DEV_DEBUG", "1")
.build();
let p: toml::TomlProfile = config.get("profile.dev").unwrap();
assert_eq!(p.debug_assertions, None);
assert_eq!(p.debug, Some(toml::U32OrBool::U32(1)));
let config = ConfigBuilder::new()
.env("CARGO_PROFILE_DEV_DEBUG_ASSERTIONS", "false")
.env("CARGO_PROFILE_DEV_DEBUG", "1")
.build();
let p: toml::TomlProfile = config.get("profile.dev").unwrap();
assert_eq!(p.debug_assertions, Some(false));
assert_eq!(p.debug, Some(toml::U32OrBool::U32(1)));
}
#[cargo_test]
fn config_deserialize_any() {
// Some tests to exercise deserialize_any for deserializers that need to
@ -1103,3 +1129,53 @@ Caused by:
expected string but found integer in list",
);
}
#[cargo_test]
fn struct_with_opt_inner_struct() {
// Struct with a key that is Option of another struct.
// Check that can be defined with environment variable.
#[derive(Deserialize)]
struct Inner {
value: Option<i32>,
}
#[derive(Deserialize)]
struct Foo {
inner: Option<Inner>,
}
let config = ConfigBuilder::new()
.env("CARGO_FOO_INNER_VALUE", "12")
.build();
let f: Foo = config.get("foo").unwrap();
assert_eq!(f.inner.unwrap().value.unwrap(), 12);
}
#[cargo_test]
fn overlapping_env_config() {
// Issue where one key is a prefix of another.
#[derive(Deserialize)]
#[serde(rename_all = "kebab-case")]
struct Ambig {
debug: Option<u32>,
debug_assertions: Option<bool>,
}
let config = ConfigBuilder::new()
.env("CARGO_AMBIG_DEBUG_ASSERTIONS", "true")
.build();
let s: Ambig = config.get("ambig").unwrap();
assert_eq!(s.debug_assertions, Some(true));
assert_eq!(s.debug, None);
let config = ConfigBuilder::new().env("CARGO_AMBIG_DEBUG", "0").build();
let s: Ambig = config.get("ambig").unwrap();
assert_eq!(s.debug_assertions, None);
assert_eq!(s.debug, Some(0));
let config = ConfigBuilder::new()
.env("CARGO_AMBIG_DEBUG", "1")
.env("CARGO_AMBIG_DEBUG_ASSERTIONS", "true")
.build();
let s: Ambig = config.get("ambig").unwrap();
assert_eq!(s.debug_assertions, Some(true));
assert_eq!(s.debug, Some(1));
}