`Command::env` doesn't report an error for keys with `=` in them

[5225225]

fn main() {
    println!(
        "{:?}",
        std::process::Command::new("printenv")
            .arg("foo")
            .env("foo=bar", "baz")
            .output()
    );
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=4c8d4bb6d9fb790591d7cf07dbf516e8

This has a stdout of bar=baz, so the foo key has value bar=baz.

Which isn't what I asked for in env, but what I asked for is invalid.

We probably shouldn't panic from a bad env call, but this program shouldn't be able to spawn without erroring.

I assume .env("foo=bar", "baz").env_remove("foo=bar") shouldn't error. (Also, not sure how non-unix platforms handle this, but this validation should presumably be platform-specific).

I'm not sure if this is a bug or just weird behavior.

[mindstorm38]

It should be analogous to libc's setenv, as said in the manual (https://man7.org/linux/man-pages/man3/setenv.3.html) EINVAL is returned if = is present in var name. For me, it should return an error upon process spawn (when arguments are aggregated), I would've preferred an early error in .env but the builder pattern would be horrible, and anyway its signature is already set in stone.

I think that it should be considered a bug, the windows impl should have the same issue (not tested) because no check is made on name (like for unix), and for CreateProcessW, win32 API explicitly states that "Because the equal sign is used as a separator, it must not be used in the name of an environment variable." (https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw).

[ChrisDenton]

I think that it should be considered a bug, the windows impl should have the same issue (not tested) because no check is made on name (like for unix), and for CreateProcessW, win32 API explicitly states that "Because the equal sign is used as a separator, it must not be used in the name of an environment variable." (https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw).

It's undocumented but = is allowed as the first character of a key name. This is used by the command prompt to sneak through "hidden" environment variables.

[5225225]

I assume that acts like a dotfile then, where you have the full envar be =foo=bar, meaning a envar with key foo and value bar (unless you parse it "normally" in which case it's a blank name and a value of foo=bar)? How should we parse it then?

[ChrisDenton]

Parsing is done simply by making sure the key name is at least one character. So =C:=C:\\Users\\ChrisDenton is parsed as ("=C:", "C:\\Users\\ChrisDenton"). We already handle this in the standard library

rust/library/std/src/sys/pal/windows/os.rs

Lines 148 to 153 in 6e1f7b5

	// Windows allows environment variables to start with an equals
	// symbol (in any other position, this is the separator between
	// variable name and value). Since`s` has at least length 1 at
	// this point (because the empty string terminates the array of
	// environment variables), we can safely slice.
	let pos = match s[1..].iter().position(|&u| u == b'=' as u16).map(|p| p + 1) {

So in relation to the OP we'd just skip over the first character when disallowing = in the key name but otherwise we wouldn't make any distinction between "hidden" and not hidden variables (that's up to the shell).

[tbu-]

@rustbot claim