From e83601b4356f92f2f4d05f302d5654754ff05a6d Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Sun, 7 Jan 2018 12:12:25 +1100 Subject: [PATCH] os: use WIN32_FIND_DATA.Reserved0 to identify symlinks os.Stat implementation uses instructions described at https://blogs.msdn.microsoft.com/oldnewthing/20100212-00/?p=14963/ to distinguish symlinks. In particular, it calls GetFileAttributesEx or FindFirstFile and checks either WIN32_FILE_ATTRIBUTE_DATA.dwFileAttributes or WIN32_FIND_DATA.dwFileAttributes to see if FILE_ATTRIBUTES_REPARSE_POINT flag is set. And that seems to worked fine so far. But now we discovered that OneDrive root folder is determined as directory: c:\>dir C:\Users\Alex | grep OneDrive 30/11/2017 07:25 PM OneDrive c:\> while Go identified it as symlink. But we did not follow Microsoft's advice to the letter - we never checked WIN32_FIND_DATA.Reserved0. And adding that extra check makes Go treat OneDrive as symlink. So use FindFirstFile and WIN32_FIND_DATA.Reserved0 to determine symlinks. Fixes #22579 Change-Id: I0cb88929eb8b47b1d24efaf1907ad5a0e20de83f Reviewed-on: https://go-review.googlesource.com/86556 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/os/dir_windows.go | 17 ++--- src/os/os_windows_test.go | 86 +++++++++++++++++++++++ src/os/stat_windows.go | 138 +++++++----------------------------- src/os/types_windows.go | 144 +++++++++++++++++++++++++++++++++++--- 4 files changed, 250 insertions(+), 135 deletions(-) diff --git a/src/os/dir_windows.go b/src/os/dir_windows.go index a738af2764..9e5d6bd505 100644 --- a/src/os/dir_windows.go +++ b/src/os/dir_windows.go @@ -46,19 +46,10 @@ func (file *File) readdir(n int) (fi []FileInfo, err error) { if name == "." || name == ".." { // Useless names continue } - f := &fileStat{ - name: name, - sys: syscall.Win32FileAttributeData{ - FileAttributes: d.FileAttributes, - CreationTime: d.CreationTime, - LastAccessTime: d.LastAccessTime, - LastWriteTime: d.LastWriteTime, - FileSizeHigh: d.FileSizeHigh, - FileSizeLow: d.FileSizeLow, - }, - path: file.dirinfo.path, - appendNameToPath: true, - } + f := newFileStatFromWin32finddata(d) + f.name = name + f.path = file.dirinfo.path + f.appendNameToPath = true n-- fi = append(fi, f) } diff --git a/src/os/os_windows_test.go b/src/os/os_windows_test.go index 47e2611a40..12cd9c1f2e 100644 --- a/src/os/os_windows_test.go +++ b/src/os/os_windows_test.go @@ -8,6 +8,7 @@ import ( "fmt" "internal/poll" "internal/syscall/windows" + "internal/syscall/windows/registry" "internal/testenv" "io" "io/ioutil" @@ -893,3 +894,88 @@ func main() { } } } + +func testIsDir(t *testing.T, path string, fi os.FileInfo) { + t.Helper() + if !fi.IsDir() { + t.Errorf("%q should be a directory", path) + } + if fi.Mode()&os.ModeSymlink != 0 { + t.Errorf("%q should not be a symlink", path) + } +} + +func findOneDriveDir() (string, error) { + // as per https://stackoverflow.com/questions/42519624/how-to-determine-location-of-onedrive-on-windows-7-and-8-in-c + const onedrivekey = `SOFTWARE\Microsoft\OneDrive` + k, err := registry.OpenKey(registry.CURRENT_USER, onedrivekey, registry.READ) + if err != nil { + return "", fmt.Errorf("OpenKey(%q) failed: %v", onedrivekey, err) + } + defer k.Close() + + path, _, err := k.GetStringValue("UserFolder") + if err != nil { + return "", fmt.Errorf("reading UserFolder failed: %v", err) + } + return path, nil +} + +// TestOneDrive verifies that OneDrive folder is a directory and not a symlink. +func TestOneDrive(t *testing.T) { + dir, err := findOneDriveDir() + if err != nil { + t.Skipf("Skipping, because we did not find OneDrive directory: %v", err) + } + + // test os.Stat + fi, err := os.Stat(dir) + if err != nil { + t.Fatal(err) + } + testIsDir(t, dir, fi) + + // test os.Lstat + fi, err = os.Lstat(dir) + if err != nil { + t.Fatal(err) + } + testIsDir(t, dir, fi) + + // test os.File.Stat + f, err := os.Open(dir) + if err != nil { + t.Fatal(err) + } + defer f.Close() + + fi, err = f.Stat() + if err != nil { + t.Fatal(err) + } + testIsDir(t, dir, fi) + + // test os.FileInfo returned by os.Readdir + parent, err := os.Open(filepath.Dir(dir)) + if err != nil { + t.Fatal(err) + } + defer parent.Close() + + fis, err := parent.Readdir(-1) + if err != nil { + t.Fatal(err) + } + fi = nil + base := filepath.Base(dir) + for _, fi2 := range fis { + if fi2.Name() == base { + fi = fi2 + break + } + } + if fi == nil { + t.Errorf("failed to find %q in its parent", dir) + } + testIsDir(t, dir, fi) +} diff --git a/src/os/stat_windows.go b/src/os/stat_windows.go index 5ec56422fd..0b54a15447 100644 --- a/src/os/stat_windows.go +++ b/src/os/stat_windows.go @@ -5,9 +5,7 @@ package os import ( - "internal/syscall/windows" "syscall" - "unsafe" ) // Stat returns the FileInfo structure describing file. @@ -34,26 +32,12 @@ func (file *File) Stat() (FileInfo, error) { return &fileStat{name: basename(file.name), filetype: ft}, nil } - var d syscall.ByHandleFileInformation - err = file.pfd.GetFileInformationByHandle(&d) + fs, err := newFileStatFromGetFileInformationByHandle(file.name, file.pfd.Sysfd) if err != nil { - return nil, &PathError{"GetFileInformationByHandle", file.name, err} - } - return &fileStat{ - name: basename(file.name), - sys: syscall.Win32FileAttributeData{ - FileAttributes: d.FileAttributes, - CreationTime: d.CreationTime, - LastAccessTime: d.LastAccessTime, - LastWriteTime: d.LastWriteTime, - FileSizeHigh: d.FileSizeHigh, - FileSizeLow: d.FileSizeLow, - }, - filetype: ft, - vol: d.VolumeSerialNumber, - idxhi: d.FileIndexHigh, - idxlo: d.FileIndexLow, - }, nil + return nil, err + } + fs.filetype = ft + return fs, err } // statNolog implements Stat for Windows. @@ -68,91 +52,27 @@ func statNolog(name string) (FileInfo, error) { if err != nil { return nil, &PathError{"Stat", name, err} } - // Apparently (see https://golang.org/issues/19922#issuecomment-300031421) - // GetFileAttributesEx is fastest approach to get file info. - // It does not work for symlinks. But symlinks are rare, - // so try GetFileAttributesEx first. - var fs fileStat - err = syscall.GetFileAttributesEx(namep, syscall.GetFileExInfoStandard, (*byte)(unsafe.Pointer(&fs.sys))) - if err == nil && fs.sys.FileAttributes&syscall.FILE_ATTRIBUTE_REPARSE_POINT == 0 { - fs.path = name - if !isAbs(fs.path) { - fs.path, err = syscall.FullPath(fs.path) - if err != nil { - return nil, &PathError{"FullPath", name, err} - } + fs, err := newFileStatFromGetFileAttributesExOrFindFirstFile(name, namep) + if err != nil { + return nil, err + } + if !fs.isSymlink() { + err = fs.updatePathAndName(name) + if err != nil { + return nil, err } - fs.name = basename(name) - return &fs, nil + return fs, nil } // Use Windows I/O manager to dereference the symbolic link, as per // https://blogs.msdn.microsoft.com/oldnewthing/20100212-00/?p=14963/ h, err := syscall.CreateFile(namep, 0, 0, nil, syscall.OPEN_EXISTING, syscall.FILE_FLAG_BACKUP_SEMANTICS, 0) if err != nil { - if err == windows.ERROR_SHARING_VIOLATION { - // try FindFirstFile now that CreateFile failed - return statWithFindFirstFile(name, namep) - } return nil, &PathError{"CreateFile", name, err} } defer syscall.CloseHandle(h) - var d syscall.ByHandleFileInformation - err = syscall.GetFileInformationByHandle(h, &d) - if err != nil { - return nil, &PathError{"GetFileInformationByHandle", name, err} - } - return &fileStat{ - name: basename(name), - sys: syscall.Win32FileAttributeData{ - FileAttributes: d.FileAttributes, - CreationTime: d.CreationTime, - LastAccessTime: d.LastAccessTime, - LastWriteTime: d.LastWriteTime, - FileSizeHigh: d.FileSizeHigh, - FileSizeLow: d.FileSizeLow, - }, - vol: d.VolumeSerialNumber, - idxhi: d.FileIndexHigh, - idxlo: d.FileIndexLow, - // fileStat.path is used by os.SameFile to decide if it needs - // to fetch vol, idxhi and idxlo. But these are already set, - // so set fileStat.path to "" to prevent os.SameFile doing it again. - // Also do not set fileStat.filetype, because it is only used for - // console and stdin/stdout. But you cannot call os.Stat for these. - }, nil -} - -// statWithFindFirstFile is used by Stat to handle special case of statting -// c:\pagefile.sys. We might discover that other files need similar treatment. -func statWithFindFirstFile(name string, namep *uint16) (FileInfo, error) { - var fd syscall.Win32finddata - h, err := syscall.FindFirstFile(namep, &fd) - if err != nil { - return nil, &PathError{"FindFirstFile", name, err} - } - syscall.FindClose(h) - - fullpath := name - if !isAbs(fullpath) { - fullpath, err = syscall.FullPath(fullpath) - if err != nil { - return nil, &PathError{"FullPath", name, err} - } - } - return &fileStat{ - name: basename(name), - path: fullpath, - sys: syscall.Win32FileAttributeData{ - FileAttributes: fd.FileAttributes, - CreationTime: fd.CreationTime, - LastAccessTime: fd.LastAccessTime, - LastWriteTime: fd.LastWriteTime, - FileSizeHigh: fd.FileSizeHigh, - FileSizeLow: fd.FileSizeLow, - }, - }, nil + return newFileStatFromGetFileInformationByHandle(name, h) } // lstatNolog implements Lstat for Windows. @@ -163,25 +83,17 @@ func lstatNolog(name string) (FileInfo, error) { if name == DevNull { return &devNullStat, nil } - fs := &fileStat{name: basename(name)} - namep, e := syscall.UTF16PtrFromString(fixLongPath(name)) - if e != nil { - return nil, &PathError{"Lstat", name, e} + namep, err := syscall.UTF16PtrFromString(fixLongPath(name)) + if err != nil { + return nil, &PathError{"Lstat", name, err} + } + fs, err := newFileStatFromGetFileAttributesExOrFindFirstFile(name, namep) + if err != nil { + return nil, err } - e = syscall.GetFileAttributesEx(namep, syscall.GetFileExInfoStandard, (*byte)(unsafe.Pointer(&fs.sys))) - if e != nil { - if e != windows.ERROR_SHARING_VIOLATION { - return nil, &PathError{"GetFileAttributesEx", name, e} - } - // try FindFirstFile now that GetFileAttributesEx failed - return statWithFindFirstFile(name, namep) - } - fs.path = name - if !isAbs(fs.path) { - fs.path, e = syscall.FullPath(fs.path) - if e != nil { - return nil, &PathError{"FullPath", name, e} - } + err = fs.updatePathAndName(name) + if err != nil { + return nil, err } return fs, nil } diff --git a/src/os/types_windows.go b/src/os/types_windows.go index 9fcc043284..235b9f1182 100644 --- a/src/os/types_windows.go +++ b/src/os/types_windows.go @@ -5,16 +5,30 @@ package os import ( + "internal/syscall/windows" "sync" "syscall" "time" + "unsafe" ) // A fileStat is the implementation of FileInfo returned by Stat and Lstat. type fileStat struct { - name string - sys syscall.Win32FileAttributeData - filetype uint32 // what syscall.GetFileType returns + name string + + // from ByHandleFileInformation, Win32FileAttributeData and Win32finddata + FileAttributes uint32 + CreationTime syscall.Filetime + LastAccessTime syscall.Filetime + LastWriteTime syscall.Filetime + FileSizeHigh uint32 + FileSizeLow uint32 + + // from Win32finddata + Reserved0 uint32 + + // what syscall.GetFileType returns + filetype uint32 // used to implement SameFile sync.Mutex @@ -25,23 +39,126 @@ type fileStat struct { appendNameToPath bool } +// newFileStatFromGetFileInformationByHandle calls GetFileInformationByHandle +// to gather all required information about the file handle h. +func newFileStatFromGetFileInformationByHandle(path string, h syscall.Handle) (fs *fileStat, err error) { + var d syscall.ByHandleFileInformation + err = syscall.GetFileInformationByHandle(h, &d) + if err != nil { + return nil, &PathError{"GetFileInformationByHandle", path, err} + } + return &fileStat{ + name: basename(path), + FileAttributes: d.FileAttributes, + CreationTime: d.CreationTime, + LastAccessTime: d.LastAccessTime, + LastWriteTime: d.LastWriteTime, + FileSizeHigh: d.FileSizeHigh, + FileSizeLow: d.FileSizeLow, + vol: d.VolumeSerialNumber, + idxhi: d.FileIndexHigh, + idxlo: d.FileIndexLow, + // fileStat.path is used by os.SameFile to decide if it needs + // to fetch vol, idxhi and idxlo. But these are already set, + // so set fileStat.path to "" to prevent os.SameFile doing it again. + }, nil +} + +// newFileStatFromWin32finddata copies all required information +// from syscall.Win32finddata d into the newly created fileStat. +func newFileStatFromWin32finddata(d *syscall.Win32finddata) *fileStat { + return &fileStat{ + FileAttributes: d.FileAttributes, + CreationTime: d.CreationTime, + LastAccessTime: d.LastAccessTime, + LastWriteTime: d.LastWriteTime, + FileSizeHigh: d.FileSizeHigh, + FileSizeLow: d.FileSizeLow, + Reserved0: d.Reserved0, + } +} + +// newFileStatFromGetFileAttributesExOrFindFirstFile calls GetFileAttributesEx +// and FindFirstFile to gather all required information about the provided file path pathp. +func newFileStatFromGetFileAttributesExOrFindFirstFile(path string, pathp *uint16) (*fileStat, error) { + // As suggested by Microsoft, use GetFileAttributes() to acquire the file information, + // and if it's a reparse point use FindFirstFile() to get the tag: + // https://msdn.microsoft.com/en-us/library/windows/desktop/aa363940(v=vs.85).aspx + // Notice that always calling FindFirstFile can create performance problems + // (https://golang.org/issues/19922#issuecomment-300031421) + var fa syscall.Win32FileAttributeData + err := syscall.GetFileAttributesEx(pathp, syscall.GetFileExInfoStandard, (*byte)(unsafe.Pointer(&fa))) + if err == nil && fa.FileAttributes&syscall.FILE_ATTRIBUTE_REPARSE_POINT == 0 { + // Not a symlink. + return &fileStat{ + FileAttributes: fa.FileAttributes, + CreationTime: fa.CreationTime, + LastAccessTime: fa.LastAccessTime, + LastWriteTime: fa.LastWriteTime, + FileSizeHigh: fa.FileSizeHigh, + FileSizeLow: fa.FileSizeLow, + }, nil + } + // We might have symlink here. But some directories also have + // FileAttributes FILE_ATTRIBUTE_REPARSE_POINT bit set. + // For example, OneDrive directory is like that + // (see golang.org/issue/22579 for details). + // So use FindFirstFile instead to distinguish directories like + // OneDrive from real symlinks (see instructions described at + // https://blogs.msdn.microsoft.com/oldnewthing/20100212-00/?p=14963/ + // and in particular bits about using both FileAttributes and + // Reserved0 fields). + var fd syscall.Win32finddata + sh, err := syscall.FindFirstFile(pathp, &fd) + if err != nil { + return nil, &PathError{"FindFirstFile", path, err} + } + syscall.FindClose(sh) + + return newFileStatFromWin32finddata(&fd), nil +} + +func (fs *fileStat) updatePathAndName(name string) error { + fs.path = name + if !isAbs(fs.path) { + var err error + fs.path, err = syscall.FullPath(fs.path) + if err != nil { + return &PathError{"FullPath", name, err} + } + } + fs.name = basename(name) + return nil +} + +func (fs *fileStat) isSymlink() bool { + // Use instructions described at + // https://blogs.msdn.microsoft.com/oldnewthing/20100212-00/?p=14963/ + // to recognize whether it's a symlink. + if fs.FileAttributes&syscall.FILE_ATTRIBUTE_REPARSE_POINT == 0 { + return false + } + return fs.Reserved0 == syscall.IO_REPARSE_TAG_SYMLINK || + fs.Reserved0 == windows.IO_REPARSE_TAG_MOUNT_POINT +} + func (fs *fileStat) Size() int64 { - return int64(fs.sys.FileSizeHigh)<<32 + int64(fs.sys.FileSizeLow) + return int64(fs.FileSizeHigh)<<32 + int64(fs.FileSizeLow) } func (fs *fileStat) Mode() (m FileMode) { if fs == &devNullStat { return ModeDevice | ModeCharDevice | 0666 } - if fs.sys.FileAttributes&syscall.FILE_ATTRIBUTE_READONLY != 0 { + if fs.FileAttributes&syscall.FILE_ATTRIBUTE_READONLY != 0 { m |= 0444 } else { m |= 0666 } - if fs.sys.FileAttributes&syscall.FILE_ATTRIBUTE_REPARSE_POINT != 0 { + if fs.isSymlink() { return m | ModeSymlink } - if fs.sys.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 { + if fs.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 { m |= ModeDir | 0111 } switch fs.filetype { @@ -54,11 +171,20 @@ func (fs *fileStat) Mode() (m FileMode) { } func (fs *fileStat) ModTime() time.Time { - return time.Unix(0, fs.sys.LastWriteTime.Nanoseconds()) + return time.Unix(0, fs.LastWriteTime.Nanoseconds()) } // Sys returns syscall.Win32FileAttributeData for file fs. -func (fs *fileStat) Sys() interface{} { return &fs.sys } +func (fs *fileStat) Sys() interface{} { + return &syscall.Win32FileAttributeData{ + FileAttributes: fs.FileAttributes, + CreationTime: fs.CreationTime, + LastAccessTime: fs.LastAccessTime, + LastWriteTime: fs.LastWriteTime, + FileSizeHigh: fs.FileSizeHigh, + FileSizeLow: fs.FileSizeLow, + } +} func (fs *fileStat) loadFileId() error { fs.Lock() -- 2.48.1