Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle case mismatches when looking up env vars in the Config snapshot #11824

Merged
merged 7 commits into from Mar 17, 2023
79 changes: 64 additions & 15 deletions src/cargo/util/config/mod.rs
Expand Up @@ -124,6 +124,30 @@ macro_rules! get_value_typed {
};
}

/// Generate `case_insensitive_env` and `normalized_env` from the `env`.
fn make_case_insensitive_and_normalized_env(
env: &HashMap<OsString, OsString>,
) -> (HashMap<String, String>, HashMap<String, String>) {
// See `Config.case_insensitive_env`.
// Maps from uppercased key to actual environment key.
// For example, `"PATH" => "Path"`.
let case_insensitive_env: HashMap<_, _> = env
.keys()
.filter_map(|k| k.to_str())
.map(|k| (k.to_uppercase(), k.to_owned()))
.collect();
// See `Config.normalized_env`.
// Maps from normalized (uppercased with "-" replaced by "_") key
// to actual environment key. For example, `"MY_KEY" => "my-key"`.
let normalized_env = env
.iter()
// Only keep entries where both the key and value are valid UTF-8
.filter_map(|(k, v)| Some((k.to_str()?, v.to_str()?)))
.map(|(k, _)| (k.to_uppercase().replace("-", "_"), k.to_owned()))
.collect();
(case_insensitive_env, normalized_env)
}

