Auto merge of #29913 - tbu-:pr_windows_path_error_on_nul, r=alexcrichton

On Windows: Previously these paths were silently truncated at these NUL
characters, now they fail with `ErrorKind::InvalidInput`.

Author: bors

src/libstd/sys/windows/fs.rs

@@ -22,7 +22,8 @@ use sync::Arc;
 use sys::handle::Handle;
 use sys::{c, cvt};
 use sys_common::FromInner;
-use vec::Vec;
+
+use super::to_u16s;
 
 pub struct File { handle: Handle }
 
@@ -226,7 +227,7 @@ impl File {
     }
 
     pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
-        let path = to_utf16(path);
+        let path = try!(to_u16s(path));
         let handle = unsafe {
             c::CreateFileW(path.as_ptr(),
                            opts.get_desired_access(),
@@ -377,10 +378,6 @@ impl fmt::Debug for File {
     }
 }
 
-pub fn to_utf16(s: &Path) -> Vec<u16> {
-    s.as_os_str().encode_wide().chain(Some(0)).collect()
-}
-
 impl FileAttr {
     pub fn size(&self) -> u64 {
         ((self.data.nFileSizeHigh as u64) << 32) | (self.data.nFileSizeLow as u64)
@@ -449,7 +446,7 @@ impl DirBuilder {
     pub fn new() -> DirBuilder { DirBuilder }
 
     pub fn mkdir(&self, p: &Path) -> io::Result<()> {
-        let p = to_utf16(p);
+        let p = try!(to_u16s(p));
         try!(cvt(unsafe {
             c::CreateDirectoryW(p.as_ptr(), ptr::null_mut())
         }));
@@ -460,7 +457,7 @@ impl DirBuilder {
 pub fn readdir(p: &Path) -> io::Result<ReadDir> {
     let root = p.to_path_buf();
     let star = p.join("*");
-    let path = to_utf16(&star);
+    let path = try!(to_u16s(&star));
 
     unsafe {
         let mut wfd = mem::zeroed();
@@ -478,22 +475,22 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {
 }
 
 pub fn unlink(p: &Path) -> io::Result<()> {
-    let p_utf16 = to_utf16(p);
-    try!(cvt(unsafe { c::DeleteFileW(p_utf16.as_ptr()) }));
+    let p_u16s = try!(to_u16s(p));
+    try!(cvt(unsafe { c::DeleteFileW(p_u16s.as_ptr()) }));
     Ok(())
 }
 
 pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
-    let old = to_utf16(old);
-    let new = to_utf16(new);
+    let old = try!(to_u16s(old));
+    let new = try!(to_u16s(new));
     try!(cvt(unsafe {
         c::MoveFileExW(old.as_ptr(), new.as_ptr(), c::MOVEFILE_REPLACE_EXISTING)
     }));
     Ok(())
 }
 
 pub fn rmdir(p: &Path) -> io::Result<()> {
-    let p = to_utf16(p);
+    let p = try!(to_u16s(p));
     try!(cvt(unsafe { c::RemoveDirectoryW(p.as_ptr()) }));
     Ok(())
 }
@@ -508,8 +505,8 @@ pub fn symlink(src: &Path, dst: &Path) -> io::Result<()> {
 }
 
 pub fn symlink_inner(src: &Path, dst: &Path, dir: bool) -> io::Result<()> {
-    let src = to_utf16(src);
-    let dst = to_utf16(dst);
+    let src = try!(to_u16s(src));
+    let dst = try!(to_u16s(dst));
     let flags = if dir { c::SYMBOLIC_LINK_FLAG_DIRECTORY } else { 0 };
     try!(cvt(unsafe {
         c::CreateSymbolicLinkW(dst.as_ptr(), src.as_ptr(), flags) as c::BOOL
@@ -518,8 +515,8 @@ pub fn symlink_inner(src: &Path, dst: &Path, dir: bool) -> io::Result<()> {
 }
 
 pub fn link(src: &Path, dst: &Path) -> io::Result<()> {
-    let src = to_utf16(src);
-    let dst = to_utf16(dst);
+    let src = try!(to_u16s(src));
+    let dst = try!(to_u16s(dst));
     try!(cvt(unsafe {
         c::CreateHardLinkW(dst.as_ptr(), src.as_ptr(), ptr::null_mut())
     }));
@@ -545,10 +542,10 @@ pub fn stat(p: &Path) -> io::Result<FileAttr> {
 }
 
 pub fn lstat(p: &Path) -> io::Result<FileAttr> {
-    let utf16 = to_utf16(p);
+    let u16s = try!(to_u16s(p));
     unsafe {
         let mut attr: FileAttr = mem::zeroed();
-        try!(cvt(c::GetFileAttributesExW(utf16.as_ptr(),
+        try!(cvt(c::GetFileAttributesExW(u16s.as_ptr(),
                                          c::GetFileExInfoStandard,
                                          &mut attr.data as *mut _ as *mut _)));
         if attr.is_reparse_point() {
@@ -562,7 +559,7 @@ pub fn lstat(p: &Path) -> io::Result<FileAttr> {
 }
 
 pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> {
-    let p = to_utf16(p);
+    let p = try!(to_u16s(p));
     unsafe {
         try!(cvt(c::SetFileAttributesW(p.as_ptr(), perm.attrs)));
         Ok(())
@@ -602,8 +599,8 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
         *(lpData as *mut i64) = TotalBytesTransferred;
         c::PROGRESS_CONTINUE
     }
-    let pfrom = to_utf16(from);
-    let pto = to_utf16(to);
+    let pfrom = try!(to_u16s(from));
+    let pto = try!(to_u16s(to));
     let mut size = 0i64;
     try!(cvt(unsafe {
         c::CopyFileExW(pfrom.as_ptr(), pto.as_ptr(), Some(callback),
@@ -617,6 +614,7 @@ fn directory_junctions_are_directories() {
     use ffi::OsStr;
     use env;
     use rand::{self, StdRng, Rng};
+    use vec::Vec;
 
     macro_rules! t {
         ($e:expr) => (match $e {

src/libstd/sys/windows/mod.rs

@@ -72,10 +72,17 @@ pub fn decode_error_kind(errno: i32) -> ErrorKind {
     }
 }
 
-fn to_utf16_os(s: &OsStr) -> Vec<u16> {
-    let mut v: Vec<_> = s.encode_wide().collect();
-    v.push(0);
-    v
+pub fn to_u16s<S: AsRef<OsStr>>(s: S) -> io::Result<Vec<u16>> {
+    fn inner(s: &OsStr) -> io::Result<Vec<u16>> {
+        let mut maybe_result: Vec<u16> = s.encode_wide().collect();
+        if maybe_result.iter().any(|&u| u == 0) {
+            return Err(io::Error::new(io::ErrorKind::InvalidInput,
+                                      "strings passed to WinAPI cannot contain NULs"));
+        }
+        maybe_result.push(0);
+        Ok(maybe_result)
+    }
+    inner(s.as_ref())
 }
 
 // Many Windows APIs follow a pattern of where we hand a buffer and then they

src/libstd/sys/windows/os.rs

@@ -28,6 +28,8 @@ use slice;
 use sys::{c, cvt};
 use sys::handle::Handle;
 
+use super::to_u16s;
+
 pub fn errno() -> i32 {
     unsafe { c::GetLastError() as i32 }
 }
@@ -237,7 +239,7 @@ pub fn chdir(p: &path::Path) -> io::Result<()> {
 }
 
 pub fn getenv(k: &OsStr) -> io::Result<Option<OsString>> {
-    let k = super::to_utf16_os(k);
+    let k = try!(to_u16s(k));
     let res = super::fill_utf16_buf(|buf, sz| unsafe {
         c::GetEnvironmentVariableW(k.as_ptr(), buf, sz)
     }, |buf| {
@@ -256,16 +258,16 @@ pub fn getenv(k: &OsStr) -> io::Result<Option<OsString>> {
 }
 
 pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
-    let k = super::to_utf16_os(k);
-    let v = super::to_utf16_os(v);
+    let k = try!(to_u16s(k));
+    let v = try!(to_u16s(v));
 
     cvt(unsafe {
         c::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr())
     }).map(|_| ())
 }
 
 pub fn unsetenv(n: &OsStr) -> io::Result<()> {
-    let v = super::to_utf16_os(n);
+    let v = try!(to_u16s(n));
     cvt(unsafe {
         c::SetEnvironmentVariableW(v.as_ptr(), ptr::null())
     }).map(|_| ())

src/test/run-pass/paths-containing-nul.rs

@@ -0,0 +1,48 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::fs;
+use std::io;
+
+fn assert_invalid_input<T>(on: &str, result: io::Result<T>) {
+    fn inner(on: &str, result: io::Result<()>) {
+        match result {
+            Ok(()) => panic!("{} didn't return an error on a path with NUL", on),
+            Err(e) => assert!(e.kind() == io::ErrorKind::InvalidInput,
+                              "{} returned a strange {:?} on a path with NUL", on, e.kind()),
+        }
+    }
+    inner(on, result.map(|_| ()))
+}
+
+fn main() {
+    assert_invalid_input("File::open", fs::File::open("\0"));
+    assert_invalid_input("File::create", fs::File::create("\0"));
+    assert_invalid_input("remove_file", fs::remove_file("\0"));
+    assert_invalid_input("metadata", fs::metadata("\0"));
+    assert_invalid_input("symlink_metadata", fs::symlink_metadata("\0"));
+    assert_invalid_input("rename1", fs::rename("\0", "a"));
+    assert_invalid_input("rename2", fs::rename("a", "\0"));
+    assert_invalid_input("copy1", fs::copy("\0", "a"));
+    assert_invalid_input("copy2", fs::copy("a", "\0"));
+    assert_invalid_input("hard_link1", fs::hard_link("\0", "a"));
+    assert_invalid_input("hard_link2", fs::hard_link("a", "\0"));
+    assert_invalid_input("soft_link1", fs::soft_link("\0", "a"));
+    assert_invalid_input("soft_link2", fs::soft_link("a", "\0"));
+    assert_invalid_input("read_link", fs::read_link("\0"));
+    assert_invalid_input("canonicalize", fs::canonicalize("\0"));
+    assert_invalid_input("create_dir", fs::create_dir("\0"));
+    assert_invalid_input("create_dir_all", fs::create_dir_all("\0"));
+    assert_invalid_input("remove_dir", fs::remove_dir("\0"));
+    assert_invalid_input("remove_dir_all", fs::remove_dir_all("\0"));
+    assert_invalid_input("read_dir", fs::read_dir("\0"));
+    assert_invalid_input("set_permissions",
+                         fs::set_permissions("\0", fs::metadata(".").unwrap().permissions()));
+}