From 2860e01853174e278900ef6907b1941b16fb1645 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Mon, 25 Mar 2024 16:37:05 +0100 Subject: [PATCH] os: make readdir more robust on Windows On Windows, File.readdir currently fails if the volume information can't be retrieved via GetVolumeInformationByHandle and if the directory path is relative and can't be converted to an absolute path. This change makes readdir more robust by not failing in these cases, as these steps are just necessary to support a potential call to os.SameFile, but not for the actual readdir operation. os.SameFile will still fail in these cases, but that's a separate issue tracked in #62042. Change-Id: I8d98d8379bdac4b2832fa433432a5f027756abaa Reviewed-on: https://go-review.googlesource.com/c/go/+/574155 LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov Reviewed-by: Alex Brainman Reviewed-by: Ian Lance Taylor Reviewed-by: Than McIntosh --- .../syscall/windows/symlink_windows.go | 1 + .../syscall/windows/syscall_windows.go | 15 ++++ src/os/dir_windows.go | 81 ++++++++++--------- 3 files changed, 60 insertions(+), 37 deletions(-) diff --git a/src/internal/syscall/windows/symlink_windows.go b/src/internal/syscall/windows/symlink_windows.go index 62e3f79986..b91246037b 100644 --- a/src/internal/syscall/windows/symlink_windows.go +++ b/src/internal/syscall/windows/symlink_windows.go @@ -9,6 +9,7 @@ import "syscall" const ( ERROR_INVALID_PARAMETER syscall.Errno = 87 + FILE_SUPPORTS_OBJECT_IDS = 0x00010000 FILE_SUPPORTS_OPEN_BY_FILE_ID = 0x01000000 // symlink support for CreateSymbolicLink() starting with Windows 10 (1703, v10.0.14972) diff --git a/src/internal/syscall/windows/syscall_windows.go b/src/internal/syscall/windows/syscall_windows.go index be629cc0f9..fb15e19c0e 100644 --- a/src/internal/syscall/windows/syscall_windows.go +++ b/src/internal/syscall/windows/syscall_windows.go @@ -473,3 +473,18 @@ const ( //sys OpenService(mgr syscall.Handle, serviceName *uint16, access uint32) (handle syscall.Handle, err error) = advapi32.OpenServiceW //sys QueryServiceStatus(hService syscall.Handle, lpServiceStatus *SERVICE_STATUS) (err error) = advapi32.QueryServiceStatus //sys OpenSCManager(machineName *uint16, databaseName *uint16, access uint32) (handle syscall.Handle, err error) [failretval==0] = advapi32.OpenSCManagerW + +func FinalPath(h syscall.Handle, flags uint32) (string, error) { + buf := make([]uint16, 100) + for { + n, err := GetFinalPathNameByHandle(h, &buf[0], uint32(len(buf)), flags) + if err != nil { + return "", err + } + if n < uint32(len(buf)) { + break + } + buf = make([]uint16, n) + } + return syscall.UTF16ToString(buf), nil +} diff --git a/src/os/dir_windows.go b/src/os/dir_windows.go index 5ba1d4640a..0dbc3aec3e 100644 --- a/src/os/dir_windows.go +++ b/src/os/dir_windows.go @@ -21,6 +21,7 @@ type dirInfo struct { // buf to dirBufPool. buf *[]byte // buffer for directory I/O bufp int // location of next record in buf + h syscall.Handle vol uint32 class uint32 // type of entries in buf path string // absolute directory path, empty if the file system supports FILE_ID_BOTH_DIR_INFO @@ -45,6 +46,7 @@ var dirBufPool = sync.Pool{ } func (d *dirInfo) close() { + d.h = 0 if d.buf != nil { dirBufPool.Put(d.buf) d.buf = nil @@ -56,41 +58,44 @@ func (d *dirInfo) close() { // Useful for testing purposes. var allowReadDirFileID = true +func (d *dirInfo) init(h syscall.Handle) { + d.h = h + d.class = windows.FileFullDirectoryRestartInfo + // The previous settings are enough to read the directory entries. + // The following code is only needed to support os.SameFile. + + // It is safe to query d.vol once and reuse the value. + // Hard links are not allowed to reference files in other volumes. + // Junctions and symbolic links can reference files and directories in other volumes, + // but the reparse point should still live in the parent volume. + var flags uint32 + err := windows.GetVolumeInformationByHandle(h, nil, 0, &d.vol, nil, &flags, nil, 0) + if err != nil { + d.vol = 0 // Set to zero in case Windows writes garbage to it. + // If we can't get the volume information, we can't use os.SameFile, + // but we can still read the directory entries. + return + } + if flags&windows.FILE_SUPPORTS_OBJECT_IDS == 0 { + // The file system does not support object IDs, no need to continue. + return + } + if allowReadDirFileID && flags&windows.FILE_SUPPORTS_OPEN_BY_FILE_ID != 0 { + // Use FileIdBothDirectoryRestartInfo if available as it returns the file ID + // without the need to open the file. + d.class = windows.FileIdBothDirectoryRestartInfo + } else { + // If FileIdBothDirectoryRestartInfo is not available but objects IDs are supported, + // get the directory path so that os.SameFile can use it to open the file + // and retrieve the file ID. + d.path, _ = windows.FinalPath(h, windows.FILE_NAME_OPENED) + } +} + func (file *File) readdir(n int, mode readdirMode) (names []string, dirents []DirEntry, infos []FileInfo, err error) { - // If this file has no dirinfo, create one. if file.dirinfo == nil { - // vol is used by os.SameFile. - // It is safe to query it once and reuse the value. - // Hard links are not allowed to reference files in other volumes. - // Junctions and symbolic links can reference files and directories in other volumes, - // but the reparse point should still live in the parent volume. - var vol, flags uint32 - err = windows.GetVolumeInformationByHandle(file.pfd.Sysfd, nil, 0, &vol, nil, &flags, nil, 0) - runtime.KeepAlive(file) - if err != nil { - err = &PathError{Op: "readdir", Path: file.name, Err: err} - return - } file.dirinfo = new(dirInfo) - file.dirinfo.vol = vol - if allowReadDirFileID && flags&windows.FILE_SUPPORTS_OPEN_BY_FILE_ID != 0 { - file.dirinfo.class = windows.FileIdBothDirectoryRestartInfo - } else { - file.dirinfo.class = windows.FileFullDirectoryRestartInfo - // Set the directory path for use by os.SameFile, as it is possible that - // the file system supports retrieving the file ID using GetFileInformationByHandle. - file.dirinfo.path = file.name - if !isAbs(file.dirinfo.path) { - // If the path is relative, we need to convert it to an absolute path - // in case the current directory changes between this call and a - // call to os.SameFile. - file.dirinfo.path, err = syscall.FullPath(file.dirinfo.path) - if err != nil { - err = &PathError{Op: "readdir", Path: file.name, Err: err} - return - } - } - } + file.dirinfo.init(file.pfd.Sysfd) } d := file.dirinfo if d.buf == nil { @@ -172,11 +177,13 @@ func (file *File) readdir(n int, mode readdirMode) (names []string, dirents []Di f = newFileStatFromFileIDBothDirInfo((*windows.FILE_ID_BOTH_DIR_INFO)(entry)) } else { f = newFileStatFromFileFullDirInfo((*windows.FILE_FULL_DIR_INFO)(entry)) - // Defer appending the entry name to the parent directory path until - // it is really needed, to avoid allocating a string that may not be used. - // It is currently only used in os.SameFile. - f.appendNameToPath = true - f.path = d.path + if d.path != "" { + // Defer appending the entry name to the parent directory path until + // it is really needed, to avoid allocating a string that may not be used. + // It is currently only used in os.SameFile. + f.appendNameToPath = true + f.path = d.path + } } f.name = name f.vol = d.vol