diff --git a/changelog/unreleased/pull-4980 b/changelog/unreleased/pull-4980 index 264f347fa..5713db7a2 100644 --- a/changelog/unreleased/pull-4980 +++ b/changelog/unreleased/pull-4980 @@ -8,5 +8,6 @@ Restic now completely skips the attempt to fetch extended attributes for such volumes where it is not supported. https://github.com/restic/restic/pull/4980 +https://github.com/restic/restic/pull/4998 https://github.com/restic/restic/issues/4955 https://github.com/restic/restic/issues/4950 diff --git a/changelog/unreleased/pull-4998 b/changelog/unreleased/pull-4998 deleted file mode 100644 index 23ff3dbd2..000000000 --- a/changelog/unreleased/pull-4998 +++ /dev/null @@ -1,8 +0,0 @@ -Bugfix: Fix extended attributes handling for VSS snapshots - -Restic was failing to backup extended attributes for VSS snapshots -after the fix for https://github.com/restic/restic/pull/4980. -Restic now correctly handles extended attributes for VSS snapshots. - -https://github.com/restic/restic/pull/4998 -https://github.com/restic/restic/pull/4980 diff --git a/internal/fs/ea_windows_test.go b/internal/fs/ea_windows_test.go index b249f43c4..74afd7aa5 100644 --- a/internal/fs/ea_windows_test.go +++ b/internal/fs/ea_windows_test.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "reflect" + "strings" "syscall" "testing" "unsafe" @@ -245,3 +246,78 @@ func testSetGetEA(t *testing.T, path string, handle windows.Handle, testEAs []Ex t.Fatalf("EAs read from path %s don't match", path) } } + +func TestPathSupportsExtendedAttributes(t *testing.T) { + testCases := []struct { + name string + path string + expected bool + }{ + { + name: "System drive", + path: os.Getenv("SystemDrive") + `\`, + expected: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + supported, err := PathSupportsExtendedAttributes(tc.path) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if supported != tc.expected { + t.Errorf("Expected %v, got %v for path %s", tc.expected, supported, tc.path) + } + }) + } + + // Test with an invalid path + _, err := PathSupportsExtendedAttributes("Z:\\NonExistentPath-UAS664da5s4dyu56das45f5as") + if err == nil { + t.Error("Expected an error for non-existent path, but got nil") + } +} + +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/restic/node_windows.go b/internal/restic/node_windows.go index 2ca7e42e6..bce01ccad 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -42,6 +42,7 @@ const ( extendedPathPrefix = `\\?\` uncPathPrefix = `\\?\UNC\` globalRootPrefix = `\\?\GLOBALROOT\` + volumeGUIDPrefix = `\\?\Volume{` ) // mknod is not supported on Windows. @@ -422,15 +423,21 @@ func checkAndStoreEASupport(path string) (isEASupportedVolume bool, err error) { } // If not found, check if EA is supported with manually prepared volume name isEASupportedVolume, err = fs.PathSupportsExtendedAttributes(volumeName + `\`) - // If the prepared volume name is not valid, we will next fetch the actual volume name. + // 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) { - return false, err + 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 using the GetVolumePathName function volumeNameActual, err := fs.GetVolumePathName(path) if err != nil { - return false, err + 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 @@ -441,11 +448,19 @@ func checkAndStoreEASupport(path string) (isEASupportedVolume bool, err error) { } // 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 = fs.PathSupportsExtendedAttributes(volumeNameActual + `\`) + // Debug log for cases where the prepared volume name is not valid if err != nil { - return false, err + 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) } } - eaSupportedVolumesMap.Store(volumeNameActual, isEASupportedVolume) + if volumeNameActual != "" { + eaSupportedVolumesMap.Store(volumeNameActual, isEASupportedVolume) + } return isEASupportedVolume, err } @@ -460,17 +475,19 @@ func prepareVolumeName(path string) (volumeName string, err error) { volumeName = filepath.VolumeName(path) } } else { - if strings.HasPrefix(path, uncPathPrefix) { - // Convert \\?\UNC\ extended path to standard path to get the volume name correctly - path = `\\` + path[len(uncPathPrefix):] - } else if strings.HasPrefix(path, extendedPathPrefix) { - //Extended length path prefix needs to be trimmed to get the volume name correctly - path = path[len(extendedPathPrefix):] - } else { - // Use the absolute path - path, err = filepath.Abs(path) - if err != nil { - return "", fmt.Errorf("failed to get absolute path: %w", err) + if !strings.HasPrefix(path, volumeGUIDPrefix) { // Handle volume GUID path + if strings.HasPrefix(path, uncPathPrefix) { + // Convert \\?\UNC\ extended path to standard path to get the volume name correctly + path = `\\` + path[len(uncPathPrefix):] + } else if strings.HasPrefix(path, extendedPathPrefix) { + //Extended length path prefix needs to be trimmed to get the volume name correctly + path = path[len(extendedPathPrefix):] + } else { + // Use the absolute path + path, err = filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("failed to get absolute path: %w", err) + } } } volumeName = filepath.VolumeName(path) diff --git a/internal/restic/node_windows_test.go b/internal/restic/node_windows_test.go index 4fd57bbb7..6ba25559b 100644 --- a/internal/restic/node_windows_test.go +++ b/internal/restic/node_windows_test.go @@ -12,6 +12,7 @@ import ( "strings" "syscall" "testing" + "time" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/fs" @@ -329,3 +330,198 @@ func TestRestoreExtendedAttributes(t *testing.T) { } } } + +func TestPrepareVolumeName(t *testing.T) { + currentVolume := filepath.VolumeName(func() string { + // Get the current working directory + pwd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get current working directory: %v", err) + } + return pwd + }()) + // Create a temporary directory for the test + tempDir, err := os.MkdirTemp("", "restic_test_"+time.Now().Format("20060102150405")) + if err != nil { + t.Fatalf("Failed to create temp directory: %v", err) + } + defer os.RemoveAll(tempDir) + + // Create a long file name + longFileName := `\Very\Long\Path\That\Exceeds\260\Characters\` + strings.Repeat(`\VeryLongFolderName`, 20) + `\\LongFile.txt` + longFilePath := filepath.Join(tempDir, longFileName) + + tempDirVolume := filepath.VolumeName(tempDir) + // Create the file + content := []byte("This is a test file with a very long name.") + err = os.MkdirAll(filepath.Dir(longFilePath), 0755) + test.OK(t, err) + if err != nil { + t.Fatalf("Failed to create long folder: %v", err) + } + err = os.WriteFile(longFilePath, content, 0644) + test.OK(t, err) + if err != nil { + t.Fatalf("Failed to create long file: %v", err) + } + osVolumeGUIDPath := getOSVolumeGUIDPath(t) + osVolumeGUIDVolume := filepath.VolumeName(osVolumeGUIDPath) + + testCases := []struct { + name string + path string + expectedVolume string + expectError bool + expectedEASupported bool + isRealPath bool + }{ + { + name: "Network drive path", + path: `Z:\Shared\Documents`, + expectedVolume: `Z:`, + expectError: false, + expectedEASupported: false, + }, + { + name: "Subst drive path", + path: `X:\Virtual\Folder`, + expectedVolume: `X:`, + expectError: false, + expectedEASupported: false, + }, + { + name: "Windows reserved path", + path: `\\.\` + os.Getenv("SystemDrive") + `\System32\drivers\etc\hosts`, + expectedVolume: `\\.\` + os.Getenv("SystemDrive"), + expectError: false, + expectedEASupported: true, + isRealPath: true, + }, + { + name: "Long UNC path", + path: `\\?\UNC\LongServerName\VeryLongShareName\DeepPath\File.txt`, + expectedVolume: `\\LongServerName\VeryLongShareName`, + expectError: false, + expectedEASupported: false, + }, + { + name: "Volume GUID path", + path: osVolumeGUIDPath, + expectedVolume: osVolumeGUIDVolume, + expectError: false, + expectedEASupported: true, + isRealPath: true, + }, + { + name: "Volume GUID path with subfolder", + path: osVolumeGUIDPath + `\Windows`, + expectedVolume: osVolumeGUIDVolume, + expectError: false, + expectedEASupported: true, + isRealPath: true, + }, + { + name: "Standard path", + path: os.Getenv("SystemDrive") + `\Users\`, + expectedVolume: os.Getenv("SystemDrive"), + expectError: false, + expectedEASupported: true, + isRealPath: true, + }, + { + name: "Extended length path", + path: longFilePath, + expectedVolume: tempDirVolume, + expectError: false, + expectedEASupported: true, + isRealPath: true, + }, + { + name: "UNC path", + path: `\\server\share\folder`, + expectedVolume: `\\server\share`, + expectError: false, + expectedEASupported: false, + }, + { + name: "Extended UNC path", + path: `\\?\UNC\server\share\folder`, + expectedVolume: `\\server\share`, + expectError: false, + expectedEASupported: false, + }, + { + name: "Volume Shadow Copy path", + path: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1\Users\test`, + expectedVolume: `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1`, + expectError: false, + expectedEASupported: false, + }, + { + name: "Relative path", + path: `folder\subfolder`, + + expectedVolume: currentVolume, // Get current volume + expectError: false, + expectedEASupported: true, + }, + { + name: "Empty path", + path: ``, + expectedVolume: currentVolume, + expectError: false, + expectedEASupported: true, + isRealPath: 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 { + test.Assert(t, err != nil, "Expected an error, but got none") + } else { + test.OK(t, err) + } + test.Equals(t, tc.expectedVolume, volume) + + if tc.isRealPath { + isEASupportedVolume, err := fs.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 := fs.GetVolumePathName(tc.path) + test.OK(t, err) + test.Equals(t, tc.expectedVolume, actualVolume) + } + }) + } +} + +func getOSVolumeGUIDPath(t *testing.T) string { + // Get the path of the OS drive (usually C:\) + osDrive := os.Getenv("SystemDrive") + "\\" + + // Convert to a volume GUID path + volumeName, err := windows.UTF16PtrFromString(osDrive) + test.OK(t, err) + if err != nil { + return "" + } + + var volumeGUID [windows.MAX_PATH]uint16 + err = windows.GetVolumeNameForVolumeMountPoint(volumeName, &volumeGUID[0], windows.MAX_PATH) + test.OK(t, err) + if err != nil { + return "" + } + + return windows.UTF16ToString(volumeGUID[:]) +}