Skip to content

Commit

Permalink
Reduce the reliance on PATH_MAX
Browse files Browse the repository at this point in the history
- Rewrite `std::sys::fs::readlink` not to rely on `PATH_MAX`

It currently has the following problems:

1. It uses `_PC_NAME_MAX` to query the maximum length of a file path in
the underlying system. However, the meaning of the constant is the
maximum length of *a path component*, not a full path. The correct
constant should be `_PC_PATH_MAX`.

2. `pathconf` *may* fail if the referred file does not exist. This can
be problematic if the file which the symbolic link points to does not
exist, but the link itself does exist. In this case, the current
implementation resorts to the hard-coded value of `1024`, which is not
ideal.

3. There may exist a platform where there is no limit on file path
lengths in general. That's the reaon why GNU Hurd doesn't define
`PATH_MAX` at all, in addition to having `pathconf` always returning
`-1`. In these platforms, the content of the symbolic link can be
silently truncated if the length exceeds the hard-coded limit mentioned
above.

4. The value obtained by `pathconf` may be outdated at the point of
actually calling `readlink`. This is inherently racy.

This commit introduces a loop that gradually increases the length of the
buffer passed to `readlink`, eliminating the need of `pathconf`.

- Remove the arbitrary memory limit of `std::sys::fs::realpath`

As per POSIX 2013, `realpath` will return a malloc'ed buffer if the
second argument is a null pointer.[1]

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html

- Comment on functions that are still using `PATH_MAX`

There are some functions that only work in terms of `PATH_MAX`, such as
`F_GETPATH` in OS X. Comments on them for posterity.
  • Loading branch information
barosl committed Aug 27, 2015
1 parent 4ff44ff commit 7723550
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 16 deletions.
45 changes: 30 additions & 15 deletions src/libstd/sys/unix/fs.rs
Expand Up @@ -376,13 +376,19 @@ impl fmt::Debug for File {

#[cfg(target_os = "macos")]
fn get_path(fd: c_int) -> Option<PathBuf> {
// FIXME: The use of PATH_MAX is generally not encouraged, but it
// is inevitable in this case because OS X defines `fcntl` with
// `F_GETPATH` in terms of `MAXPATHLEN`, and there are no
// alternatives. If a better method is invented, it should be used
// instead.
let mut buf = vec![0;libc::PATH_MAX as usize];
let n = unsafe { libc::fcntl(fd, libc::F_GETPATH, buf.as_ptr()) };
if n == -1 {
return None;
}
let l = buf.iter().position(|&c| c == 0).unwrap();
buf.truncate(l as usize);
buf.shrink_to_fit();
Some(PathBuf::from(OsString::from_vec(buf)))
}

Expand Down Expand Up @@ -466,18 +472,27 @@ pub fn rmdir(p: &Path) -> io::Result<()> {
pub fn readlink(p: &Path) -> io::Result<PathBuf> {
let c_path = try!(cstr(p));
let p = c_path.as_ptr();
let mut len = unsafe { libc::pathconf(p as *mut _, libc::_PC_NAME_MAX) };
if len < 0 {
len = 1024; // FIXME: read PATH_MAX from C ffi?
}
let mut buf: Vec<u8> = Vec::with_capacity(len as usize);
unsafe {
let n = try!(cvt({
libc::readlink(p, buf.as_ptr() as *mut c_char, len as size_t)
}));
buf.set_len(n as usize);

let mut buf = Vec::with_capacity(256);

loop {
let buf_read = try!(cvt(unsafe {
libc::readlink(p, buf.as_mut_ptr() as *mut _, buf.capacity() as libc::size_t)
})) as usize;

unsafe { buf.set_len(buf_read); }

if buf_read != buf.capacity() {
buf.shrink_to_fit();

return Ok(PathBuf::from(OsString::from_vec(buf)));
}

// Trigger the internal buffer resizing logic of `Vec` by requiring
// more space than the current capacity. The length is guaranteed to be
// the same as the capacity due to the if statement above.
buf.reserve(1);
}
Ok(PathBuf::from(OsString::from_vec(buf)))
}

pub fn symlink(src: &Path, dst: &Path) -> io::Result<()> {
Expand Down Expand Up @@ -514,15 +529,15 @@ pub fn lstat(p: &Path) -> io::Result<FileAttr> {

pub fn canonicalize(p: &Path) -> io::Result<PathBuf> {
let path = try!(CString::new(p.as_os_str().as_bytes()));
let mut buf = vec![0u8; 16 * 1024];
let buf;
unsafe {
let r = c::realpath(path.as_ptr(), buf.as_mut_ptr() as *mut _);
let r = c::realpath(path.as_ptr(), ptr::null_mut());
if r.is_null() {
return Err(io::Error::last_os_error())
}
buf = CStr::from_ptr(r).to_bytes().to_vec();
libc::free(r as *mut _);
}
let p = buf.iter().position(|i| *i == 0).unwrap();
buf.truncate(p);
Ok(PathBuf::from(OsString::from_vec(buf)))
}

Expand Down
6 changes: 5 additions & 1 deletion src/rt/rust_builtin.c
Expand Up @@ -341,7 +341,11 @@ const char * rust_current_exe()
char **paths;
size_t sz;
int i;
char buf[2*PATH_MAX], exe[2*PATH_MAX];
/* If `PATH_MAX` is defined on the platform, `realpath` will truncate the
* resolved path up to `PATH_MAX`. While this can make the resolution fail if
* the executable is placed in a deep path, the usage of a buffer whose
* length depends on `PATH_MAX` is still memory safe. */
char buf[2*PATH_MAX], exe[PATH_MAX];

if (self != NULL)
return self;
Expand Down

0 comments on commit 7723550

Please sign in to comment.