From 9977df96d5b2967bc34716a4b3dcd34c287d80da Mon Sep 17 00:00:00 2001 From: Gabor Pihaj Date: Mon, 27 Apr 2026 23:54:21 +0200 Subject: [PATCH] fix: "Follow symlink" to work with arbitrary links (#12246) This change introduces a Path method on the TreeEntry struct, that collects the path by moving upwards in the tree. The existing FollowSymlink(s) methods interface has been changed, the previously returned string has been removed, as after the fix it wasn't used anywhere. Fixes: #9931 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12246 Reviewed-by: Gusted --- modules/base/tool.go | 2 +- modules/git/tree_entry.go | 65 ++++++++++++++++++++++++++-------- modules/git/tree_entry_test.go | 46 ++++++++++++++++++++++++ routers/web/repo/view.go | 11 +++--- tests/integration/repo_test.go | 2 +- 5 files changed, 105 insertions(+), 21 deletions(-) create mode 100644 modules/git/tree_entry_test.go diff --git a/modules/base/tool.go b/modules/base/tool.go index ada73df7da..1cc053ba58 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -103,7 +103,7 @@ func Int64sToStrings(ints []int64) []string { func EntryIcon(entry *git.TreeEntry) string { switch { case entry.IsLink(): - te, _, err := entry.FollowLink() + te, err := entry.FollowLink() if err != nil { log.Debug(err.Error()) return "file-symlink-file" diff --git a/modules/git/tree_entry.go b/modules/git/tree_entry.go index 5e3bb8ac21..0b3564178b 100644 --- a/modules/git/tree_entry.go +++ b/modules/git/tree_entry.go @@ -5,6 +5,7 @@ package git import ( + "fmt" "io" "sort" "strings" @@ -136,11 +137,11 @@ func (te *TreeEntry) LinkTarget() (string, error) { } // FollowLink returns the entry pointed to by a symlink -func (te *TreeEntry) FollowLink() (*TreeEntry, string, error) { +func (te *TreeEntry) FollowLink() (*TreeEntry, error) { // read the link lnk, err := te.LinkTarget() if err != nil { - return nil, "", err + return nil, err } t := te.ptree @@ -151,35 +152,33 @@ func (te *TreeEntry) FollowLink() (*TreeEntry, string, error) { } if t == nil { - return nil, "", ErrBadLink{te.Name(), "points outside of repo"} + return nil, ErrBadLink{te.Name(), "points outside of repo"} } target, err := t.GetTreeEntryByPath(lnk) if err != nil { if IsErrNotExist(err) { - return nil, "", ErrBadLink{te.Name(), "broken link"} + return nil, ErrBadLink{te.Name(), "broken link"} } - return nil, "", err + return nil, err } - return target, lnk, nil + return target, nil } // FollowLinks returns the entry ultimately pointed to by a symlink -func (te *TreeEntry) FollowLinks() (*TreeEntry, string, error) { +func (te *TreeEntry) FollowLinks() (*TreeEntry, error) { if !te.IsLink() { - return nil, "", ErrBadLink{te.Name(), "not a symlink"} + return nil, ErrBadLink{te.Name(), "not a symlink"} } entry := te - entryLink := "" for range 999 { if entry.IsLink() { - next, link, err := entry.FollowLink() - entryLink = link + next, err := entry.FollowLink() if err != nil { - return nil, "", err + return nil, err } if next.ID == entry.ID { - return nil, "", ErrBadLink{ + return nil, ErrBadLink{ entry.Name(), "recursive link", } @@ -190,12 +189,12 @@ func (te *TreeEntry) FollowLinks() (*TreeEntry, string, error) { } } if entry.IsLink() { - return nil, "", ErrBadLink{ + return nil, ErrBadLink{ te.Name(), "too many levels of symbolic links", } } - return entry, entryLink, nil + return entry, nil } // returns the Tree pointed to by this TreeEntry, or nil if this is not a tree @@ -208,6 +207,42 @@ func (te *TreeEntry) Tree() *Tree { return t } +// returns the calulcated path within the tree of this TreeEntry, or an error if it can be determined +func (te *TreeEntry) Path() (string, error) { + targetPath := te.Name() + parentTree := te.ptree + if parentTree == nil { + return "", fmt.Errorf("couldn't find the parent tree of the entry") + } + + prevID := parentTree.ID + parentTree = parentTree.ptree + for parentTree != nil { + entries, err := parentTree.ListEntries() + if err != nil { + return "", fmt.Errorf("couldn't list entries: %v", err) + } + + var matchingEntry *TreeEntry + for _, entry := range entries { + if entry.ID == prevID { + matchingEntry = entry + break + } + } + + if matchingEntry == nil { + return "", fmt.Errorf("this shouldn't happen: couldn't find entry (ID: %s) in tree (ID: %s)", prevID, parentTree.ID) + } + + targetPath = matchingEntry.name + "/" + targetPath + prevID = parentTree.ID + parentTree = parentTree.ptree + } + + return targetPath, nil +} + // GetSubJumpablePathName return the full path of subdirectory jumpable ( contains only one directory ) func (te *TreeEntry) GetSubJumpablePathName() string { if te.IsSubmodule() || !te.IsDir() { diff --git a/modules/git/tree_entry_test.go b/modules/git/tree_entry_test.go new file mode 100644 index 0000000000..325d3686f0 --- /dev/null +++ b/modules/git/tree_entry_test.go @@ -0,0 +1,46 @@ +// Copyright 2015 The Gogs Authors. All rights reserved. +// Copyright 2019 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git_test + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTreeEntry_Path(t *testing.T) { + repo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "templates_repo")) + require.NoError(t, err) + defer repo.Close() + + tests := []struct { + name string // description of this test case + path string + }{ + { + name: "Top level dir", + path: ".forgejo", + }, + { + name: "File in subdir", + path: ".forgejo/default_merge_message/MERGE_TEMPLATE.md", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tree, err := repo.GetTree("HEAD^{tree}") + require.NoError(t, err) + + te, err := tree.GetTreeEntryByPath(tt.path) + require.NoError(t, err) + + got, gotErr := te.Path() + require.NoError(t, gotErr, "Path() failed: %v", gotErr) + assert.Equal(t, tt.path, got, "Path() = %v, want %v", got, tt.path) + }) + } +} diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 287cd1b6aa..df0255279a 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -120,7 +120,7 @@ func FindReadmeFileInEntries(ctx *context.Context, entries []*git.TreeEntry, try log.Debug("Potential readme file: %s", entry.Name()) if readmeFiles[i] == nil || base.NaturalSortLess(readmeFiles[i].Name(), entry.Blob().Name()) { if entry.IsLink() { - target, _, err := entry.FollowLinks() + target, err := entry.FollowLinks() if err != nil && !git.IsErrBadLink(err) { return "", nil, err } else if target != nil && (target.IsExecutable() || target.IsRegular()) { @@ -270,7 +270,7 @@ func getFileReader(ctx gocontext.Context, repoID int64, blob *git.Blob) ([]byte, func renderReadmeFile(ctx *context.Context, subfolder string, readmeFile *git.TreeEntry) { target := readmeFile if readmeFile != nil && readmeFile.IsLink() { - target, _, _ = readmeFile.FollowLinks() + target, _ = readmeFile.FollowLinks() } if target == nil { // if findReadmeFile() failed and/or gave us a broken symlink (which it shouldn't) @@ -398,11 +398,14 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) { ctx.Data["OpenGraphNoDescription"] = true if entry.IsLink() { - _, link, err := entry.FollowLinks() + target, err := entry.FollowLinks() // Errors should be allowed, because this shouldn't // block rendering invalid symlink files. if err == nil { - ctx.Data["SymlinkURL"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() + "/" + util.PathEscapeSegments(link) + targetPath, err := target.Path() + if err == nil { + ctx.Data["SymlinkURL"] = ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL() + "/" + util.PathEscapeSegments(targetPath) + } } } diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go index 2f189bf7a8..299299ff0b 100644 --- a/tests/integration/repo_test.go +++ b/tests/integration/repo_test.go @@ -1079,7 +1079,7 @@ func TestRepoFollowSymlink(t *testing.T) { t.Run("Normal", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - assertCase(t, "/user2/readme-test/src/branch/symlink/up/back/down/down/README.md", "/user2/readme-test/src/branch/symlink/down/side/../left/right/../reelmein", true) + assertCase(t, "/user2/readme-test/src/branch/symlink/up/back/down/down/README.md", "/user2/readme-test/src/branch/symlink/up/down/left/reelmein", true) }) t.Run("Broken symlink", func(t *testing.T) {