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 <[email protected]>

Author: delvh

modules/fileicon/entry.go

@@ -6,17 +6,17 @@ package fileicon
 import "code.gitea.io/gitea/modules/git"
 
 type EntryInfo struct {
-	FullName      string
+	BaseName      string
 	EntryMode     git.EntryMode
 	SymlinkToMode git.EntryMode
 	IsOpen        bool
 }
 
-func EntryInfoFromGitTreeEntry(gitEntry *git.TreeEntry) *EntryInfo {
-	ret := &EntryInfo{FullName: gitEntry.Name(), EntryMode: gitEntry.Mode()}
+func EntryInfoFromGitTreeEntry(commit *git.Commit, fullPath string, gitEntry *git.TreeEntry) *EntryInfo {
+	ret := &EntryInfo{BaseName: gitEntry.Name(), EntryMode: gitEntry.Mode()}
 	if gitEntry.IsLink() {
-		if te, err := gitEntry.FollowLink(); err == nil && te.IsDir() {
-			ret.SymlinkToMode = te.Mode()
+		if res, err := git.EntryFollowLink(commit, fullPath, gitEntry); err == nil && res.TargetEntry.IsDir() {
+			ret.SymlinkToMode = res.TargetEntry.Mode()
 		}
 	}
 	return ret

modules/fileicon/material.go

@@ -5,7 +5,6 @@ package fileicon
 
 import (
 	"html/template"
-	"path"
 	"strings"
 	"sync"
 
@@ -134,7 +133,7 @@ func (m *MaterialIconProvider) FindIconName(entry *EntryInfo) string {
 		return "folder-git"
 	}
 
-	fileNameLower := strings.ToLower(path.Base(entry.FullName))
+	fileNameLower := strings.ToLower(entry.BaseName)
 	if entry.EntryMode.IsDir() {
 		if s, ok := m.rules.FolderNames[fileNameLower]; ok {
 			return s

modules/fileicon/material_test.go

@@ -20,8 +20,8 @@ func TestMain(m *testing.M) {
 func TestFindIconName(t *testing.T) {
 	unittest.PrepareTestEnv(t)
 	p := fileicon.DefaultMaterialIconProvider()
-	assert.Equal(t, "php", p.FindIconName(&fileicon.EntryInfo{FullName: "foo.php", EntryMode: git.EntryModeBlob}))
-	assert.Equal(t, "php", p.FindIconName(&fileicon.EntryInfo{FullName: "foo.PHP", EntryMode: git.EntryModeBlob}))
-	assert.Equal(t, "javascript", p.FindIconName(&fileicon.EntryInfo{FullName: "foo.js", EntryMode: git.EntryModeBlob}))
-	assert.Equal(t, "visualstudio", p.FindIconName(&fileicon.EntryInfo{FullName: "foo.vba", EntryMode: git.EntryModeBlob}))
+	assert.Equal(t, "php", p.FindIconName(&fileicon.EntryInfo{BaseName: "foo.php", EntryMode: git.EntryModeBlob}))
+	assert.Equal(t, "php", p.FindIconName(&fileicon.EntryInfo{BaseName: "foo.PHP", EntryMode: git.EntryModeBlob}))
+	assert.Equal(t, "javascript", p.FindIconName(&fileicon.EntryInfo{BaseName: "foo.js", EntryMode: git.EntryModeBlob}))
+	assert.Equal(t, "visualstudio", p.FindIconName(&fileicon.EntryInfo{BaseName: "foo.vba", EntryMode: git.EntryModeBlob}))
 }

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) {
+type EntryFollowResult struct {
+	SymlinkContent string
+	TargetFullPath string
+	TargetEntry    *TreeEntry
+}
+
+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)
 	}
 
-	// read the link
-	r, err := te.Blob().DataAsync()
-	if err != nil {
-		return nil, err
+	// 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)
 	}
-	closed := false
-	defer func() {
-		if !closed {
-			_ = r.Close()
-		}
-	}()
-	buf := make([]byte, te.Size())
-	_, err = io.ReadFull(r, buf)
+
+	link, err := te.Blob().GetBlobContent(maxSymlinkSize)
 	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 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)
 	}
 
-	if t == nil {
-		return nil, ErrSymlinkUnresolved{te.Name(), "points outside of repo"}
-	}
-
-	target, err := t.GetTreeEntryByPath(lnk)
+	targetFullPath := path.Join(path.Dir(fullPath), link)
+	targetEntry, err := commit.GetTreeEntryByPath(targetFullPath)
 	if err != nil {
-		if IsErrNotExist(err) {
-			return nil, ErrSymlinkUnresolved{te.Name(), "broken link"}
-		}
-		return nil, err
+		return &EntryFollowResult{SymlinkContent: link}, err
 	}
-	return target, nil
+	return &EntryFollowResult{SymlinkContent: link, TargetFullPath: targetFullPath, TargetEntry: targetEntry}, nil
 }
 
-// FollowLinks returns the entry ultimately pointed to by a symlink
-func (te *TreeEntry) FollowLinks(optLimit ...int) (*TreeEntry, error) {
-	if !te.IsLink() {
-		return nil, ErrSymlinkUnresolved{te.Name(), "not a symlink"}
-	}
+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() {
-			break
-		}
-		next, err := entry.FollowLink()
+		res, err = EntryFollowLink(commit, fullPath, treeEntry)
 		if err != nil {
-			return nil, err
+			return res, err
 		}
-		if next.ID == entry.ID {
-			return nil, ErrSymlinkUnresolved{entry.Name(), "recursive link"}
+		treeEntry, fullPath = res.TargetEntry, res.TargetFullPath
+		if !treeEntry.IsLink() {
+			break
 		}
-		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
 }