Skip to content

Commit 7723550

Browse files
committed
Reduce the reliance on PATH_MAX
- 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.
1 parent 4ff44ff commit 7723550

File tree

2 files changed

+35
-16
lines changed

2 files changed

+35
-16
lines changed

src/libstd/sys/unix/fs.rs

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -376,13 +376,19 @@ impl fmt::Debug for File {
376376

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

@@ -466,18 +472,27 @@ pub fn rmdir(p: &Path) -> io::Result<()> {
466472
pub fn readlink(p: &Path) -> io::Result<PathBuf> {
467473
let c_path = try!(cstr(p));
468474
let p = c_path.as_ptr();
469-
let mut len = unsafe { libc::pathconf(p as *mut _, libc::_PC_NAME_MAX) };
470-
if len < 0 {
471-
len = 1024; // FIXME: read PATH_MAX from C ffi?
472-
}
473-
let mut buf: Vec<u8> = Vec::with_capacity(len as usize);
474-
unsafe {
475-
let n = try!(cvt({
476-
libc::readlink(p, buf.as_ptr() as *mut c_char, len as size_t)
477-
}));
478-
buf.set_len(n as usize);
475+
476+
let mut buf = Vec::with_capacity(256);
477+
478+
loop {
479+
let buf_read = try!(cvt(unsafe {
480+
libc::readlink(p, buf.as_mut_ptr() as *mut _, buf.capacity() as libc::size_t)
481+
})) as usize;
482+
483+
unsafe { buf.set_len(buf_read); }
484+
485+
if buf_read != buf.capacity() {
486+
buf.shrink_to_fit();
487+
488+
return Ok(PathBuf::from(OsString::from_vec(buf)));
489+
}
490+
491+
// Trigger the internal buffer resizing logic of `Vec` by requiring
492+
// more space than the current capacity. The length is guaranteed to be
493+
// the same as the capacity due to the if statement above.
494+
buf.reserve(1);
479495
}
480-
Ok(PathBuf::from(OsString::from_vec(buf)))
481496
}
482497

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

515530
pub fn canonicalize(p: &Path) -> io::Result<PathBuf> {
516531
let path = try!(CString::new(p.as_os_str().as_bytes()));
517-
let mut buf = vec![0u8; 16 * 1024];
532+
let buf;
518533
unsafe {
519-
let r = c::realpath(path.as_ptr(), buf.as_mut_ptr() as *mut _);
534+
let r = c::realpath(path.as_ptr(), ptr::null_mut());
520535
if r.is_null() {
521536
return Err(io::Error::last_os_error())
522537
}
538+
buf = CStr::from_ptr(r).to_bytes().to_vec();
539+
libc::free(r as *mut _);
523540
}
524-
let p = buf.iter().position(|i| *i == 0).unwrap();
525-
buf.truncate(p);
526541
Ok(PathBuf::from(OsString::from_vec(buf)))
527542
}
528543

src/rt/rust_builtin.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,11 @@ const char * rust_current_exe()
341341
char **paths;
342342
size_t sz;
343343
int i;
344-
char buf[2*PATH_MAX], exe[2*PATH_MAX];
344+
/* If `PATH_MAX` is defined on the platform, `realpath` will truncate the
345+
* resolved path up to `PATH_MAX`. While this can make the resolution fail if
346+
* the executable is placed in a deep path, the usage of a buffer whose
347+
* length depends on `PATH_MAX` is still memory safe. */
348+
char buf[2*PATH_MAX], exe[PATH_MAX];
345349

346350
if (self != NULL)
347351
return self;

0 commit comments

Comments
 (0)