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

Setting process env vars with interior nuls leads to unexpected env for child #30862

Closed
kamalmarhubi opened this issue Jan 12, 2016 · 4 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@kamalmarhubi
Copy link
Contributor

Noticed by @talchas when discussing #30858.

The environment variables are never checked for interior nuls. This leads to very unexpected results in the child's env. Keys with interior nuls are truncated and have no values, and values are truncated at the nul:

use std::process::Command;

fn main() {
    Command::new("/usr/bin/env").env("a\0b", "c\0d")
                                .env("ab", "c\0d")
                                .status()
                                .unwrap();
}

(playground)

Output:

a
ab=c
USER=rust
HOME=/home/rust
PATH=/usr/local/bin:/usr/bin:/bin
LOGNAME=rust
PWD=/home/rust
@kamalmarhubi
Copy link
Contributor Author

As with #30858, it's probably best to validate when spawning the child, so that the error can be returned in a Result.

@nagisa
Copy link
Member

nagisa commented Jan 12, 2016

The underlying issue in both cases is that OsString allows interior nulls, while neither POSIX-y nor Windows systems support that use-case at all.

EDIT: apparently that’s intended, so we’re just skimping on conversion to CString in some places.

cc @alexcrichton

@kamalmarhubi
Copy link
Contributor Author

Yeah, conversion to CString at spawn would be the right thing to do.

@huonw huonw added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 14, 2016
@kamalmarhubi
Copy link
Contributor Author

I have a patch in the works for this.

kamalmarhubi added a commit to kamalmarhubi/rust that referenced this issue Feb 3, 2016
This reports an error at the point of calling `Command::spawn()` or one of
its equivalents.

Fixes rust-lang#30858
Fixes rust-lang#30862
bors added a commit that referenced this issue Feb 3, 2016
…hton

This reports an error at the point of calling `Command::spawn()` or one of
its equivalents.

Fixes #30858
Fixes #30862
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants