Follow file symlinks in the UI to their target

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
 }

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()
 }