Stop recursive symlink expansion and honor cancellation

2026-10-07 20:10:14 +00:00
parent 1498ec7498
commit 56e652aae3
3 changed files with 122 additions and 47 deletions
--- a/internal/validation/destination_test.go
+++ b/internal/validation/destination_test.go
@@ -2,10 +2,12 @@ package validation

 import (
 	"context"
+	"errors"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 )

 func TestResolveDestinationClassifiesCommittedPaths(t *testing.T) {
@@ -81,6 +83,65 @@ func TestResolveDestinationClassifiesCommittedPaths(t *testing.T) {
 	}
 }

+func TestResolveDestinationHandlesExpandingCyclesAndRepeatedLinks(t *testing.T) {
+	repository := newTreeRepository(t)
+	writeTreeFile(t, repository, "README.md", []byte("source\n"))
+	for name, target := range map[string]string{
+		"direct":   "direct/child",
+		"mutual-a": "mutual-b/child",
+		"mutual-b": "mutual-a/child",
+		"alias":    ".",
+	} {
+		if err := os.Symlink(target, filepath.Join(repository, name)); err != nil {
+			t.Fatal(err)
+		}
+	}
+	gitTree(t, repository, "add", "--all")
+	gitTree(t, repository, "commit", "-m", "capture recursive links")
+	tree, err := ReadCandidateTree(t.Context(), repository, strings.TrimSpace(gitTree(t, repository, "rev-parse", "HEAD")))
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, test := range []struct {
+		destination string
+		kind        DestinationKind
+	}{
+		{"direct", DestinationSymlinkCycle},
+		{"mutual-a", DestinationSymlinkCycle},
+		{"alias/alias/README.md", DestinationEntry},
+	} {
+		t.Run(test.destination, func(t *testing.T) {
+			type result struct {
+				destination Destination
+				err         error
+			}
+			done := make(chan result, 1)
+			ctx, cancel := context.WithTimeout(t.Context(), time.Second)
+			defer cancel()
+			go func() {
+				destination, err := tree.ResolveDestination(ctx, "README.md", test.destination)
+				done <- result{destination, err}
+			}()
+			select {
+			case got := <-done:
+				if got.err != nil || got.destination.Kind != test.kind {
+					t.Fatalf("resolution = %#v, %v; want %s", got.destination, got.err, test.kind)
+				}
+				if test.kind == DestinationEntry && got.destination.Path != "README.md" {
+					t.Fatalf("finite repeated link resolved to %q", got.destination.Path)
+				}
+			case <-ctx.Done():
+				t.Fatal("symlink resolution did not finish")
+			}
+		})
+	}
+	ctx, cancel := context.WithCancel(t.Context())
+	cancel()
+	if _, err := tree.ResolveDestination(ctx, "README.md", "alias/README.md"); !errors.Is(err, context.Canceled) {
+		t.Fatalf("canceled resolution error = %v", err)
+	}
+}
+
 func TestResolveDestinationIgnoresNeighboringFiles(t *testing.T) {
 	repository := newTreeRepository(t)
 	writeTreeFile(t, repository, "docs/page.md", []byte("source\n"))
--- a/internal/validation/tree.go
+++ b/internal/validation/tree.go
@@ -100,65 +100,73 @@ type Destination struct {
 // ResolveDestination resolves a repository-relative destination from the
 // source file's directory using only the captured Git tree.
 func (tree *CandidateTree) ResolveDestination(ctx context.Context, sourcePath, destination string) (Destination, error) {
+	if err := ctx.Err(); err != nil {
+		return Destination{}, err
+	}
 	if strings.HasPrefix(destination, "/") {
 		return Destination{Kind: DestinationOutsideCheckout, Path: path.Clean(destination)}, nil
 	}
 	resolved := path.Clean(path.Join(path.Dir(sourcePath), destination))
+	return tree.resolveDestination(ctx, resolved, make(map[string]bool))
+}
+
+func (tree *CandidateTree) resolveDestination(ctx context.Context, resolved string, active map[string]bool) (Destination, error) {
+	if err := ctx.Err(); err != nil {
+		return Destination{}, err
+	}
 	if outsideCheckout(resolved) {
 		return Destination{Kind: DestinationOutsideCheckout, Path: resolved}, nil
 	}
-	seen := make(map[string]struct{})
-	for {
-		components := strings.Split(resolved, "/")
-		partial := ""
-		restarted := false
-		for index, component := range components {
-			partial = path.Join(partial, component)
-			remaining := strings.Join(components[index+1:], "/")
-			entry, exists := tree.Entry(partial)
-			if !exists {
-				if _, directory := tree.directories[partial]; !directory {
-					return Destination{Kind: DestinationMissing, Path: resolved}, nil
-				}
-				continue
-			}
-			if entry.Mode == "160000" {
-				return Destination{Kind: DestinationSubmodule, Path: path.Join(partial, remaining)}, nil
-			}
-			if entry.Mode == "120000" {
-				state := partial + "\x00" + remaining
-				if _, cycle := seen[state]; cycle {
-					return Destination{Kind: DestinationSymlinkCycle, Path: partial}, nil
-				}
-				seen[state] = struct{}{}
-				linkText, err := tree.ReadFile(ctx, partial)
-				if err != nil {
-					return Destination{}, fmt.Errorf("read committed symlink %q: %w", partial, err)
-				}
-				target := string(linkText)
-				if strings.HasPrefix(target, "/") {
-					return Destination{Kind: DestinationOutsideCheckout, Path: target}, nil
-				}
-				resolved = path.Clean(path.Join(path.Dir(partial), target, remaining))
-				if outsideCheckout(resolved) {
-					return Destination{Kind: DestinationOutsideCheckout, Path: resolved}, nil
-				}
-				restarted = true
-				break
-			}
-			if remaining != "" {
+	components := strings.Split(resolved, "/")
+	partial := ""
+	for index, component := range components {
+		if err := ctx.Err(); err != nil {
+			return Destination{}, err
+		}
+		partial = path.Join(partial, component)
+		remaining := strings.Join(components[index+1:], "/")
+		entry, exists := tree.Entry(partial)
+		if !exists {
+			if _, directory := tree.directories[partial]; !directory {
 				return Destination{Kind: DestinationMissing, Path: resolved}, nil
 			}
-			return Destination{Kind: DestinationEntry, Path: partial}, nil
-		}
-		if restarted {
 			continue
 		}
-		if _, directory := tree.directories[resolved]; directory {
-			return Destination{Kind: DestinationDirectory, Path: resolved}, nil
+		if entry.Mode == "160000" {
+			return Destination{Kind: DestinationSubmodule, Path: path.Join(partial, remaining)}, nil
 		}
-		return Destination{Kind: DestinationMissing, Path: resolved}, nil
+		if entry.Mode == "120000" {
+			if active[partial] {
+				return Destination{Kind: DestinationSymlinkCycle, Path: partial}, nil
+			}
+			linkText, err := tree.ReadFile(ctx, partial)
+			if err != nil {
+				return Destination{}, fmt.Errorf("read committed symlink %q: %w", partial, err)
+			}
+			target := string(linkText)
+			if strings.HasPrefix(target, "/") {
+				return Destination{Kind: DestinationOutsideCheckout, Path: target}, nil
+			}
+			active[partial] = true
+			destination, err := tree.resolveDestination(ctx, path.Join(path.Dir(partial), target), active)
+			delete(active, partial)
+			if err != nil || remaining == "" || destination.Kind != DestinationDirectory {
+				if err == nil && remaining != "" && destination.Kind == DestinationEntry {
+					return Destination{Kind: DestinationMissing, Path: path.Join(destination.Path, remaining)}, nil
+				}
+				return destination, err
+			}
+			return tree.resolveDestination(ctx, path.Join(destination.Path, remaining), active)
+		}
+		if remaining != "" {
+			return Destination{Kind: DestinationMissing, Path: resolved}, nil
+		}
+		return Destination{Kind: DestinationEntry, Path: partial}, nil
 	}
+	if _, directory := tree.directories[resolved]; directory {
+		return Destination{Kind: DestinationDirectory, Path: resolved}, nil
+	}
+	return Destination{Kind: DestinationMissing, Path: resolved}, nil
 }

 func outsideCheckout(name string) bool {