]> Cypherpunks repositories - gostls13.git/commitdiff
os: use WIN32_FIND_DATA.Reserved0 to identify symlinks
authorAlex Brainman <alex.brainman@gmail.com>
Sun, 7 Jan 2018 01:12:25 +0000 (12:12 +1100)
committerAlex Brainman <alex.brainman@gmail.com>
Wed, 7 Mar 2018 08:51:04 +0000 (08:51 +0000)
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    <DIR>          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 <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/os/dir_windows.go
src/os/os_windows_test.go
src/os/stat_windows.go
src/os/types_windows.go

index a738af276407b0a00f86ede5be54aa1ca0183f40..9e5d6bd5052024c0babea1c07457e27c07588cad 100644 (file)
@@ -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)
        }
index 47e2611a40d8fba611fe877c62f34a4c5d6d78d5..12cd9c1f2e205b72b024905c843badbeb87854aa 100644 (file)
@@ -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)
+}
index 5ec56422fdec7bcd0c21fcdb3e883e68b49a699a..0b54a154474151c625fab6a3f84642ce1aece181 100644 (file)
@@ -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
 }
index 9fcc0432848c844a4318f5372df4c3062fd2838d..235b9f1182b641c8c68df4fba95e2d622524fed4 100644 (file)
@@ -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()