]> Cypherpunks repositories - gostls13.git/commitdiff
os,syscall: File.Stat to use file handle for directories on Windows
authorqmuntal <quimmuntal@gmail.com>
Tue, 10 May 2022 07:52:20 +0000 (09:52 +0200)
committerRoland Shoemaker <roland@golang.org>
Mon, 14 Nov 2022 19:47:59 +0000 (19:47 +0000)
Updates syscall.Open to support opening directories via CreateFileW.

CreateFileW handles are more versatile than FindFirstFile handles.
They can be used in Win32 APIs like GetFileInformationByHandle and
SetFilePointerEx, which are needed by some Go APIs.

Fixes #52747
Fixes #36019

Change-Id: I26a00cef9844fb4abeeb18d2f9d854162a146651
Reviewed-on: https://go-review.googlesource.com/c/go/+/405275
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Patrik Nyblom <pnyb@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/internal/poll/fd_windows.go
src/os/dir_windows.go
src/os/file.go
src/os/file_windows.go
src/os/os_test.go
src/os/os_windows_test.go
src/os/stat_windows.go
src/syscall/syscall_windows.go
src/syscall/syscall_windows_test.go

index 1af2011f94d63ef3caab42657ef4ae0640554ed2..3a4a74f2aeb559e96e4360cb8afaefc2146cbfa4 100644 (file)
@@ -268,7 +268,6 @@ const (
        kindNet fileKind = iota
        kindFile
        kindConsole
-       kindDir
        kindPipe
 )
 
@@ -286,12 +285,10 @@ func (fd *FD) Init(net string, pollable bool) (string, error) {
        }
 
        switch net {
-       case "file":
+       case "file", "dir":
                fd.kind = kindFile
        case "console":
                fd.kind = kindConsole
-       case "dir":
-               fd.kind = kindDir
        case "pipe":
                fd.kind = kindPipe
        case "tcp", "tcp4", "tcp6",
@@ -371,8 +368,6 @@ func (fd *FD) destroy() error {
        case kindNet:
                // The net package uses the CloseFunc variable for testing.
                err = CloseFunc(fd.Sysfd)
-       case kindDir:
-               err = syscall.FindClose(fd.Sysfd)
        default:
                err = syscall.CloseHandle(fd.Sysfd)
        }
@@ -1008,15 +1003,6 @@ func (fd *FD) Seek(offset int64, whence int) (int64, error) {
        return syscall.Seek(fd.Sysfd, offset, whence)
 }
 
-// FindNextFile wraps syscall.FindNextFile.
-func (fd *FD) FindNextFile(data *syscall.Win32finddata) error {
-       if err := fd.incref(); err != nil {
-               return err
-       }
-       defer fd.decref()
-       return syscall.FindNextFile(fd.Sysfd, data)
-}
-
 // Fchmod updates syscall.ByHandleFileInformation.Fileattributes when needed.
 func (fd *FD) Fchmod(mode uint32) error {
        if err := fd.incref(); err != nil {
index 253adad0b90e503b8c0a1756bacf687dcfabbbe1..445e4f7c4ff66858060cc7b7dab4f1614cd65eb1 100644 (file)
@@ -11,8 +11,15 @@ import (
 )
 
 func (file *File) readdir(n int, mode readdirMode) (names []string, dirents []DirEntry, infos []FileInfo, err error) {
-       if !file.isdir() {
-               return nil, nil, nil, &PathError{Op: "readdir", Path: file.name, Err: syscall.ENOTDIR}
+       // If this file has no dirinfo, create one.
+       needdata := true
+       if file.dirinfo == nil {
+               needdata = false
+               file.dirinfo, err = openDir(file.name)
+               if err != nil {
+                       err = &PathError{Op: "readdir", Path: file.name, Err: err}
+                       return
+               }
        }
        wantAll := n <= 0
        if wantAll {
@@ -20,8 +27,8 @@ func (file *File) readdir(n int, mode readdirMode) (names []string, dirents []Di
        }
        d := &file.dirinfo.data
        for n != 0 && !file.dirinfo.isempty {
-               if file.dirinfo.needdata {
-                       e := file.pfd.FindNextFile(d)
+               if needdata {
+                       e := syscall.FindNextFile(file.dirinfo.h, d)
                        runtime.KeepAlive(file)
                        if e != nil {
                                if e == syscall.ERROR_NO_MORE_FILES {
@@ -32,7 +39,7 @@ func (file *File) readdir(n int, mode readdirMode) (names []string, dirents []Di
                                }
                        }
                }
-               file.dirinfo.needdata = true
+               needdata = true
                name := syscall.UTF16ToString(d.FileName[0:])
                if name == "." || name == ".." { // Useless names
                        continue
index 070ccd0264d023fec0e2feacb3d5e3d5ca9fe128..753aeb662a1b3aa10194d74d9775737ce9cd45fc 100644 (file)
@@ -225,10 +225,6 @@ func (f *File) WriteAt(b []byte, off int64) (n int, err error) {
 // relative to the current offset, and 2 means relative to the end.
 // It returns the new offset and an error, if any.
 // The behavior of Seek on a file opened with O_APPEND is not specified.
-//
-// If f is a directory, the behavior of Seek varies by operating
-// system; you can seek to the beginning of the directory on Unix-like
-// operating systems, but not on Windows.
 func (f *File) Seek(offset int64, whence int) (ret int64, err error) {
        if err := f.checkValid("seek"); err != nil {
                return 0, err
index db5c27dd30f41f6d6f79afa34636e92b098cd1ff..d94b78f52478d1926ac52b87fbe0695c94045f56 100644 (file)
@@ -86,10 +86,14 @@ func NewFile(fd uintptr, name string) *File {
 
 // Auxiliary information if the File describes a directory
 type dirInfo struct {
-       data     syscall.Win32finddata
-       needdata bool
-       path     string
-       isempty  bool // set if FindFirstFile returns ERROR_FILE_NOT_FOUND
+       h       syscall.Handle // search handle created with FindFirstFile
+       data    syscall.Win32finddata
+       path    string
+       isempty bool // set if FindFirstFile returns ERROR_FILE_NOT_FOUND
+}
+
+func (d *dirInfo) close() error {
+       return syscall.FindClose(d.h)
 }
 
 func epipecheck(file *File, e error) {
@@ -99,17 +103,7 @@ func epipecheck(file *File, e error) {
 // On Unix-like systems, it is "/dev/null"; on Windows, "NUL".
 const DevNull = "NUL"
 
-func (f *file) isdir() bool { return f != nil && f.dirinfo != nil }
-
-func openFile(name string, flag int, perm FileMode) (file *File, err error) {
-       r, e := syscall.Open(fixLongPath(name), flag|syscall.O_CLOEXEC, syscallMode(perm))
-       if e != nil {
-               return nil, e
-       }
-       return newFile(r, name, "file"), nil
-}
-
-func openDir(name string) (file *File, err error) {
+func openDir(name string) (d *dirInfo, e error) {
        var mask string
 
        path := fixLongPath(name)
@@ -130,25 +124,27 @@ func openDir(name string) (file *File, err error) {
        if e != nil {
                return nil, e
        }
-       d := new(dirInfo)
-       r, e := syscall.FindFirstFile(maskp, &d.data)
+       d = new(dirInfo)
+       d.h, e = syscall.FindFirstFile(maskp, &d.data)
        if e != nil {
                // FindFirstFile returns ERROR_FILE_NOT_FOUND when
                // no matching files can be found. Then, if directory
                // exists, we should proceed.
-               if e != syscall.ERROR_FILE_NOT_FOUND {
-                       return nil, e
-               }
+               // If FindFirstFile failed because name does not point
+               // to a directory, we should return ENOTDIR.
                var fa syscall.Win32FileAttributeData
-               pathp, e := syscall.UTF16PtrFromString(path)
-               if e != nil {
+               pathp, e1 := syscall.UTF16PtrFromString(path)
+               if e1 != nil {
                        return nil, e
                }
-               e = syscall.GetFileAttributesEx(pathp, syscall.GetFileExInfoStandard, (*byte)(unsafe.Pointer(&fa)))
-               if e != nil {
+               e1 = syscall.GetFileAttributesEx(pathp, syscall.GetFileExInfoStandard, (*byte)(unsafe.Pointer(&fa)))
+               if e1 != nil {
                        return nil, e
                }
                if fa.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY == 0 {
+                       return nil, syscall.ENOTDIR
+               }
+               if e != syscall.ERROR_FILE_NOT_FOUND {
                        return nil, e
                }
                d.isempty = true
@@ -157,12 +153,11 @@ func openDir(name string) (file *File, err error) {
        if !isAbs(d.path) {
                d.path, e = syscall.FullPath(d.path)
                if e != nil {
+                       d.close()
                        return nil, e
                }
        }
-       f := newFile(r, name, "dir")
-       f.dirinfo = d
-       return f, nil
+       return d, nil
 }
 
 // openFileNolog is the Windows implementation of OpenFile.
@@ -170,28 +165,36 @@ func openFileNolog(name string, flag int, perm FileMode) (*File, error) {
        if name == "" {
                return nil, &PathError{Op: "open", Path: name, Err: syscall.ENOENT}
        }
-       r, errf := openFile(name, flag, perm)
-       if errf == nil {
-               return r, nil
-       }
-       r, errd := openDir(name)
-       if errd == nil {
-               if flag&O_WRONLY != 0 || flag&O_RDWR != 0 {
-                       r.Close()
-                       return nil, &PathError{Op: "open", Path: name, Err: syscall.EISDIR}
+       path := fixLongPath(name)
+       r, e := syscall.Open(path, flag|syscall.O_CLOEXEC, syscallMode(perm))
+       if e != nil {
+               // We should return EISDIR when we are trying to open a directory with write access.
+               if e == syscall.ERROR_ACCESS_DENIED && (flag&O_WRONLY != 0 || flag&O_RDWR != 0) {
+                       pathp, e1 := syscall.UTF16PtrFromString(path)
+                       if e1 == nil {
+                               var fa syscall.Win32FileAttributeData
+                               e1 = syscall.GetFileAttributesEx(pathp, syscall.GetFileExInfoStandard, (*byte)(unsafe.Pointer(&fa)))
+                               if e1 == nil && fa.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
+                                       e = syscall.EISDIR
+                               }
+                       }
                }
-               return r, nil
+               return nil, &PathError{Op: "open", Path: name, Err: e}
+       }
+       f, e := newFile(r, name, "file"), nil
+       if e != nil {
+               return nil, &PathError{Op: "open", Path: name, Err: e}
        }
-       return nil, &PathError{Op: "open", Path: name, Err: errf}
+       return f, nil
 }
 
 func (file *file) close() error {
        if file == nil {
                return syscall.EINVAL
        }
-       if file.isdir() && file.dirinfo.isempty {
-               // "special" empty directories
-               return nil
+       if file.dirinfo != nil {
+               file.dirinfo.close()
+               file.dirinfo = nil
        }
        var err error
        if e := file.pfd.Close(); e != nil {
@@ -211,6 +214,12 @@ func (file *file) close() error {
 // relative to the current offset, and 2 means relative to the end.
 // It returns the new offset and an error, if any.
 func (f *File) seek(offset int64, whence int) (ret int64, err error) {
+       if f.dirinfo != nil {
+               // Free cached dirinfo, so we allocate a new one if we
+               // access this file as a directory again. See #35767 and #37161.
+               f.dirinfo.close()
+               f.dirinfo = nil
+       }
        ret, err = f.pfd.Seek(offset, whence)
        runtime.KeepAlive(f)
        return ret, err
index ee030a80a7314caa6a986236de1ae427d514223f..e548777bfc0089658490388cabdc887973937773 100644 (file)
@@ -2564,9 +2564,6 @@ func TestUserHomeDir(t *testing.T) {
 }
 
 func TestDirSeek(t *testing.T) {
-       if runtime.GOOS == "windows" {
-               testenv.SkipFlaky(t, 36019)
-       }
        wd, err := Getwd()
        if err != nil {
                t.Fatal(err)
index 55253cdf799dd54d612486aa24a9d26f6c08b9b8..b9fad71bfd6ff86a9da949fbb247cfea3315602b 100644 (file)
@@ -1252,3 +1252,28 @@ func TestWindowsReadlink(t *testing.T) {
        mklink(t, "relfilelink", "file")
        testReadlink(t, "relfilelink", "file")
 }
+
+func TestOpenDirTOCTOU(t *testing.T) {
+       // Check opened directories can't be renamed until the handle is closed.
+       // See issue 52747.
+       tmpdir := t.TempDir()
+       dir := filepath.Join(tmpdir, "dir")
+       if err := os.Mkdir(dir, 0777); err != nil {
+               t.Fatal(err)
+       }
+       f, err := os.Open(dir)
+       if err != nil {
+               t.Fatal(err)
+       }
+       newpath := filepath.Join(tmpdir, "dir1")
+       err = os.Rename(dir, newpath)
+       if err == nil || !errors.Is(err, windows.ERROR_SHARING_VIOLATION) {
+               f.Close()
+               t.Fatalf("Rename(%q, %q) = %v; want windows.ERROR_SHARING_VIOLATION", dir, newpath, err)
+       }
+       f.Close()
+       err = os.Rename(dir, newpath)
+       if err != nil {
+               t.Error(err)
+       }
+}
index f8f229c709dbd90d9c8e1ecc521edd73e1d1fef6..8747c198881cad65c764b5e8fa4ad90a0415c946 100644 (file)
@@ -16,10 +16,6 @@ func (file *File) Stat() (FileInfo, error) {
        if file == nil {
                return nil, ErrInvalid
        }
-       if file.isdir() {
-               // I don't know any better way to do that for directory
-               return Stat(file.dirinfo.path)
-       }
        return statHandle(file.name, file.pfd.Sysfd)
 }
 
index 9547ae0720d90c37d3aa2b267a2cffa0800acf4c..4fbcdcd3ff543d58881ff69e439a5979c95d881e 100644 (file)
@@ -371,8 +371,11 @@ func Open(path string, mode int, perm uint32) (fd Handle, err error) {
                        }
                }
        }
-       h, e := CreateFile(pathp, access, sharemode, sa, createmode, attrs, 0)
-       return h, e
+       if createmode == OPEN_EXISTING && access == GENERIC_READ {
+               // Necessary for opening directory handles.
+               attrs |= FILE_FLAG_BACKUP_SEMANTICS
+       }
+       return CreateFile(pathp, access, sharemode, sa, createmode, attrs, 0)
 }
 
 func Read(fd Handle, p []byte) (n int, err error) {
index 87f6580bdc553b903a3b24c761d4e57680ec006c..3b567218e2b50da3540dba82ee9ccf0e11b10d07 100644 (file)
@@ -15,6 +15,28 @@ import (
        "testing"
 )
 
+func TestOpen_Dir(t *testing.T) {
+       dir := t.TempDir()
+
+       h, err := syscall.Open(dir, syscall.O_RDONLY, 0)
+       if err != nil {
+               t.Fatalf("Open failed: %v", err)
+       }
+       syscall.CloseHandle(h)
+       h, err = syscall.Open(dir, syscall.O_RDONLY|syscall.O_TRUNC, 0)
+       if err == nil {
+               t.Error("Open should have failed")
+       } else {
+               syscall.CloseHandle(h)
+       }
+       h, err = syscall.Open(dir, syscall.O_RDONLY|syscall.O_CREAT, 0)
+       if err == nil {
+               t.Error("Open should have failed")
+       } else {
+               syscall.CloseHandle(h)
+       }
+}
+
 func TestWin32finddata(t *testing.T) {
        dir := t.TempDir()