Follow file symlinks in the UI to their target (#28835)

Symlinks are followed when you click on a link next to an entry, either
until a file has been found or until we know that the link is dead.
When the link cannot be accessed, we fall back to the current behavior
of showing the document containing the target.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

modules/git/commit.go

@@ -20,7 +20,8 @@ import (
 
 // Commit represents a git commit.
 type Commit struct {
-	Tree
+	Tree // FIXME: bad design, this field can be nil if the commit is from "last commit cache"
+
 	ID            ObjectID // The ID of this commit object
 	Author        *Signature
 	Committer     *Signature

modules/git/error.go

@@ -32,22 +32,6 @@ func (err ErrNotExist) Unwrap() error {
 	return util.ErrNotExist
 }
 
-// ErrSymlinkUnresolved entry.FollowLink error
-type ErrSymlinkUnresolved struct {
-	Name    string
-	Message string
-}
-
-func (err ErrSymlinkUnresolved) Error() string {
-	return fmt.Sprintf("%s: %s", err.Name, err.Message)
-}
-
-// IsErrSymlinkUnresolved if some error is ErrSymlinkUnresolved
-func IsErrSymlinkUnresolved(err error) bool {
-	_, ok := err.(ErrSymlinkUnresolved)
-	return ok
-}
-
 // ErrBranchNotExist represents a "BranchNotExist" kind of error.
 type ErrBranchNotExist struct {
 	Name string

modules/git/tree_blob_nogogit.go

@@ -11,7 +11,7 @@ import (
 )
 
 // GetTreeEntryByPath get the tree entries according the sub dir
-func (t *Tree) GetTreeEntryByPath(relpath string) (*TreeEntry, error) {
+func (t *Tree) GetTreeEntryByPath(relpath string) (_ *TreeEntry, err error) {
 	if len(relpath) == 0 {
 		return &TreeEntry{
 			ptree:     t,
@@ -21,27 +21,25 @@ func (t *Tree) GetTreeEntryByPath(relpath string) (*TreeEntry, error) {
 		}, nil
 	}
 
-	// FIXME: This should probably use git cat-file --batch to be a bit more efficient
 	relpath = path.Clean(relpath)
 	parts := strings.Split(relpath, "/")
-	var err error
+
 	tree := t
-	for i, name := range parts {
-		if i == len(parts)-1 {
-			entries, err := tree.ListEntries()
-			if err != nil {
-				return nil, err
-			}
-			for _, v := range entries {
-				if v.Name() == name {
-					return v, nil
-				}
-			}
-		} else {
-			tree, err = tree.SubTree(name)
-			if err != nil {
-				return nil, err
-			}
+	for _, name := range parts[:len(parts)-1] {
+		tree, err = tree.SubTree(name)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	name := parts[len(parts)-1]
+	entries, err := tree.ListEntries()
+	if err != nil {
+		return nil, err
+	}
+	for _, v := range entries {
+		if v.Name() == name {
+			return v, nil
 		}
 	}
 	return nil, ErrNotExist{"", relpath}

modules/git/tree_entry.go

@@ -5,7 +5,7 @@
 package git
 
 import (
-	"io"
+	"path"
 	"sort"
 	"strings"
 
@@ -24,77 +24,57 @@ func (te *TreeEntry) Type() string {
 	}
 }
 
-// FollowLink returns the entry pointed to by a symlink
-func (te *TreeEntry) FollowLink() (*TreeEntry, error) {
-	if !te.IsLink() {
-		return nil, ErrSymlinkUnresolved{te.Name(), "not a symlink"}
-	}
-
-	// read the link
-	r, err := te.Blob().DataAsync()
-	if err != nil {
-		return nil, err
-	}
-	closed := false
-	defer func() {
-		if !closed {
-			_ = r.Close()
-		}
-	}()
-	buf := make([]byte, te.Size())
-	_, err = io.ReadFull(r, buf)
-	if err != nil {
-		return nil, err
-	}
-	_ = r.Close()
-	closed = true
-
-	lnk := string(buf)
-	t := te.ptree
-
-	// traverse up directories
-	for ; t != nil && strings.HasPrefix(lnk, "../"); lnk = lnk[3:] {
-		t = t.ptree
-	}
-
-	if t == nil {
-		return nil, ErrSymlinkUnresolved{te.Name(), "points outside of repo"}
-	}
-
-	target, err := t.GetTreeEntryByPath(lnk)
-	if err != nil {
-		if IsErrNotExist(err) {
-			return nil, ErrSymlinkUnresolved{te.Name(), "broken link"}
-		}
-		return nil, err
-	}
-	return target, nil
+type EntryFollowResult struct {
+	SymlinkContent string
+	TargetFullPath string
+	TargetEntry    *TreeEntry
 }
 
-// FollowLinks returns the entry ultimately pointed to by a symlink
-func (te *TreeEntry) FollowLinks(optLimit ...int) (*TreeEntry, error) {
+func EntryFollowLink(commit *Commit, fullPath string, te *TreeEntry) (*EntryFollowResult, error) {
 	if !te.IsLink() {
-		return nil, ErrSymlinkUnresolved{te.Name(), "not a symlink"}
+		return nil, util.ErrorWrap(util.ErrUnprocessableContent, "%q is not a symlink", fullPath)
 	}
+
+	// git's filename max length is 4096, hopefully a link won't be longer than multiple of that
+	const maxSymlinkSize = 20 * 4096
+	if te.Blob().Size() > maxSymlinkSize {
+		return nil, util.ErrorWrap(util.ErrUnprocessableContent, "%q content exceeds symlink limit", fullPath)
+	}
+
+	link, err := te.Blob().GetBlobContent(maxSymlinkSize)
+	if err != nil {
+		return nil, err
+	}
+	if strings.HasPrefix(link, "/") {
+		// It's said that absolute path will be stored as is in Git
+		return &EntryFollowResult{SymlinkContent: link}, util.ErrorWrap(util.ErrUnprocessableContent, "%q is an absolute symlink", fullPath)
+	}
+
+	targetFullPath := path.Join(path.Dir(fullPath), link)
+	targetEntry, err := commit.GetTreeEntryByPath(targetFullPath)
+	if err != nil {
+		return &EntryFollowResult{SymlinkContent: link}, err
+	}
+	return &EntryFollowResult{SymlinkContent: link, TargetFullPath: targetFullPath, TargetEntry: targetEntry}, nil
+}
+
+func EntryFollowLinks(commit *Commit, firstFullPath string, firstTreeEntry *TreeEntry, optLimit ...int) (res *EntryFollowResult, err error) {
 	limit := util.OptionalArg(optLimit, 10)
-	entry := te
+	treeEntry, fullPath := firstTreeEntry, firstFullPath
 	for range limit {
-		if !entry.IsLink() {
+		res, err = EntryFollowLink(commit, fullPath, treeEntry)
+		if err != nil {
+			return res, err
+		}
+		treeEntry, fullPath = res.TargetEntry, res.TargetFullPath
+		if !treeEntry.IsLink() {
 			break
 		}
-		next, err := entry.FollowLink()
-		if err != nil {
-			return nil, err
-		}
-		if next.ID == entry.ID {
-			return nil, ErrSymlinkUnresolved{entry.Name(), "recursive link"}
-		}
-		entry = next
 	}
-	if entry.IsLink() {
-		return nil, ErrSymlinkUnresolved{te.Name(), "too many levels of symbolic links"}
+	if treeEntry.IsLink() {
+		return res, util.ErrorWrap(util.ErrUnprocessableContent, "%q has too many links", firstFullPath)
 	}
-	return entry, nil
+	return res, nil
 }
 
 // returns the Tree pointed to by this TreeEntry, or nil if this is not a tree

modules/git/tree_entry_common_test.go

@@ -0,0 +1,76 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package git
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/modules/util"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFollowLink(t *testing.T) {
+	r, err := openRepositoryWithDefaultContext("tests/repos/repo1_bare")
+	require.NoError(t, err)
+	defer r.Close()
+
+	commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123")
+	require.NoError(t, err)
+
+	// get the symlink
+	{
+		lnkFullPath := "foo/bar/link_to_hello"
+		lnk, err := commit.Tree.GetTreeEntryByPath("foo/bar/link_to_hello")
+		require.NoError(t, err)
+		assert.True(t, lnk.IsLink())
+
+		// should be able to dereference to target
+		res, err := EntryFollowLink(commit, lnkFullPath, lnk)
+		require.NoError(t, err)
+		assert.Equal(t, "hello", res.TargetEntry.Name())
+		assert.Equal(t, "foo/nar/hello", res.TargetFullPath)
+		assert.False(t, res.TargetEntry.IsLink())
+		assert.Equal(t, "b14df6442ea5a1b382985a6549b85d435376c351", res.TargetEntry.ID.String())
+	}
+
+	{
+		// should error when called on a normal file
+		entry, err := commit.Tree.GetTreeEntryByPath("file1.txt")
+		require.NoError(t, err)
+		res, err := EntryFollowLink(commit, "file1.txt", entry)
+		assert.ErrorIs(t, err, util.ErrUnprocessableContent)
+		assert.Nil(t, res)
+	}
+
+	{
+		// should error for broken links
+		entry, err := commit.Tree.GetTreeEntryByPath("foo/broken_link")
+		require.NoError(t, err)
+		assert.True(t, entry.IsLink())
+		res, err := EntryFollowLink(commit, "foo/broken_link", entry)
+		assert.ErrorIs(t, err, util.ErrNotExist)
+		assert.Equal(t, "nar/broken_link", res.SymlinkContent)
+	}
+
+	{
+		// should error for external links
+		entry, err := commit.Tree.GetTreeEntryByPath("foo/outside_repo")
+		require.NoError(t, err)
+		assert.True(t, entry.IsLink())
+		res, err := EntryFollowLink(commit, "foo/outside_repo", entry)
+		assert.ErrorIs(t, err, util.ErrNotExist)
+		assert.Equal(t, "../../outside_repo", res.SymlinkContent)
+	}
+
+	{
+		// testing fix for short link bug
+		entry, err := commit.Tree.GetTreeEntryByPath("foo/link_short")
+		require.NoError(t, err)
+		res, err := EntryFollowLink(commit, "foo/link_short", entry)
+		assert.ErrorIs(t, err, util.ErrNotExist)
+		assert.Equal(t, "a", res.SymlinkContent)
+	}
+}

modules/git/tree_entry_gogit.go

@@ -19,16 +19,12 @@ type TreeEntry struct {
 	gogitTreeEntry *object.TreeEntry
 	ptree          *Tree
 
-	size     int64
-	sized    bool
-	fullName string
+	size  int64
+	sized bool
 }
 
 // Name returns the name of the entry
 func (te *TreeEntry) Name() string {
-	if te.fullName != "" {
-		return te.fullName
-	}
 	return te.gogitTreeEntry.Name
 }
 
@@ -55,7 +51,7 @@ func (te *TreeEntry) Size() int64 {
 	return te.size
 }
 
-// IsSubModule if the entry is a sub module
+// IsSubModule if the entry is a submodule
 func (te *TreeEntry) IsSubModule() bool {
 	return te.gogitTreeEntry.Mode == filemode.Submodule
 }

modules/git/tree_entry_mode.go

@@ -15,7 +15,7 @@ type EntryMode int
 // one of these.
 const (
 	// EntryModeNoEntry is possible if the file was added or removed in a commit. In the case of
-	// added the base commit will not have the file in its tree so a mode of 0o000000 is used.
+	// when adding the base commit doesn't have the file in its tree, a mode of 0o000000 is used.
 	EntryModeNoEntry EntryMode = 0o000000
 
 	EntryModeBlob    EntryMode = 0o100644
@@ -30,7 +30,7 @@ func (e EntryMode) String() string {
 	return strconv.FormatInt(int64(e), 8)
 }
 
-// IsSubModule if the entry is a sub module
+// IsSubModule if the entry is a submodule
 func (e EntryMode) IsSubModule() bool {
 	return e == EntryModeCommit
 }

modules/git/tree_entry_nogogit.go

@@ -57,7 +57,7 @@ func (te *TreeEntry) Size() int64 {
 	return te.size
 }
 
-// IsSubModule if the entry is a sub module
+// IsSubModule if the entry is a submodule
 func (te *TreeEntry) IsSubModule() bool {
 	return te.entryMode.IsSubModule()
 }

modules/git/tree_entry_test.go

@@ -53,50 +53,3 @@ func TestEntriesCustomSort(t *testing.T) {
 	assert.Equal(t, "bcd", entries[6].Name())
 	assert.Equal(t, "abc", entries[7].Name())
 }
-
-func TestFollowLink(t *testing.T) {
-	r, err := openRepositoryWithDefaultContext("tests/repos/repo1_bare")
-	assert.NoError(t, err)
-	defer r.Close()
-
-	commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123")
-	assert.NoError(t, err)
-
-	// get the symlink
-	lnk, err := commit.Tree.GetTreeEntryByPath("foo/bar/link_to_hello")
-	assert.NoError(t, err)
-	assert.True(t, lnk.IsLink())
-
-	// should be able to dereference to target
-	target, err := lnk.FollowLink()
-	assert.NoError(t, err)
-	assert.Equal(t, "hello", target.Name())
-	assert.False(t, target.IsLink())
-	assert.Equal(t, "b14df6442ea5a1b382985a6549b85d435376c351", target.ID.String())
-
-	// should error when called on normal file
-	target, err = commit.Tree.GetTreeEntryByPath("file1.txt")
-	assert.NoError(t, err)
-	_, err = target.FollowLink()
-	assert.EqualError(t, err, "file1.txt: not a symlink")
-
-	// should error for broken links
-	target, err = commit.Tree.GetTreeEntryByPath("foo/broken_link")
-	assert.NoError(t, err)
-	assert.True(t, target.IsLink())
-	_, err = target.FollowLink()
-	assert.EqualError(t, err, "broken_link: broken link")
-
-	// should error for external links
-	target, err = commit.Tree.GetTreeEntryByPath("foo/outside_repo")
-	assert.NoError(t, err)
-	assert.True(t, target.IsLink())
-	_, err = target.FollowLink()
-	assert.EqualError(t, err, "outside_repo: points outside of repo")
-
-	// testing fix for short link bug
-	target, err = commit.Tree.GetTreeEntryByPath("foo/link_short")
-	assert.NoError(t, err)
-	_, err = target.FollowLink()
-	assert.EqualError(t, err, "link_short: broken link")
-}

modules/git/tree_gogit.go

@@ -69,7 +69,7 @@ func (t *Tree) ListEntriesRecursiveWithSize() (Entries, error) {
 	seen := map[plumbing.Hash]bool{}
 	walker := object.NewTreeWalker(t.gogitTree, true, seen)
 	for {
-		fullName, entry, err := walker.Next()
+		_, entry, err := walker.Next()
 		if err == io.EOF {
 			break
 		}
@@ -84,7 +84,6 @@ func (t *Tree) ListEntriesRecursiveWithSize() (Entries, error) {
 			ID:             ParseGogitHash(entry.Hash),
 			gogitTreeEntry: &entry,
 			ptree:          t,
-			fullName:       fullName,
 		}
 		entries = append(entries, convertedEntry)
 	}