/// Indicates why a config value is being loaded.
#[derive(Clone, Copy, Debug)]
enum WhyLoad {
Expand Down Expand Up @@ -201,8 +225,13 @@ pub struct Config {
target_dir: Option<Filesystem>,
/// Environment variables, separated to assist testing.
env: HashMap<OsString, OsString>,
/// Environment variables, converted to uppercase to check for case mismatch
upper_case_env: HashMap<String, String>,
/// Environment variables converted to uppercase to check for case mismatch
/// (relevant on Windows, where environment variables are case-insensitive).
case_insensitive_env: HashMap<String, String>,
/// Environment variables converted to uppercase and with "-" replaced by "_"
/// (the format expected by Cargo). This only contains entries where the key and variable are
/// both valid UTF-8.
normalized_env: HashMap<String, String>,
/// Tracks which sources have been updated to avoid multiple updates.
updated_sources: LazyCell<RefCell<HashSet<SourceId>>>,
/// Cache of credentials from configuration or credential providers.
Expand Down Expand Up @@ -261,12 +290,7 @@ impl Config {
});

let env: HashMap<_, _> = env::vars_os().collect();

let upper_case_env = env
.iter()
.filter_map(|(k, _)| k.to_str()) // Only keep valid UTF-8
.map(|k| (k.to_uppercase().replace("-", "_"), k.to_owned()))
.collect();
let (case_insensitive_env, normalized_env) = make_case_insensitive_and_normalized_env(&env);

let cache_key: &OsStr = "CARGO_CACHE_RUSTC_INFO".as_ref();
let cache_rustc_info = match env.get(cache_key) {
Expand Down Expand Up @@ -303,7 +327,8 @@ impl Config {
creation_time: Instant::now(),
target_dir: None,
env,
upper_case_env,
case_insensitive_env,
normalized_env,
updated_sources: LazyCell::new(),
credential_cache: LazyCell::new(),
package_cache_lock: RefCell::new(None),
Expand Down Expand Up @@ -730,6 +755,10 @@ impl Config {
/// Helper primarily for testing.
pub fn set_env(&mut self, env: HashMap<String, String>) {
self.env = env.into_iter().map(|(k, v)| (k.into(), v.into())).collect();
let (case_insensitive_env, normalized_env) =
make_case_insensitive_and_normalized_env(&self.env);
self.case_insensitive_env = case_insensitive_env;
self.normalized_env = normalized_env;
}

/// Returns all environment variables as an iterator, filtering out entries
Expand Down Expand Up @@ -772,10 +801,10 @@ impl Config {
/// This can be used similarly to `std::env::var`.
pub fn get_env(&self, key: impl AsRef<OsStr>) -> CargoResult<String> {
let key = key.as_ref();
let s = match self.env.get(key) {
Some(s) => s,
None => bail!("{key:?} could not be found in the environment snapshot",),
};
let s = self
.get_env_os(key)
.ok_or_else(|| anyhow!("{key:?} could not be found in the environment snapshot"))?;

match s.to_str() {
Some(s) => Ok(s.to_owned()),
None => bail!("environment variable value is not valid unicode: {s:?}"),
Expand All @@ -786,7 +815,27 @@ impl Config {
///
/// This can be used similarly to `std::env::var_os`.
pub fn get_env_os(&self, key: impl AsRef<OsStr>) -> Option<OsString> {
self.env.get(key.as_ref()).cloned()
match self.env.get(key.as_ref()) {
Some(s) => Some(s.clone()),
None => {
if cfg!(windows) {
self.get_env_case_insensitive(key).cloned()
} else {
None
}
}
}
}

/// Wrapper for `self.env.get` when `key` should be case-insensitive.
/// This is relevant on Windows, where environment variables are case-insensitive.
/// Note that this only works on keys that are valid UTF-8.
fn get_env_case_insensitive(&self, key: impl AsRef<OsStr>) -> Option<&OsString> {
let upper_case_key = key.as_ref().to_str()?.to_uppercase();
// `self.case_insensitive_env` holds pairs like `("PATH", "Path")`
// or `("MY-VAR", "my-var")`.
let env_key: &OsStr = self.case_insensitive_env.get(&upper_case_key)?.as_ref();
self.env.get(env_key)
}

/// Get the value of environment variable `key`.
Expand Down Expand Up @@ -821,7 +870,7 @@ impl Config {
}

fn check_environment_key_case_mismatch(&self, key: &ConfigKey) {
if let Some(env_key) = self.upper_case_env.get(key.as_env_key()) {
if let Some(env_key) = self.normalized_env.get(key.as_env_key()) {
let _ = self.shell().warn(format!(
"Environment variables are expected to use uppercase letters and underscores, \
the variable `{}` will be ignored and have no effect",
Expand Down
26 changes: 26 additions & 0 deletions tests/testsuite/cargo_command.rs
Expand Up @@ -104,6 +104,32 @@ fn list_command_looks_at_path() {
);
}

#[cfg(windows)]
#[cargo_test]
fn list_command_looks_at_path_case_mismatch() {
let proj = project()
.executable(Path::new("path-test").join("cargo-1"), "")
.build();

let mut path = path();
path.push(proj.root().join("path-test"));
let path = env::join_paths(path.iter()).unwrap();

// See issue #11814: Environment variable names are case-insensitive on Windows.
// We need to check that having "Path" instead of "PATH" is okay.
let output = cargo_process("-v --list")
.env("Path", &path)
.env_remove("PATH")
.exec_with_output()
.unwrap();
let output = str::from_utf8(&output.stdout).unwrap();
assert!(
output.contains("\n 1 "),
"missing 1: {}",
output
);
}

#[cargo_test]
fn list_command_handles_known_external_commands() {
let p = project()
Expand Down
25 changes: 25 additions & 0 deletions tests/testsuite/config.rs
Expand Up @@ -211,6 +211,31 @@ f1 = 123
assert_eq!(s, S { f1: Some(456) });
}

#[cfg(windows)]
#[cargo_test]
fn environment_variable_casing() {
// Issue #11814: Environment variable names are case-insensitive on Windows.
let config = ConfigBuilder::new()
.env("Path", "abc")
.env("Two-Words", "abc")
.env("two_words", "def")
.build();

let var = config.get_env("PATH").unwrap();
assert_eq!(var, String::from("abc"));

let var = config.get_env("path").unwrap();
assert_eq!(var, String::from("abc"));

let var = config.get_env("TWO-WORDS").unwrap();
assert_eq!(var, String::from("abc"));

// Make sure that we can still distinguish between dashes and underscores
// in variable names.
let var = config.get_env("Two_Words").unwrap();
assert_eq!(var, String::from("def"));
}

#[cargo_test]
fn config_works_with_extension() {
write_config_toml(
Expand Down