From 190b3bfd0e7debf274022b64e294204a94074d1f Mon Sep 17 00:00:00 2001 From: Gareth Date: Mon, 31 Mar 2025 23:23:59 -0700 Subject: [PATCH] fix: glob escape some linux filename characters (#721) --- internal/api/backresthandler_test.go | 45 +++++++++++++++---------- internal/orchestrator/repo/repo.go | 17 +++++++--- internal/orchestrator/repo/repo_test.go | 10 ++++-- 3 files changed, 49 insertions(+), 23 deletions(-) diff --git a/internal/api/backresthandler_test.go b/internal/api/backresthandler_test.go index 0e77666..404eec3 100644 --- a/internal/api/backresthandler_test.go +++ b/internal/api/backresthandler_test.go @@ -602,15 +602,20 @@ func TestHookOnErrorHandling(t *testing.T) { t.Fatalf("Couldn't find hook operation in oplog: %v", err) } - backupOps := slices.DeleteFunc(getOperations(t, sut.oplog), func(op *v1.Operation) bool { - _, ok := op.GetOp().(*v1.Operation_OperationBackup) - return !ok - }) - if len(backupOps) != 1 { - t.Errorf("expected 1 backup operation, got %d", len(backupOps)) - } - if backupOps[0].Status != tc.wantBackupStatus { - t.Errorf("expected backup operation cancelled status, got %v", backupOps[0].Status) + if err := testutil.Retry(t, ctx, func() error { + backupOps := slices.DeleteFunc(getOperations(t, sut.oplog), func(op *v1.Operation) bool { + _, ok := op.GetOp().(*v1.Operation_OperationBackup) + return !ok + }) + if len(backupOps) != 1 { + return fmt.Errorf("expected 1 backup operation, got %d", len(backupOps)) + } + if backupOps[0].Status != tc.wantBackupStatus { + return fmt.Errorf("expected backup operation status %v, got %v", tc.wantBackupStatus, backupOps[0].Status) + } + return nil + }); err != nil { + t.Fatalf("Failed to verify backup operation: %v", err) } }) } @@ -969,7 +974,7 @@ func TestMultihostIndexSnapshots(t *testing.T) { host1.handler.Backup(context.Background(), connect.NewRequest(&types.StringValue{Value: "test1"})) host2.handler.Backup(context.Background(), connect.NewRequest(&types.StringValue{Value: "test2"})) - for i := 0; i < 2; i++ { + for i := 0; i < 1; i++ { if _, err := host1.handler.IndexSnapshots(context.Background(), connect.NewRequest(&types.StringValue{Value: "local1"})); err != nil { t.Errorf("local1 sut1 IndexSnapshots() error = %v", err) } @@ -1003,15 +1008,21 @@ func TestMultihostIndexSnapshots(t *testing.T) { testutil.TryNonfatal(t, ctx, func() error { ops = getOperations(t, host1.oplog) ops2 = getOperations(t, host2.oplog) - var err error - if ops := findSnapshotsFromInstance(ops, "test1"); len(ops) != 1 { - err = multierror.Append(err, fmt.Errorf("expected exactly 1 snapshot from test1, got %d", len(ops))) + for _, logOps := range []struct { + ops []*v1.Operation + instance string + }{ + {ops, "test1"}, + {ops, "test2"}, + {ops2, "test1"}, + {ops2, "test2"}, + } { + snapshotOps := findSnapshotsFromInstance(logOps.ops, logOps.instance) + if len(snapshotOps) != 1 { + err = multierror.Append(err, fmt.Errorf("expected exactly 1 snapshot from %s, got %d", logOps.instance, len(snapshotOps))) + } } - if ops := findSnapshotsFromInstance(ops2, "test2"); len(ops) != 1 { - err = multierror.Append(err, fmt.Errorf("expected exactly 1 snapshot from test2, got %d", len(ops))) - } - return err }) diff --git a/internal/orchestrator/repo/repo.go b/internal/orchestrator/repo/repo.go index 0cf9daf..c50e855 100644 --- a/internal/orchestrator/repo/repo.go +++ b/internal/orchestrator/repo/repo.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "path" + "runtime" "slices" "sort" "strings" @@ -314,13 +315,12 @@ func (r *RepoOrchestrator) Restore(ctx context.Context, snapshotId string, snaps dir := path.Dir(normalizedPath) base := path.Base(normalizedPath) - - + if dir != "" { - snapshotId = snapshotId + ":" + dir + snapshotId = snapshotId + ":" + dir } if base != "" { - opts = append(opts, restic.WithFlags("--include", base)) + opts = append(opts, restic.WithFlags("--include", escapeGlob(base))) } } @@ -434,3 +434,12 @@ func chunkBy[T any](items []T, chunkSize int) (chunks [][]T) { } return append(chunks, items) } + +var globEscapeReplacer = strings.NewReplacer(`\`, `\\`, `*`, `\*`, `?`, `\?`, `[`, `\[`, `]`, `\]`) + +func escapeGlob(s string) string { + if runtime.GOOS == "windows" { + return s // escaping is not supported on Windows + } + return globEscapeReplacer.Replace(s) +} diff --git a/internal/orchestrator/repo/repo_test.go b/internal/orchestrator/repo/repo_test.go index ac961ee..c27c11c 100644 --- a/internal/orchestrator/repo/repo_test.go +++ b/internal/orchestrator/repo/repo_test.go @@ -96,7 +96,13 @@ func TestBackup(t *testing.T) { func TestRestore(t *testing.T) { t.Parallel() - testFile := path.Join(t.TempDir(), "test.txt") + // Use a filepath that exercises a few of the glob characters to test escaping + messyFilePathToTestGlobs := "test.txt" + if runtime.GOOS != "windows" { + messyFilePathToTestGlobs = "test*?[].txt" + } + + testFile := path.Join(t.TempDir(), messyFilePathToTestGlobs) if err := ioutil.WriteFile(testFile, []byte("lorum ipsum"), 0644); err != nil { t.Fatalf("failed to create test file: %v", err) } @@ -149,7 +155,7 @@ func TestRestore(t *testing.T) { } // Check the restored file - restoredFile := path.Join(restoreDir, "test.txt") + restoredFile := path.Join(restoreDir, messyFilePathToTestGlobs) if _, err := os.Stat(restoredFile); err != nil { t.Fatalf("failed to stat restored file: %v", err) }