From 27516b11b10da4718adb83de027647080203d32b Mon Sep 17 00:00:00 2001 From: Kathryn Baldauf Date: Thu, 22 Apr 2021 10:33:11 -0700 Subject: [PATCH 1/3] Revert "[Temporary] Revert Implement winio.GetFileStandardInfo FileInfo commits " (cherry picked from commit 8f0d50b3b381b741820d6737cfbda5b9284be616) Signed-off-by: Kirtana Ashok --- backuptar/tar.go | 11 ++-- fileinfo.go | 36 ++++++++---- fileinfo_test.go | 134 ++++++++++++++++++++++++++++++++++++++++++++ zsyscall_windows.go | 18 ------ 4 files changed, 164 insertions(+), 35 deletions(-) create mode 100644 fileinfo_test.go diff --git a/backuptar/tar.go b/backuptar/tar.go index b32bee3c..689e4da6 100644 --- a/backuptar/tar.go +++ b/backuptar/tar.go @@ -15,6 +15,7 @@ import ( "time" "github.com/Microsoft/go-winio" + "golang.org/x/sys/windows" ) const ( @@ -325,11 +326,11 @@ func FileInfoFromHeader(hdr *tar.Header) (name string, size int64, fileInfo *win size = hdr.Size } fileInfo = &winio.FileBasicInfo{ - LastAccessTime: syscall.NsecToFiletime(hdr.AccessTime.UnixNano()), - LastWriteTime: syscall.NsecToFiletime(hdr.ModTime.UnixNano()), - ChangeTime: syscall.NsecToFiletime(hdr.ChangeTime.UnixNano()), + LastAccessTime: windows.NsecToFiletime(hdr.AccessTime.UnixNano()), + LastWriteTime: windows.NsecToFiletime(hdr.ModTime.UnixNano()), + ChangeTime: windows.NsecToFiletime(hdr.ChangeTime.UnixNano()), // Default to ModTime, we'll pull hdrCreationTime below if present - CreationTime: syscall.NsecToFiletime(hdr.ModTime.UnixNano()), + CreationTime: windows.NsecToFiletime(hdr.ModTime.UnixNano()), } if attrStr, ok := hdr.PAXRecords[hdrFileAttributes]; ok { attr, err := strconv.ParseUint(attrStr, 10, 32) @@ -347,7 +348,7 @@ func FileInfoFromHeader(hdr *tar.Header) (name string, size int64, fileInfo *win if err != nil { return "", 0, nil, err } - fileInfo.CreationTime = syscall.NsecToFiletime(creationTime.UnixNano()) + fileInfo.CreationTime = windows.NsecToFiletime(creationTime.UnixNano()) } return } diff --git a/fileinfo.go b/fileinfo.go index ada2fbab..3ab6bff6 100644 --- a/fileinfo.go +++ b/fileinfo.go @@ -5,21 +5,14 @@ package winio import ( "os" "runtime" - "syscall" "unsafe" -) - -//sys getFileInformationByHandleEx(h syscall.Handle, class uint32, buffer *byte, size uint32) (err error) = GetFileInformationByHandleEx -//sys setFileInformationByHandle(h syscall.Handle, class uint32, buffer *byte, size uint32) (err error) = SetFileInformationByHandle -const ( - fileBasicInfo = 0 - fileIDInfo = 0x12 + "golang.org/x/sys/windows" ) // FileBasicInfo contains file access time and file attributes information. type FileBasicInfo struct { - CreationTime, LastAccessTime, LastWriteTime, ChangeTime syscall.Filetime + CreationTime, LastAccessTime, LastWriteTime, ChangeTime windows.Filetime FileAttributes uint32 pad uint32 // padding } @@ -27,7 +20,7 @@ type FileBasicInfo struct { // GetFileBasicInfo retrieves times and attributes for a file. func GetFileBasicInfo(f *os.File) (*FileBasicInfo, error) { bi := &FileBasicInfo{} - if err := getFileInformationByHandleEx(syscall.Handle(f.Fd()), fileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil { + if err := windows.GetFileInformationByHandleEx(windows.Handle(f.Fd()), windows.FileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil { return nil, &os.PathError{Op: "GetFileInformationByHandleEx", Path: f.Name(), Err: err} } runtime.KeepAlive(f) @@ -36,13 +29,32 @@ func GetFileBasicInfo(f *os.File) (*FileBasicInfo, error) { // SetFileBasicInfo sets times and attributes for a file. func SetFileBasicInfo(f *os.File, bi *FileBasicInfo) error { - if err := setFileInformationByHandle(syscall.Handle(f.Fd()), fileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil { + if err := windows.SetFileInformationByHandle(windows.Handle(f.Fd()), windows.FileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil { return &os.PathError{Op: "SetFileInformationByHandle", Path: f.Name(), Err: err} } runtime.KeepAlive(f) return nil } +// FileStandardInfo contains extended information for the file. +// FILE_STANDARD_INFO in WinBase.h +// https://docs.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_standard_info +type FileStandardInfo struct { + AllocationSize, EndOfFile int64 + NumberOfLinks uint32 + DeletePending, Directory bool +} + +// GetFileStandardInfo retrieves ended information for the file. +func GetFileStandardInfo(f *os.File) (*FileStandardInfo, error) { + si := &FileStandardInfo{} + if err := windows.GetFileInformationByHandleEx(windows.Handle(f.Fd()), windows.FileStandardInfo, (*byte)(unsafe.Pointer(si)), uint32(unsafe.Sizeof(*si))); err != nil { + return nil, &os.PathError{Op: "GetFileInformationByHandleEx", Path: f.Name(), Err: err} + } + runtime.KeepAlive(f) + return si, nil +} + // FileIDInfo contains the volume serial number and file ID for a file. This pair should be // unique on a system. type FileIDInfo struct { @@ -53,7 +65,7 @@ type FileIDInfo struct { // GetFileID retrieves the unique (volume, file ID) pair for a file. func GetFileID(f *os.File) (*FileIDInfo, error) { fileID := &FileIDInfo{} - if err := getFileInformationByHandleEx(syscall.Handle(f.Fd()), fileIDInfo, (*byte)(unsafe.Pointer(fileID)), uint32(unsafe.Sizeof(*fileID))); err != nil { + if err := windows.GetFileInformationByHandleEx(windows.Handle(f.Fd()), windows.FileIdInfo, (*byte)(unsafe.Pointer(fileID)), uint32(unsafe.Sizeof(*fileID))); err != nil { return nil, &os.PathError{Op: "GetFileInformationByHandleEx", Path: f.Name(), Err: err} } runtime.KeepAlive(f) diff --git a/fileinfo_test.go b/fileinfo_test.go new file mode 100644 index 00000000..e79700ba --- /dev/null +++ b/fileinfo_test.go @@ -0,0 +1,134 @@ +package winio + +import ( + "io/ioutil" + "os" + "testing" + + "golang.org/x/sys/windows" +) + +// Checks if current matches expected. Note that AllocationSize is filesystem-specific, +// so we check that the current.AllocationSize is >= expected.AllocationSize. +// https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/5afa7f66-619c-48f3-955f-68c4ece704ae +func checkFileStandardInfo(t *testing.T, current, expected *FileStandardInfo) { + if current.AllocationSize < expected.AllocationSize { + t.Fatalf("FileStandardInfo unexpectedly had AllocationSize %d, expecting >=%d", current.AllocationSize, expected.AllocationSize) + } + + if current.EndOfFile != expected.EndOfFile { + t.Fatalf("FileStandardInfo unexpectedly had EndOfFile %d, expecting %d", current.EndOfFile, expected.EndOfFile) + } + + if current.NumberOfLinks != expected.NumberOfLinks { + t.Fatalf("FileStandardInfo unexpectedly had NumberOfLinks %d, expecting %d", current.NumberOfLinks, expected.NumberOfLinks) + } + + if current.DeletePending != expected.DeletePending { + if current.DeletePending { + t.Fatalf("FileStandardInfo unexpectedly DeletePending") + } else { + t.Fatalf("FileStandardInfo unexpectedly not DeletePending") + } + } + + if current.Directory != expected.Directory { + if current.Directory { + t.Fatalf("FileStandardInfo unexpectedly Directory") + } else { + t.Fatalf("FileStandardInfo unexpectedly not Directory") + } + } +} + +func TestGetFileStandardInfo_File(t *testing.T) { + f, err := ioutil.TempFile("", "tst") + if err != nil { + t.Fatal(err) + } + defer f.Close() + defer os.Remove(f.Name()) + + expectedFileInfo := &FileStandardInfo{ + AllocationSize: 0, + EndOfFile: 0, + NumberOfLinks: 1, + DeletePending: false, + Directory: false, + } + + info, err := GetFileStandardInfo(f) + if err != nil { + t.Fatal(err) + } + checkFileStandardInfo(t, info, expectedFileInfo) + + bytesWritten, err := f.Write([]byte("0123456789")) + if err != nil { + t.Fatal(err) + } + + expectedFileInfo.EndOfFile = int64(bytesWritten) + expectedFileInfo.AllocationSize = int64(bytesWritten) + + info, err = GetFileStandardInfo(f) + if err != nil { + t.Fatal(err) + } + checkFileStandardInfo(t, info, expectedFileInfo) + + linkName := f.Name() + ".link" + + if err = os.Link(f.Name(), linkName); err != nil { + t.Fatal(err) + } + defer os.Remove(linkName) + + expectedFileInfo.NumberOfLinks = 2 + + info, err = GetFileStandardInfo(f) + if err != nil { + t.Fatal(err) + } + checkFileStandardInfo(t, info, expectedFileInfo) + + os.Remove(linkName) + + expectedFileInfo.NumberOfLinks = 1 + + info, err = GetFileStandardInfo(f) + if err != nil { + t.Fatal(err) + } + checkFileStandardInfo(t, info, expectedFileInfo) +} + +func TestGetFileStandardInfo_Directory(t *testing.T) { + tempDir, err := ioutil.TempDir("", "tst") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tempDir) + + // os.Open returns the Search Handle, not the Directory Handle + // See https://github.com/golang/go/issues/13738 + f, err := OpenForBackup(tempDir, windows.GENERIC_READ, 0, windows.OPEN_EXISTING) + if err != nil { + t.Fatal(err) + } + defer f.Close() + + expectedFileInfo := &FileStandardInfo{ + AllocationSize: 0, + EndOfFile: 0, + NumberOfLinks: 1, + DeletePending: false, + Directory: true, + } + + info, err := GetFileStandardInfo(f) + if err != nil { + t.Fatal(err) + } + checkFileStandardInfo(t, info, expectedFileInfo) +} diff --git a/zsyscall_windows.go b/zsyscall_windows.go index 4579c745..176ff75e 100644 --- a/zsyscall_windows.go +++ b/zsyscall_windows.go @@ -63,14 +63,12 @@ var ( procCreateIoCompletionPort = modkernel32.NewProc("CreateIoCompletionPort") procCreateNamedPipeW = modkernel32.NewProc("CreateNamedPipeW") procGetCurrentThread = modkernel32.NewProc("GetCurrentThread") - procGetFileInformationByHandleEx = modkernel32.NewProc("GetFileInformationByHandleEx") procGetNamedPipeHandleStateW = modkernel32.NewProc("GetNamedPipeHandleStateW") procGetNamedPipeInfo = modkernel32.NewProc("GetNamedPipeInfo") procGetQueuedCompletionStatus = modkernel32.NewProc("GetQueuedCompletionStatus") procLocalAlloc = modkernel32.NewProc("LocalAlloc") procLocalFree = modkernel32.NewProc("LocalFree") procSetFileCompletionNotificationModes = modkernel32.NewProc("SetFileCompletionNotificationModes") - procSetFileInformationByHandle = modkernel32.NewProc("SetFileInformationByHandle") procNtCreateNamedPipeFile = modntdll.NewProc("NtCreateNamedPipeFile") procRtlDefaultNpAcl = modntdll.NewProc("RtlDefaultNpAcl") procRtlDosPathNameToNtPathName_U = modntdll.NewProc("RtlDosPathNameToNtPathName_U") @@ -339,14 +337,6 @@ func getCurrentThread() (h syscall.Handle) { return } -func getFileInformationByHandleEx(h syscall.Handle, class uint32, buffer *byte, size uint32) (err error) { - r1, _, e1 := syscall.Syscall6(procGetFileInformationByHandleEx.Addr(), 4, uintptr(h), uintptr(class), uintptr(unsafe.Pointer(buffer)), uintptr(size), 0, 0) - if r1 == 0 { - err = errnoErr(e1) - } - return -} - func getNamedPipeHandleState(pipe syscall.Handle, state *uint32, curInstances *uint32, maxCollectionCount *uint32, collectDataTimeout *uint32, userName *uint16, maxUserNameSize uint32) (err error) { r1, _, e1 := syscall.Syscall9(procGetNamedPipeHandleStateW.Addr(), 7, uintptr(pipe), uintptr(unsafe.Pointer(state)), uintptr(unsafe.Pointer(curInstances)), uintptr(unsafe.Pointer(maxCollectionCount)), uintptr(unsafe.Pointer(collectDataTimeout)), uintptr(unsafe.Pointer(userName)), uintptr(maxUserNameSize), 0, 0) if r1 == 0 { @@ -390,14 +380,6 @@ func setFileCompletionNotificationModes(h syscall.Handle, flags uint8) (err erro return } -func setFileInformationByHandle(h syscall.Handle, class uint32, buffer *byte, size uint32) (err error) { - r1, _, e1 := syscall.Syscall6(procSetFileInformationByHandle.Addr(), 4, uintptr(h), uintptr(class), uintptr(unsafe.Pointer(buffer)), uintptr(size), 0, 0) - if r1 == 0 { - err = errnoErr(e1) - } - return -} - func ntCreateNamedPipeFile(pipe *syscall.Handle, access uint32, oa *objectAttributes, iosb *ioStatusBlock, share uint32, disposition uint32, options uint32, typ uint32, readMode uint32, completionMode uint32, maxInstances uint32, inboundQuota uint32, outputQuota uint32, timeout *int64) (status ntstatus) { r0, _, _ := syscall.Syscall15(procNtCreateNamedPipeFile.Addr(), 14, uintptr(unsafe.Pointer(pipe)), uintptr(access), uintptr(unsafe.Pointer(oa)), uintptr(unsafe.Pointer(iosb)), uintptr(share), uintptr(disposition), uintptr(options), uintptr(typ), uintptr(readMode), uintptr(completionMode), uintptr(maxInstances), uintptr(inboundQuota), uintptr(outputQuota), uintptr(unsafe.Pointer(timeout)), 0) status = ntstatus(r0) From ca561619a357c7a4ef4b533518346c71da431758 Mon Sep 17 00:00:00 2001 From: Davis Goodin Date: Tue, 16 Jan 2024 11:46:45 -0800 Subject: [PATCH 2/3] fileinfo: internally fix FileBasicInfo memory alignment (#312) * fileinfo: internally fix FileBasicInfo memory alignment Signed-off-by: Davis Goodin * Update test with review feedback Remove unused winName. Extract more into Windows alignment consts to repeat less. Document reason for having multiple alignment consts for the same value. Signed-off-by: Davis Goodin --------- Signed-off-by: Davis Goodin (cherry picked from commit 008bc6ea439f15884bef0b52f2772190c382bf46) Signed-off-by: Kirtana Ashok --- fileinfo.go | 33 ++++++++++++++++++++++++++---- fileinfo_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/fileinfo.go b/fileinfo.go index 3ab6bff6..ea439c84 100644 --- a/fileinfo.go +++ b/fileinfo.go @@ -1,3 +1,4 @@ +//go:build windows // +build windows package winio @@ -17,19 +18,43 @@ type FileBasicInfo struct { pad uint32 // padding } +// alignedFileBasicInfo is a FileBasicInfo, but aligned to uint64 by containing +// uint64 rather than windows.Filetime. Filetime contains two uint32s. uint64 +// alignment is necessary to pass this as FILE_BASIC_INFO. +type alignedFileBasicInfo struct { + CreationTime, LastAccessTime, LastWriteTime, ChangeTime uint64 + FileAttributes uint32 + _ uint32 // padding +} + // GetFileBasicInfo retrieves times and attributes for a file. func GetFileBasicInfo(f *os.File) (*FileBasicInfo, error) { - bi := &FileBasicInfo{} - if err := windows.GetFileInformationByHandleEx(windows.Handle(f.Fd()), windows.FileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil { + bi := &alignedFileBasicInfo{} + if err := windows.GetFileInformationByHandleEx( + windows.Handle(f.Fd()), + windows.FileBasicInfo, + (*byte)(unsafe.Pointer(bi)), + uint32(unsafe.Sizeof(*bi)), + ); err != nil { return nil, &os.PathError{Op: "GetFileInformationByHandleEx", Path: f.Name(), Err: err} } runtime.KeepAlive(f) - return bi, nil + // Reinterpret the alignedFileBasicInfo as a FileBasicInfo so it matches the + // public API of this module. The data may be unnecessarily aligned. + return (*FileBasicInfo)(unsafe.Pointer(bi)), nil } // SetFileBasicInfo sets times and attributes for a file. func SetFileBasicInfo(f *os.File, bi *FileBasicInfo) error { - if err := windows.SetFileInformationByHandle(windows.Handle(f.Fd()), windows.FileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil { + // Create an alignedFileBasicInfo based on a FileBasicInfo. The copy is + // suitable to pass to GetFileInformationByHandleEx. + biAligned := *(*alignedFileBasicInfo)(unsafe.Pointer(bi)) + if err := windows.SetFileInformationByHandle( + windows.Handle(f.Fd()), + windows.FileBasicInfo, + (*byte)(unsafe.Pointer(&biAligned)), + uint32(unsafe.Sizeof(biAligned)), + ); err != nil { return &os.PathError{Op: "SetFileInformationByHandle", Path: f.Name(), Err: err} } runtime.KeepAlive(f) diff --git a/fileinfo_test.go b/fileinfo_test.go index e79700ba..e34ec4cc 100644 --- a/fileinfo_test.go +++ b/fileinfo_test.go @@ -4,6 +4,7 @@ import ( "io/ioutil" "os" "testing" + "unsafe" "golang.org/x/sys/windows" ) @@ -132,3 +133,54 @@ func TestGetFileStandardInfo_Directory(t *testing.T) { } checkFileStandardInfo(t, info, expectedFileInfo) } + +// TestFileInfoStructAlignment checks that the alignment of Go fileinfo structs +// match what is expected by the Windows API. +func TestFileInfoStructAlignment(t *testing.T) { + //nolint:revive // SNAKE_CASE is not idiomatic in Go, but aligned with Win32 API. + const ( + // The alignment of various types, as named in the Windows APIs. When + // deciding on an expectedAlignment for a struct's test case, use the + // type of the largest field in the struct as written in the Windows + // docs. This is intended to help reviewers by allowing them to first + // check that a new align* const is correct, then independently check + // that the test case is correct, rather than all at once. + alignLARGE_INTEGER = unsafe.Alignof(uint64(0)) + alignULONGLONG = unsafe.Alignof(uint64(0)) + ) + tests := []struct { + name string + actualAlign uintptr + actualSize uintptr + expectedAlignment uintptr + }{ + { + // alignedFileBasicInfo is passed to the Windows API rather than FileBasicInfo. + "alignedFileBasicInfo", unsafe.Alignof(alignedFileBasicInfo{}), unsafe.Sizeof(alignedFileBasicInfo{}), + // https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_basic_info + alignLARGE_INTEGER, + }, + { + "FileStandardInfo", unsafe.Alignof(FileStandardInfo{}), unsafe.Sizeof(FileStandardInfo{}), + // https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_standard_info + alignLARGE_INTEGER, + }, + { + "FileIDInfo", unsafe.Alignof(FileIDInfo{}), unsafe.Sizeof(FileIDInfo{}), + // https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_id_info + alignULONGLONG, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.actualAlign != tt.expectedAlignment { + t.Errorf("alignment mismatch: actual %d, expected %d", tt.actualAlign, tt.expectedAlignment) + } + if r := tt.actualSize % tt.expectedAlignment; r != 0 { + t.Errorf( + "size is not a multiple of alignment: size %% alignment (%d %% %d) is %d, expected 0", + tt.actualSize, tt.expectedAlignment, r) + } + }) + } +} From 15989af11849e96188f37c4a261a145734651f0b Mon Sep 17 00:00:00 2001 From: Kirtana Ashok Date: Tue, 21 May 2024 12:19:44 -0700 Subject: [PATCH 3/3] fix TestGetFSTypeOfInvalidPath Signed-off-by: Kirtana Ashok --- pkg/fs/fs_windows_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/fs/fs_windows_test.go b/pkg/fs/fs_windows_test.go index 512b2352..f8c771d0 100644 --- a/pkg/fs/fs_windows_test.go +++ b/pkg/fs/fs_windows_test.go @@ -17,7 +17,9 @@ func TestGetFSTypeOfKnownDrive(t *testing.T) { } func TestGetFSTypeOfInvalidPath(t *testing.T) { - _, err := GetFileSystemType("7:\\") + // [filepath.VolumeName] doesn't mandate that the drive letters matches [a-zA-Z]. + // Instead, use non-character drive. + _, err := GetFileSystemType(`No:\`) if err != ErrInvalidPath { t.Fatalf("Expected `ErrInvalidPath`, got %v", err) }