Repository navigation
libstd trusts environment variables too much #40320
Description
I have not reviewed all the code(I needed specifically only these 2 functions) but at least home_dir() and temp_dir() are affected by this issue.
Let's take temp_dir as an example here.
It relies on the TMPDIR variable to be defined, otherwise it falls back to /tmp(not on Android).
My machine is on Linux and TMPDIR is not defined, which means that Rust falls back to /tmp, which is OK.
The issue is that if I do: export TMPDIR - now temp_dir will reliably return PathBuf::from("").
Same goes for export HOME -> home_dir -> Some(PathBuf::from("")).
As an example: it's not unusual for a bare-bones Linux container to have HOME undefined or just set to export HOME=.
I think that all the functions that have a fallback when an environment variable is not defined, should check if the string is not empty - if it is, then the functions should use the same fallback.
Pinging @alexcrichton
Activity
nagisa commentedon Mar 7, 2017
It is not libstd trusting its environment too much. It is your environment being misconfigured, regardless of it being a common practice or not.
(Also,
TMPDIR=''is a great way to make current working directory a TMPDIR, even across cwd changes; so I’m against changing anything here)lilianmoraru commentedon Mar 7, 2017
@nagisa The fallback seems to me like it is trying to avoid exactly this situation.
I want to find the user's home dir - why would the API tell me that it found it successfully but then return me an empty string? Although, it doesn't return an empty string if it detects that the variable was not set...
I think it is okay for an API like
env_osto return an empty string, but definitely nothome_dir.The issue I have now is that I have to scroll every time through Rust's
libstdto find out if after calling a function, I cannot rely on it to return the result, even though it returnsOk(),Some().I hit this issue when I tried in my library to obtain the user's
home_dirin a Linux container and then try to create a directory - for some reason(than found that it is because of this) I could not find the directory anywhere...I think I understand from what point of view you are coming.
It seems to me that you think of it from the point of executing something in a terminal(or setting an empty string in any code).
If you set a variable to an empty string, than you should expect an empty string.
If you set in the terminal
$HOMEto an empty string, than querying it, you should expect an empty string.My point of view is:
If you set a variable to an empty string, than something like
env_osshould definitely return an empty string.But a call to
home_dirortemp_dirto me should be a wrapper over system calls, not over terminal environment variables.So, that call to
getpwuid_ronLinuxorGetUserProfileDirectoryWonWindowsis actually the correct abstraction of the these API functions.nagisa commentedon Mar 7, 2017
Okay, so this issue is valid for at least
$HOMEas the documentation forhome_dirsays:temp_dirhas a similar claim:but only for Windows.we probably should unify the behaviour docs between window and unix here.
So this is not your average “I dislike its semantics” bug, but “it does not work as documented” one and is therefore much more severe than initially seemed to me.
lilianmoraru commentedon Mar 7, 2017
@nagisa I think that what the documentation says does not make the behavior correct.
I think the documentation states only what the code does.
Yes, it currently reads the
HOMEenvironment variable, but it does not mean that this is the way to go in a native programming language.I would understand a scripting language(
Bashas an example) to use the environment variables, but not a programming language that can make OS-specific low-level API calls.nagisa commentedon Mar 7, 2017
@lilianmoraru read what the documentation says again, please.
lilianmoraru commentedon Mar 7, 2017
@nagisa The documentation implies that before returning
$HOME, it makes sure that the variable is set(which it does) and that the string is not empty(which it doesn't), and if these conditions are not met, thangetpwuid_ris called.I understand that either the docs or the code is wrong here.
But, my point is that I am not sure that the environment variable should be read at all when you have access to OS syscalls, but at the same time,
home_diris understd::env...tbu- commentedon Mar 7, 2017
bashexpands~toHOMEif it is present (even if empty) and uses the same fallback as we do if it is not present.I propose that we keep doing the same as the platform-specific tools.
tbu- commentedon Mar 9, 2017
Python stdlib is doing the same as we do on non-Windows (haven't checked the Windows code path, it's in the same file).
os.path.gethomedir, used intos.path.expanduser.[-]`libstd` trusts environment variables to much [/-][+]`libstd` trusts environment variables too much [/+]Mark-Simulacrum commentedon Dec 9, 2019
AFAICT, the behavior as it stands for temp_dir and home_dir is the one we want based on a re-read of this issue so I'm going to close, but if I've misread something please leave a comment with the specific problem I missed. :)