From 6e4c7f9b50ac99f31617fffc9aa8832ea30f2d7c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 2 Nov 2024 23:32:24 +0100 Subject: [PATCH] fs: add file handle based xattrs --- internal/fs/ea_windows.go | 10 +- internal/fs/ea_windows_test.go | 11 +- internal/fs/file_windows.go | 9 +- internal/fs/meta_fd_notwindows.go | 9 +- internal/fs/meta_windows.go | 28 +++-- internal/fs/meta_xattr.go | 14 ++- internal/fs/node_windows.go | 82 +------------ internal/fs/node_windows_test.go | 197 +++++++++--------------------- internal/fs/node_xattr.go | 36 +++++- 9 files changed, 140 insertions(+), 256 deletions(-) diff --git a/internal/fs/ea_windows.go b/internal/fs/ea_windows.go index fe9a3c42a..957a04836 100644 --- a/internal/fs/ea_windows.go +++ b/internal/fs/ea_windows.go @@ -284,14 +284,10 @@ func setFileEA(handle windows.Handle, iosb *ioStatusBlock, buf *uint8, bufLen ui return } -// pathSupportsExtendedAttributes returns true if the path supports extended attributes. -func pathSupportsExtendedAttributes(path string) (supported bool, err error) { +// handleSupportsExtendedAttributes returns true if the handle is on a volume that supports extended attributes. +func handleSupportsExtendedAttributes(handle windows.Handle) (supported bool, err error) { var fileSystemFlags uint32 - utf16Path, err := windows.UTF16PtrFromString(path) - if err != nil { - return false, err - } - err = windows.GetVolumeInformation(utf16Path, nil, 0, nil, nil, &fileSystemFlags, nil, 0) + err = windows.GetVolumeInformationByHandle(handle, nil, 0, nil, nil, &fileSystemFlags, nil, 0) if err != nil { return false, err } diff --git a/internal/fs/ea_windows_test.go b/internal/fs/ea_windows_test.go index 00cbe97f8..a1f940b49 100644 --- a/internal/fs/ea_windows_test.go +++ b/internal/fs/ea_windows_test.go @@ -14,6 +14,7 @@ import ( "testing" "unsafe" + rtest "github.com/restic/restic/internal/test" "golang.org/x/sys/windows" ) @@ -261,7 +262,9 @@ func TestPathSupportsExtendedAttributes(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - supported, err := pathSupportsExtendedAttributes(tc.path) + handle, err := openMetadataHandle(tc.path, false) + rtest.OK(t, err) + supported, err := handleSupportsExtendedAttributes(windows.Handle(handle.Fd())) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -272,8 +275,12 @@ func TestPathSupportsExtendedAttributes(t *testing.T) { } // Test with an invalid path - _, err := pathSupportsExtendedAttributes("Z:\\NonExistentPath-UAS664da5s4dyu56das45f5as") + + handle, err := openMetadataHandle("Z:\\NonExistentPath-UAS664da5s4dyu56das45f5as", false) + rtest.OK(t, err) + _, err = handleSupportsExtendedAttributes(windows.Handle(handle.Fd())) if err == nil { t.Error("Expected an error for non-existent path, but got nil") } + rtest.OK(t, handle.Close()) } diff --git a/internal/fs/file_windows.go b/internal/fs/file_windows.go index bea38511c..9ca5003a0 100644 --- a/internal/fs/file_windows.go +++ b/internal/fs/file_windows.go @@ -113,13 +113,10 @@ func clearAttribute(path string, attribute uint32) error { return nil } -// openHandleForEA return a file handle for file or dir for setting/getting EAs -func openHandleForEA(path string, writeAccess bool) (handle windows.Handle, err error) { +// openWriteHandleForEA return a file handle for a file or dir for setting EAs +func openWriteHandleForEA(path string) (handle windows.Handle, err error) { path = fixpath(path) - fileAccess := windows.FILE_READ_EA - if writeAccess { - fileAccess = fileAccess | windows.FILE_WRITE_EA - } + fileAccess := windows.FILE_READ_EA | windows.FILE_WRITE_EA utf16Path := windows.StringToUTF16Ptr(path) return windows.CreateFile(utf16Path, uint32(fileAccess), 0, nil, windows.OPEN_EXISTING, windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_BACKUP_SEMANTICS, 0) diff --git a/internal/fs/meta_fd_notwindows.go b/internal/fs/meta_fd_notwindows.go index ba1058739..c9404e4c4 100644 --- a/internal/fs/meta_fd_notwindows.go +++ b/internal/fs/meta_fd_notwindows.go @@ -5,8 +5,13 @@ package fs import "github.com/restic/restic/internal/restic" func (p *fdMetadataHandle) Xattr(ignoreListError bool) ([]restic.ExtendedAttribute, error) { - // FIXME - return xattrFromPath(p.Name(), ignoreListError) + path := p.Name() + return xattrFromPath( + path, + func() ([]string, error) { return flistxattr(p.f) }, + func(attr string) ([]byte, error) { return fgetxattr(p.f, attr) }, + ignoreListError, + ) } func (p *fdMetadataHandle) SecurityDescriptor() (*[]byte, error) { diff --git a/internal/fs/meta_windows.go b/internal/fs/meta_windows.go index fec9794d1..d6edeb939 100644 --- a/internal/fs/meta_windows.go +++ b/internal/fs/meta_windows.go @@ -11,23 +11,28 @@ import ( ) func (p *pathMetadataHandle) Xattr(_ bool) ([]restic.ExtendedAttribute, error) { - return xattrFromPath(p.name) + f, err := openMetadataHandle(p.name, false) + if err != nil { + return nil, err + } + defer func() { + cerr := f.Close() + if err == nil { + err = cerr + } + }() + + return xattrFromHandle(p.name, windows.Handle(f.Fd())) } -func xattrFromPath(path string) ([]restic.ExtendedAttribute, error) { - allowExtended, err := checkAndStoreEASupport(path) - if err != nil || !allowExtended { +func xattrFromHandle(path string, handle windows.Handle) ([]restic.ExtendedAttribute, error) { + if supp, err := handleSupportsExtendedAttributes(handle); err != nil || !supp { return nil, err } - var fileHandle windows.Handle - if fileHandle, err = openHandleForEA(path, false); err != nil { - return nil, errors.Errorf("get EA failed while opening file handle for path %v, with: %v", path, err) - } - defer closeFileHandle(fileHandle, path) //Get the windows Extended Attributes using the file handle var extAtts []extendedAttribute - extAtts, err = fgetEA(fileHandle) + extAtts, err := fgetEA(handle) debug.Log("fillExtendedAttributes(%v) %v", path, extAtts) if err != nil { return nil, errors.Errorf("get EA failed for path %v, with: %v", path, err) @@ -64,8 +69,7 @@ func (p *pathMetadataHandle) SecurityDescriptor() (buf *[]byte, err error) { } func (p *fdMetadataHandle) Xattr(_ bool) ([]restic.ExtendedAttribute, error) { - // FIXME - return xattrFromPath(p.name) + return xattrFromHandle(p.name, windows.Handle(p.f.Fd())) } func (p *fdMetadataHandle) SecurityDescriptor() (*[]byte, error) { diff --git a/internal/fs/meta_xattr.go b/internal/fs/meta_xattr.go index de72afab3..224154cb1 100644 --- a/internal/fs/meta_xattr.go +++ b/internal/fs/meta_xattr.go @@ -12,11 +12,17 @@ import ( ) func (p *pathMetadataHandle) Xattr(ignoreListError bool) ([]restic.ExtendedAttribute, error) { - return xattrFromPath(p.Name(), ignoreListError) + path := p.Name() + return xattrFromPath( + path, + func() ([]string, error) { return listxattr(path) }, + func(attr string) ([]byte, error) { return getxattr(path, attr) }, + ignoreListError, + ) } -func xattrFromPath(path string, ignoreListError bool) ([]restic.ExtendedAttribute, error) { - xattrs, err := listxattr(path) +func xattrFromPath(path string, listxattr func() ([]string, error), getxattr func(attr string) ([]byte, error), ignoreListError bool) ([]restic.ExtendedAttribute, error) { + xattrs, err := listxattr() debug.Log("fillExtendedAttributes(%v) %v %v", path, xattrs, err) if err != nil { if ignoreListError && isListxattrPermissionError(err) { @@ -27,7 +33,7 @@ func xattrFromPath(path string, ignoreListError bool) ([]restic.ExtendedAttribut extendedAttrs := make([]restic.ExtendedAttribute, 0, len(xattrs)) for _, attr := range xattrs { - attrVal, err := getxattr(path, attr) + attrVal, err := getxattr(attr) if err != nil { fmt.Fprintf(os.Stderr, "can not obtain extended attribute %v for %v:\n", attr, path) continue diff --git a/internal/fs/node_windows.go b/internal/fs/node_windows.go index a01f7a1b6..240be3cfd 100644 --- a/internal/fs/node_windows.go +++ b/internal/fs/node_windows.go @@ -6,7 +6,6 @@ import ( "path/filepath" "reflect" "strings" - "sync" "syscall" "unsafe" @@ -20,9 +19,6 @@ var ( modAdvapi32 = syscall.NewLazyDLL("advapi32.dll") procEncryptFile = modAdvapi32.NewProc("EncryptFileW") procDecryptFile = modAdvapi32.NewProc("DecryptFileW") - - // eaSupportedVolumesMap is a map of volumes to boolean values indicating if they support extended attributes. - eaSupportedVolumesMap = sync.Map{} ) const ( @@ -115,7 +111,7 @@ func restoreExtendedAttributes(nodeType restic.NodeType, path string, eas []exte } var fileHandle windows.Handle - if fileHandle, err = openHandleForEA(path, true); err != nil { + if fileHandle, err = openWriteHandleForEA(path); err != nil { return errors.Errorf("set EA failed while opening file handle for path %v, with: %v", path, err) } defer closeFileHandle(fileHandle, path) // Replaced inline defer with named function call @@ -337,82 +333,6 @@ func nodeFillGenericAttributes(node *restic.Node, meta metadataHandle, stat *Ext return err } -// checkAndStoreEASupport checks if the volume of the path supports extended attributes and stores the result in a map -// If the result is already in the map, it returns the result from the map. -func checkAndStoreEASupport(path string) (isEASupportedVolume bool, err error) { - var volumeName string - volumeName, err = prepareVolumeName(path) - if err != nil { - return false, err - } - - if volumeName != "" { - // First check if the manually prepared volume name is already in the map - eaSupportedValue, exists := eaSupportedVolumesMap.Load(volumeName) - if exists { - // Cache hit, immediately return the cached value - return eaSupportedValue.(bool), nil - } - // If not found, check if EA is supported with manually prepared volume name - isEASupportedVolume, err = pathSupportsExtendedAttributes(volumeName + `\`) - // If the prepared volume name is not valid, we will fetch the actual volume name next. - if err != nil && !errors.Is(err, windows.DNS_ERROR_INVALID_NAME) { - debug.Log("Error checking if extended attributes are supported for prepared volume name %s: %v", volumeName, err) - // There can be multiple errors like path does not exist, bad network path, etc. - // We just gracefully disallow extended attributes for cases. - return false, nil - } - } - // If an entry is not found, get the actual volume name - volumeNameActual, err := getVolumePathName(path) - if err != nil { - debug.Log("Error getting actual volume name %s for path %s: %v", volumeName, path, err) - // There can be multiple errors like path does not exist, bad network path, etc. - // We just gracefully disallow extended attributes for cases. - return false, nil - } - if volumeNameActual != volumeName { - // If the actual volume name is different, check cache for the actual volume name - eaSupportedValue, exists := eaSupportedVolumesMap.Load(volumeNameActual) - if exists { - // Cache hit, immediately return the cached value - return eaSupportedValue.(bool), nil - } - // If the actual volume name is different and is not in the map, again check if the new volume supports extended attributes with the actual volume name - isEASupportedVolume, err = pathSupportsExtendedAttributes(volumeNameActual + `\`) - // Debug log for cases where the prepared volume name is not valid - if err != nil { - debug.Log("Error checking if extended attributes are supported for actual volume name %s: %v", volumeNameActual, err) - // There can be multiple errors like path does not exist, bad network path, etc. - // We just gracefully disallow extended attributes for cases. - return false, nil - } else { - debug.Log("Checking extended attributes. Prepared volume name: %s, actual volume name: %s, isEASupportedVolume: %v, err: %v", volumeName, volumeNameActual, isEASupportedVolume, err) - } - } - if volumeNameActual != "" { - eaSupportedVolumesMap.Store(volumeNameActual, isEASupportedVolume) - } - return isEASupportedVolume, err -} - -// getVolumePathName returns the volume path name for the given path. -func getVolumePathName(path string) (volumeName string, err error) { - utf16Path, err := windows.UTF16PtrFromString(path) - if err != nil { - return "", err - } - // Get the volume path (e.g., "D:") - var volumePath [windows.MAX_PATH + 1]uint16 - err = windows.GetVolumePathName(utf16Path, &volumePath[0], windows.MAX_PATH+1) - if err != nil { - return "", err - } - // Trim any trailing backslashes - volumeName = strings.TrimRight(windows.UTF16ToString(volumePath[:]), "\\") - return volumeName, nil -} - // isVolumePath returns whether a path refers to a volume func isVolumePath(path string) (bool, error) { volName, err := prepareVolumeName(path) diff --git a/internal/fs/node_windows_test.go b/internal/fs/node_windows_test.go index 438d91b84..ad7a6d409 100644 --- a/internal/fs/node_windows_test.go +++ b/internal/fs/node_windows_test.go @@ -372,126 +372,100 @@ func TestPrepareVolumeName(t *testing.T) { osVolumeGUIDVolume := filepath.VolumeName(osVolumeGUIDPath) testCases := []struct { - name string - path string - expectedVolume string - expectError bool - expectedEASupported bool - isRealPath bool + name string + path string + expectedVolume string + expectError bool }{ { - name: "Network drive path", - path: `Z:\Shared\Documents`, - expectedVolume: `Z:`, - expectError: false, - expectedEASupported: false, + name: "Network drive path", + path: `Z:\Shared\Documents`, + expectedVolume: `Z:`, + expectError: false, }, { - name: "Subst drive path", - path: `X:\Virtual\Folder`, - expectedVolume: `X:`, - expectError: false, - expectedEASupported: false, + name: "Subst drive path", + path: `X:\Virtual\Folder`, + expectedVolume: `X:`, + expectError: false, }, { - name: "Windows reserved path", - path: `\\.\` + os.Getenv("SystemDrive") + `\System32\drivers\etc\hosts`, - expectedVolume: `\\.\` + os.Getenv("SystemDrive"), - expectError: false, - expectedEASupported: true, - isRealPath: true, + name: "Windows reserved path", + path: `\\.\` + os.Getenv("SystemDrive") + `\System32\drivers\etc\hosts`, + expectedVolume: `\\.\` + os.Getenv("SystemDrive"), + expectError: false, }, { - name: "Long UNC path", - path: `\\?\UNC\LongServerName\VeryLongShareName\DeepPath\File.txt`, - expectedVolume: `\\LongServerName\VeryLongShareName`, - expectError: false, - expectedEASupported: false, + name: "Long UNC path", + path: `\\?\UNC\LongServerName\VeryLongShareName\DeepPath\File.txt`, + expectedVolume: `\\LongServerName\VeryLongShareName`, + expectError: false, }, { - name: "Volume GUID path", - path: osVolumeGUIDPath, - expectedVolume: osVolumeGUIDVolume, - expectError: false, - expectedEASupported: true, - isRealPath: true, + name: "Volume GUID path", + path: osVolumeGUIDPath, + expectedVolume: osVolumeGUIDVolume, + expectError: false, }, { - name: "Volume GUID path with subfolder", - path: osVolumeGUIDPath + `\Windows`, - expectedVolume: osVolumeGUIDVolume, - expectError: false, - expectedEASupported: true, - isRealPath: true, + name: "Volume GUID path with subfolder", + path: osVolumeGUIDPath + `\Windows`, + expectedVolume: osVolumeGUIDVolume, + expectError: false, }, { - name: "Standard path", - path: os.Getenv("SystemDrive") + `\Users\`, - expectedVolume: os.Getenv("SystemDrive"), - expectError: false, - expectedEASupported: true, - isRealPath: true, + name: "Standard path", + path: os.Getenv("SystemDrive") + `\Users\`, + expectedVolume: os.Getenv("SystemDrive"), + expectError: false, }, { - name: "Extended length path", - path: longFilePath, - expectedVolume: tempDirVolume, - expectError: false, - expectedEASupported: true, - isRealPath: true, + name: "Extended length path", + path: longFilePath, + expectedVolume: tempDirVolume, + expectError: false, }, { - name: "UNC path", - path: `\\server\share\folder`, - expectedVolume: `\\server\share`, - expectError: false, - expectedEASupported: false, + name: "UNC path", + path: `\\server\share\folder`, + expectedVolume: `\\server\share`, + expectError: false, }, { - name: "Extended UNC path", - path: `\\?\UNC\server\share\folder`, - expectedVolume: `\\server\share`, - expectError: false, - expectedEASupported: false, + name: "Extended UNC path", + path: `\\?\UNC\server\share\folder`, + expectedVolume: `\\server\share`, + expectError: false, }, { - name: "Volume Shadow Copy root", - path: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555`, - expectedVolume: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555`, - expectError: false, - expectedEASupported: false, + name: "Volume Shadow Copy root", + path: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555`, + expectedVolume: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555`, + expectError: false, }, { - name: "Volume Shadow Copy path", - path: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555\Users\test`, - expectedVolume: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555`, - expectError: false, - expectedEASupported: false, + name: "Volume Shadow Copy path", + path: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555\Users\test`, + expectedVolume: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555`, + expectError: false, }, { name: "Relative path", path: `folder\subfolder`, - expectedVolume: currentVolume, // Get current volume - expectError: false, - expectedEASupported: true, + expectedVolume: currentVolume, // Get current volume + expectError: false, }, { - name: "Empty path", - path: ``, - expectedVolume: currentVolume, - expectError: false, - expectedEASupported: true, - isRealPath: false, + name: "Empty path", + path: ``, + expectedVolume: currentVolume, + expectError: false, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - isEASupported, err := checkAndStoreEASupport(tc.path) - test.OK(t, err) - test.Equals(t, tc.expectedEASupported, isEASupported) - volume, err := prepareVolumeName(tc.path) if tc.expectError { @@ -500,18 +474,6 @@ func TestPrepareVolumeName(t *testing.T) { test.OK(t, err) } test.Equals(t, tc.expectedVolume, volume) - - if tc.isRealPath { - isEASupportedVolume, err := pathSupportsExtendedAttributes(volume + `\`) - // If the prepared volume name is not valid, we will next fetch the actual volume name. - test.OK(t, err) - - test.Equals(t, tc.expectedEASupported, isEASupportedVolume) - - actualVolume, err := getVolumePathName(tc.path) - test.OK(t, err) - test.Equals(t, tc.expectedVolume, actualVolume) - } }) } } @@ -536,46 +498,3 @@ func getOSVolumeGUIDPath(t *testing.T) string { return windows.UTF16ToString(volumeGUID[:]) } - -func TestGetVolumePathName(t *testing.T) { - tempDirVolume := filepath.VolumeName(os.TempDir()) - testCases := []struct { - name string - path string - expectedPrefix string - }{ - { - name: "Root directory", - path: os.Getenv("SystemDrive") + `\`, - expectedPrefix: os.Getenv("SystemDrive"), - }, - { - name: "Nested directory", - path: os.Getenv("SystemDrive") + `\Windows\System32`, - expectedPrefix: os.Getenv("SystemDrive"), - }, - { - name: "Temp directory", - path: os.TempDir() + `\`, - expectedPrefix: tempDirVolume, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - volumeName, err := getVolumePathName(tc.path) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - if !strings.HasPrefix(volumeName, tc.expectedPrefix) { - t.Errorf("Expected volume name to start with %s, but got %s", tc.expectedPrefix, volumeName) - } - }) - } - - // Test with an invalid path - _, err := getVolumePathName("Z:\\NonExistentPath") - if err == nil { - t.Error("Expected an error for non-existent path, but got nil") - } -} diff --git a/internal/fs/node_xattr.go b/internal/fs/node_xattr.go index 08c83b3d4..f0bc32f62 100644 --- a/internal/fs/node_xattr.go +++ b/internal/fs/node_xattr.go @@ -4,7 +4,9 @@ package fs import ( + "fmt" "os" + "runtime" "syscall" "github.com/restic/restic/internal/errors" @@ -13,12 +15,40 @@ import ( "github.com/pkg/xattr" ) +func linuxFdPath(fd uintptr) string { + // A file handle opened using O_PATH on Linux cannot be used to read xattrs. + // However, the file descriptor objects in the procfs filesystem + // can be used in place of the original file and therefore allow xattr access. + return fmt.Sprintf("/proc/self/fd/%d", int(fd)) +} + // getxattr retrieves extended attribute data associated with path. -func getxattr(path, name string) ([]byte, error) { - b, err := xattr.LGet(path, name) +func fgetxattr(f *os.File, name string) (b []byte, err error) { + if runtime.GOOS == "linux" { + b, err = xattr.Get(linuxFdPath(f.Fd()), name) + } else { + b, err = xattr.FGet(f, name) + } return b, handleXattrErr(err) } +// getxattr retrieves extended attribute data associated with path. +func getxattr(path string, name string) (b []byte, err error) { + b, err = xattr.LGet(path, name) + return b, handleXattrErr(err) +} + +// flistxattr retrieves a list of names of extended attributes associated with the +// given file. +func flistxattr(f *os.File) (l []string, err error) { + if runtime.GOOS == "linux" { + l, err = xattr.List(linuxFdPath(f.Fd())) + } else { + l, err = xattr.FList(f) + } + return l, handleXattrErr(err) +} + // listxattr retrieves a list of names of extended attributes associated with the // given path in the file system. func listxattr(path string) ([]string, error) { @@ -74,7 +104,7 @@ func nodeRestoreExtendedAttributes(node *restic.Node, path string) error { } // remove unexpected xattrs - xattrs, err := listxattr(path) + xattrs, err := llistxattr(path) if err != nil { return err }