[release/0.4] Cherry pick fixes from main branch

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
 }

fileinfo.go

@@ -1,48 +1,85 @@
+//go:build windows
 // +build windows
 
 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
 }
 
+// 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 := getFileInformationByHandleEx(syscall.Handle(f.Fd()), 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 := setFileInformationByHandle(syscall.Handle(f.Fd()), 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)
 	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 +90,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)

fileinfo_test.go

@@ -0,0 +1,186 @@
+package winio
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+	"unsafe"
+
+	"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)
+}
+
+// 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)
+			}
+		})
+	}
+}

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)
 	}