Fix various bugs (1.24) (#35186)

Backport #35177, #35183

modules/git/commit_info.go

@@ -3,8 +3,6 @@
 
 package git
 
-import "path"
-
 // CommitInfo describes the first commit with the provided entry
 type CommitInfo struct {
 	Entry         *TreeEntry
@@ -12,15 +10,14 @@ type CommitInfo struct {
 	SubmoduleFile *CommitSubmoduleFile
 }
 
-func getCommitInfoSubmoduleFile(repoLink string, entry *TreeEntry, commit *Commit, treePathDir string) (*CommitSubmoduleFile, error) {
+func GetCommitInfoSubmoduleFile(repoLink, fullPath string, commit *Commit, refCommitID ObjectID) (*CommitSubmoduleFile, error) {
-	fullPath := path.Join(treePathDir, entry.Name())
 	submodule, err := commit.GetSubModule(fullPath)
 	if err != nil {
 		return nil, err
 	}
 	if submodule == nil {
 		// unable to find submodule from ".gitmodules" file
-		return NewCommitSubmoduleFile(repoLink, fullPath, "", entry.ID.String()), nil
+		return NewCommitSubmoduleFile(repoLink, fullPath, "", refCommitID.String()), nil
 	}
-	return NewCommitSubmoduleFile(repoLink, fullPath, submodule.URL, entry.ID.String()), nil
+	return NewCommitSubmoduleFile(repoLink, fullPath, submodule.URL, refCommitID.String()), nil
 }

modules/git/commit_info_gogit.go

@@ -73,7 +73,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, repoLink string, commit *
 
 		// If the entry is a submodule, add a submodule file for this
 		if entry.IsSubModule() {
-			commitsInfo[i].SubmoduleFile, err = getCommitInfoSubmoduleFile(repoLink, entry, commit, treePath)
+			commitsInfo[i].SubmoduleFile, err = GetCommitInfoSubmoduleFile(repoLink, path.Join(treePath, entry.Name()), commit, entry.ID)
 			if err != nil {
 				return nil, nil, err
 			}

modules/git/commit_info_nogogit.go

@@ -67,7 +67,7 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, repoLink string, commit *
 
 		// If the entry is a submodule, add a submodule file for this
 		if entry.IsSubModule() {
-			commitsInfo[i].SubmoduleFile, err = getCommitInfoSubmoduleFile(repoLink, entry, commit, treePath)
+			commitsInfo[i].SubmoduleFile, err = GetCommitInfoSubmoduleFile(repoLink, path.Join(treePath, entry.Name()), commit, entry.ID)
 			if err != nil {
 				return nil, nil, err
 			}

modules/git/commit_info_test.go

@@ -125,9 +125,9 @@ func TestEntries_GetCommitsInfo(t *testing.T) {
 	t.Run("NonExistingSubmoduleAsNil", func(t *testing.T) {
 		commit, err := bareRepo1.GetCommit("HEAD")
 		require.NoError(t, err)
-		tree, err := commit.GetTreeEntryByPath("file1.txt")
+		treeEntry, err := commit.GetTreeEntryByPath("file1.txt")
 		require.NoError(t, err)
-		cisf, err := getCommitInfoSubmoduleFile("/any/repo-link", tree, commit, "")
+		cisf, err := GetCommitInfoSubmoduleFile("/any/repo-link", "file1.txt", commit, treeEntry.ID)
 		require.NoError(t, err)
 		assert.Equal(t, &CommitSubmoduleFile{
 			repoLink: "/any/repo-link",

modules/git/commit_submodule_file.go

@@ -42,7 +42,7 @@ func (sf *CommitSubmoduleFile) getWebLinkInTargetRepo(ctx context.Context, moreL
 		return nil
 	}
 	if strings.HasPrefix(sf.refURL, "../") {
-		targetLink := path.Join(sf.repoLink, path.Dir(sf.fullPath), sf.refURL)
+		targetLink := path.Join(sf.repoLink, sf.refURL)
 		return &SubmoduleWebLink{RepoWebLink: targetLink, CommitWebLink: targetLink + moreLinkPath}
 	}
 	if !sf.parsed {

modules/git/commit_submodule_file_test.go

@@ -32,7 +32,7 @@ func TestCommitSubmoduleLink(t *testing.T) {
 		assert.Equal(t, "/subpath/user/repo", wl.RepoWebLink)
 		assert.Equal(t, "/subpath/user/repo/tree/aaaa", wl.CommitWebLink)
 
-		sf = NewCommitSubmoduleFile("/subpath/any/repo-home-link", "dir/submodule", "../../../user/repo", "aaaa")
+		sf = NewCommitSubmoduleFile("/subpath/any/repo-home-link", "dir/submodule", "../../user/repo", "aaaa")
 		wl = sf.SubmoduleWebLinkCompare(t.Context(), "1111", "2222")
 		assert.Equal(t, "/subpath/user/repo", wl.RepoWebLink)
 		assert.Equal(t, "/subpath/user/repo/compare/1111...2222", wl.CommitWebLink)

modules/git/utils.go

@@ -11,6 +11,8 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+
+	"code.gitea.io/gitea/modules/util"
 )
 
 // ObjectCache provides thread-safe cache operations.
@@ -106,3 +108,16 @@ func HashFilePathForWebUI(s string) string {
 	_, _ = h.Write([]byte(s))
 	return hex.EncodeToString(h.Sum(nil))
 }
+
+func SplitCommitTitleBody(commitMessage string, titleRuneLimit int) (title, body string) {
+	title, body, _ = strings.Cut(commitMessage, "\n")
+	title, title2 := util.EllipsisTruncateRunes(title, titleRuneLimit)
+	if title2 != "" {
+		if body == "" {
+			body = title2
+		} else {
+			body = title2 + "\n" + body
+		}
+	}
+	return title, body
+}

modules/git/utils_test.go

@@ -15,3 +15,17 @@ func TestHashFilePathForWebUI(t *testing.T) {
 		HashFilePathForWebUI("foobar"),
 	)
 }
+
+func TestSplitCommitTitleBody(t *testing.T) {
+	title, body := SplitCommitTitleBody("啊bcdefg", 4)
+	assert.Equal(t, "啊…", title)
+	assert.Equal(t, "…bcdefg", body)
+
+	title, body = SplitCommitTitleBody("abcdefg\n1234567", 4)
+	assert.Equal(t, "a…", title)
+	assert.Equal(t, "…bcdefg\n1234567", body)
+
+	title, body = SplitCommitTitleBody("abcdefg\n1234567", 100)
+	assert.Equal(t, "abcdefg", title)
+	assert.Equal(t, "1234567", body)
+}

modules/util/truncate.go

@@ -19,7 +19,7 @@ func IsLikelyEllipsisLeftPart(s string) bool {
 	return strings.HasSuffix(s, utf8Ellipsis) || strings.HasSuffix(s, asciiEllipsis)
 }
 
-func ellipsisGuessDisplayWidth(r rune) int {
+func ellipsisDisplayGuessWidth(r rune) int {
 	// To make the truncated string as long as possible,
 	// CJK/emoji chars are considered as 2-ASCII width but not 3-4 bytes width.
 	// Here we only make the best guess (better than counting them in bytes),
@@ -48,13 +48,17 @@ func ellipsisGuessDisplayWidth(r rune) int {
 // It appends "…" or "..." at the end of truncated string.
 // It guarantees the length of the returned runes doesn't exceed the limit.
 func EllipsisDisplayString(str string, limit int) string {
-	s, _, _, _ := ellipsisDisplayString(str, limit)
+	s, _, _, _ := ellipsisDisplayString(str, limit, ellipsisDisplayGuessWidth)
 	return s
 }
 
 // EllipsisDisplayStringX works like EllipsisDisplayString while it also returns the right part
 func EllipsisDisplayStringX(str string, limit int) (left, right string) {
-	left, offset, truncated, encounterInvalid := ellipsisDisplayString(str, limit)
+	return ellipsisDisplayStringX(str, limit, ellipsisDisplayGuessWidth)
+}
+
+func ellipsisDisplayStringX(str string, limit int, widthGuess func(rune) int) (left, right string) {
+	left, offset, truncated, encounterInvalid := ellipsisDisplayString(str, limit, widthGuess)
 	if truncated {
 		right = str[offset:]
 		r, _ := utf8.DecodeRune(UnsafeStringToBytes(right))
@@ -68,7 +72,7 @@ func EllipsisDisplayStringX(str string, limit int) (left, right string) {
 	return left, right
 }
 
-func ellipsisDisplayString(str string, limit int) (res string, offset int, truncated, encounterInvalid bool) {
+func ellipsisDisplayString(str string, limit int, widthGuess func(rune) int) (res string, offset int, truncated, encounterInvalid bool) {
 	if len(str) <= limit {
 		return str, len(str), false, false
 	}
@@ -81,7 +85,7 @@ func ellipsisDisplayString(str string, limit int) (res string, offset int, trunc
 	for i, r := range str {
 		encounterInvalid = encounterInvalid || r == utf8.RuneError
 		pos = i
-		runeWidth := ellipsisGuessDisplayWidth(r)
+		runeWidth := widthGuess(r)
 		if used+runeWidth+3 > limit {
 			break
 		}
@@ -96,7 +100,7 @@ func ellipsisDisplayString(str string, limit int) (res string, offset int, trunc
 			if nextCnt >= 4 {
 				break
 			}
-			nextWidth += ellipsisGuessDisplayWidth(r)
+			nextWidth += widthGuess(r)
 			nextCnt++
 		}
 		if nextCnt <= 3 && used+nextWidth <= limit {
@@ -114,6 +118,10 @@ func ellipsisDisplayString(str string, limit int) (res string, offset int, trunc
 	return str[:offset] + ellipsis, offset, true, encounterInvalid
 }
 
+func EllipsisTruncateRunes(str string, limit int) (left, right string) {
+	return ellipsisDisplayStringX(str, limit, func(r rune) int { return 1 })
+}
+
 // TruncateRunes returns a truncated string with given rune limit,
 // it returns input string if its rune length doesn't exceed the limit.
 func TruncateRunes(str string, limit int) string {

modules/util/truncate_test.go

@@ -29,7 +29,7 @@ func TestEllipsisGuessDisplayWidth(t *testing.T) {
 		t.Run(c.r, func(t *testing.T) {
 			w := 0
 			for _, r := range c.r {
-				w += ellipsisGuessDisplayWidth(r)
+				w += ellipsisDisplayGuessWidth(r)
 			}
 			assert.Equal(t, c.want, w, "hex=% x", []byte(c.r))
 		})

routers/web/feed/branch.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/services/context"
 
 	"github.com/gorilla/feeds"
@@ -15,10 +16,14 @@ import (
 
 // ShowBranchFeed shows tags and/or releases on the repo as RSS / Atom feed
 func ShowBranchFeed(ctx *context.Context, repo *repo.Repository, formatType string) {
-	commits, err := ctx.Repo.Commit.CommitsByRange(0, 10, "")
+	var commits []*git.Commit
-	if err != nil {
+	var err error
-		ctx.ServerError("ShowBranchFeed", err)
+	if ctx.Repo.Commit != nil {
-		return
+		commits, err = ctx.Repo.Commit.CommitsByRange(0, 10, "")
+		if err != nil {
+			ctx.ServerError("ShowBranchFeed", err)
+			return
+		}
 	}
 
 	title := "Latest commits for branch " + ctx.Repo.BranchName

routers/web/repo/view_home.go

@@ -20,8 +20,8 @@ import (
 	unit_model "code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
-	giturl "code.gitea.io/gitea/modules/git/url"
 	"code.gitea.io/gitea/modules/gitrepo"
+	"code.gitea.io/gitea/modules/htmlutil"
 	"code.gitea.io/gitea/modules/httplib"
 	"code.gitea.io/gitea/modules/log"
 	repo_module "code.gitea.io/gitea/modules/repository"
@@ -309,35 +309,41 @@ func handleRepoEmptyOrBroken(ctx *context.Context) {
 	ctx.Redirect(link)
 }
 
-func handleRepoViewSubmodule(ctx *context.Context, submodule *git.SubModule) {
+func isViewHomeOnlyContent(ctx *context.Context) bool {
-	// TODO: it needs to use git.NewCommitSubmoduleFile and SubmoduleWebLinkTree to correctly handle relative paths
+	return ctx.FormBool("only_content")
-	submoduleRepoURL, err := giturl.ParseRepositoryURL(ctx, submodule.URL)
+}
-	if err != nil {
+
-		HandleGitError(ctx, "handleRepoViewSubmodule: ParseRepositoryURL", err)
+func handleRepoViewSubmodule(ctx *context.Context, commitSubmoduleFile *git.CommitSubmoduleFile) {
+	submoduleWebLink := commitSubmoduleFile.SubmoduleWebLinkTree(ctx)
+	if submoduleWebLink == nil {
+		ctx.Data["NotFoundPrompt"] = ctx.Repo.TreePath
+		ctx.NotFound(nil)
 		return
 	}
-	submoduleURL := giturl.MakeRepositoryWebLink(submoduleRepoURL)
+
-	if httplib.IsCurrentGiteaSiteURL(ctx, submoduleURL) {
+	redirectLink := submoduleWebLink.CommitWebLink
-		ctx.RedirectToCurrentSite(submoduleURL)
+	if isViewHomeOnlyContent(ctx) {
-	} else {
+		ctx.Resp.Header().Set("Content-Type", "text/html; charset=utf-8")
+		_, _ = ctx.Resp.Write([]byte(htmlutil.HTMLFormat(`<a href="%s">%s</a>`, redirectLink, redirectLink)))
+	} else if !httplib.IsCurrentGiteaSiteURL(ctx, redirectLink) {
 		// don't auto-redirect to external URL, to avoid open redirect or phishing
-		ctx.Data["NotFoundPrompt"] = submoduleURL
+		ctx.Data["NotFoundPrompt"] = redirectLink
 		ctx.NotFound(nil)
+	} else {
+		ctx.Redirect(submoduleWebLink.CommitWebLink)
 	}
 }
 
 func prepareToRenderDirOrFile(entry *git.TreeEntry) func(ctx *context.Context) {
 	return func(ctx *context.Context) {
 		if entry.IsSubModule() {
-			submodule, err := ctx.Repo.Commit.GetSubModule(entry.Name())
+			commitSubmoduleFile, err := git.GetCommitInfoSubmoduleFile(ctx.Repo.RepoLink, ctx.Repo.TreePath, ctx.Repo.Commit, entry.ID)
 			if err != nil {
-				HandleGitError(ctx, "prepareToRenderDirOrFile: GetSubModule", err)
+				HandleGitError(ctx, "prepareToRenderDirOrFile: GetCommitInfoSubmoduleFile", err)
 				return
 			}
-			handleRepoViewSubmodule(ctx, submodule)
+			handleRepoViewSubmodule(ctx, commitSubmoduleFile)
-			return
+		} else if entry.IsDir() {
-		}
-		if entry.IsDir() {
 			prepareToRenderDirectory(ctx)
 		} else {
 			prepareToRenderFile(ctx, entry)
@@ -473,7 +479,7 @@ func Home(ctx *context.Context) {
 		}
 	}
 
-	if ctx.FormBool("only_content") {
+	if isViewHomeOnlyContent(ctx) {
 		ctx.HTML(http.StatusOK, tplRepoViewContent)
 	} else if len(treeNames) != 0 {
 		ctx.HTML(http.StatusOK, tplRepoView)

routers/web/repo/view_home_test.go

@@ -9,7 +9,6 @@ import (
 
 	"code.gitea.io/gitea/models/unittest"
 	git_module "code.gitea.io/gitea/modules/git"
-	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/services/contexttest"
 
 	"github.com/stretchr/testify/assert"
@@ -19,14 +18,20 @@ func TestViewHomeSubmoduleRedirect(t *testing.T) {
 	unittest.PrepareTestEnv(t)
 
 	ctx, _ := contexttest.MockContext(t, "/user2/repo1/src/branch/master/test-submodule")
-	submodule := &git_module.SubModule{Path: "test-submodule", URL: setting.AppURL + "user2/repo-other.git"}
+	submodule := git_module.NewCommitSubmoduleFile("/user2/repo1", "test-submodule", "../repo-other", "any-ref-id")
 	handleRepoViewSubmodule(ctx, submodule)
 	assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
-	assert.Equal(t, "/user2/repo-other", ctx.Resp.Header().Get("Location"))
+	assert.Equal(t, "/user2/repo-other/tree/any-ref-id", ctx.Resp.Header().Get("Location"))
 
 	ctx, _ = contexttest.MockContext(t, "/user2/repo1/src/branch/master/test-submodule")
-	submodule = &git_module.SubModule{Path: "test-submodule", URL: "https://other/user2/repo-other.git"}
+	submodule = git_module.NewCommitSubmoduleFile("/user2/repo1", "test-submodule", "https://other/user2/repo-other.git", "any-ref-id")
 	handleRepoViewSubmodule(ctx, submodule)
 	// do not auto-redirect for external URLs, to avoid open redirect or phishing
 	assert.Equal(t, http.StatusNotFound, ctx.Resp.WrittenStatus())
+
+	ctx, respWriter := contexttest.MockContext(t, "/user2/repo1/src/branch/master/test-submodule?only_content=true")
+	submodule = git_module.NewCommitSubmoduleFile("/user2/repo1", "test-submodule", "../repo-other", "any-ref-id")
+	handleRepoViewSubmodule(ctx, submodule)
+	assert.Equal(t, http.StatusOK, ctx.Resp.WrittenStatus())
+	assert.Equal(t, `<a href="/user2/repo-other/tree/any-ref-id">/user2/repo-other/tree/any-ref-id</a>`, respWriter.Body.String())
 }

services/repository/files/tree.go

@@ -90,15 +90,8 @@ func GetTreeBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git
 	if rangeStart >= len(entries) {
 		return tree, nil
 	}
-	var rangeEnd int
+	rangeEnd := min(rangeStart+perPage, len(entries))
-	if len(entries) > perPage {
+	tree.Truncated = rangeEnd < len(entries)
-		tree.Truncated = true
-	}
-	if rangeStart+perPage < len(entries) {
-		rangeEnd = rangeStart + perPage
-	} else {
-		rangeEnd = len(entries)
-	}
 	tree.Entries = make([]api.GitEntry, rangeEnd-rangeStart)
 	for e := rangeStart; e < rangeEnd; e++ {
 		i := e - rangeStart

services/repository/push.go

@@ -402,16 +402,11 @@ func pushUpdateAddTags(ctx context.Context, repo *repo_model.Repository, gitRepo
 		}
 
 		rel, has := relMap[lowerTag]
-
+		title, note := git.SplitCommitTitleBody(tag.Message, 255)
-		parts := strings.SplitN(tag.Message, "\n", 2)
-		note := ""
-		if len(parts) > 1 {
-			note = parts[1]
-		}
 		if !has {
 			rel = &repo_model.Release{
 				RepoID:       repo.ID,
-				Title:        parts[0],
+				Title:        title,
 				TagName:      tags[i],
 				LowerTagName: lowerTag,
 				Target:       "",
@@ -430,7 +425,7 @@ func pushUpdateAddTags(ctx context.Context, repo *repo_model.Repository, gitRepo
 			rel.Sha1 = commit.ID.String()
 			rel.CreatedUnix = timeutil.TimeStamp(createdAt.Unix())
 			if rel.IsTag {
-				rel.Title = parts[0]
+				rel.Title = title
 				rel.Note = note
 			} else {
 				rel.IsDraft = false

tests/integration/api_repo_git_trees_test.go

@@ -11,7 +11,11 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/tests"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestAPIReposGitTrees(t *testing.T) {
@@ -32,13 +36,21 @@ func TestAPIReposGitTrees(t *testing.T) {
 	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository)
 
 	// Test a public repo that anyone can GET the tree of
-	for _, ref := range [...]string{
+	_ = MakeRequest(t, NewRequest(t, "GET", "/api/v1/repos/user2/repo1/git/trees/master"), http.StatusOK)
-		"master",     // Branch
+
-		repo1TreeSHA, // Tree SHA
+	resp := MakeRequest(t, NewRequest(t, "GET", "/api/v1/repos/user2/repo1/git/trees/62fb502a7172d4453f0322a2cc85bddffa57f07a?per_page=1"), http.StatusOK)
-	} {
+	var respGitTree api.GitTreeResponse
-		req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/git/trees/%s", user2.Name, repo1.Name, ref)
+	DecodeJSON(t, resp, &respGitTree)
-		MakeRequest(t, req, http.StatusOK)
+	assert.True(t, respGitTree.Truncated)
-	}
+	require.Len(t, respGitTree.Entries, 1)
+	assert.Equal(t, "File-WoW", respGitTree.Entries[0].Path)
+
+	resp = MakeRequest(t, NewRequest(t, "GET", "/api/v1/repos/user2/repo1/git/trees/62fb502a7172d4453f0322a2cc85bddffa57f07a?page=2&per_page=1"), http.StatusOK)
+	respGitTree = api.GitTreeResponse{}
+	DecodeJSON(t, resp, &respGitTree)
+	assert.False(t, respGitTree.Truncated)
+	require.Len(t, respGitTree.Entries, 1)
+	assert.Equal(t, "README.md", respGitTree.Entries[0].Path)
 
 	// Tests a private repo with no token so will fail
 	for _, ref := range [...]string{

tests/integration/empty_repo_test.go

@@ -75,6 +75,11 @@ func TestEmptyRepoAddFile(t *testing.T) {
 	req = NewRequest(t, "GET", "/api/v1/repos/user30/empty/raw/main/README.md").AddTokenAuth(token)
 	session.MakeRequest(t, req, http.StatusNotFound)
 
+	// test feed
+	req = NewRequest(t, "GET", "/user30/empty/rss/branch/main/README.md").AddTokenAuth(token).SetHeader("Accept", "application/rss+xml")
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	assert.Contains(t, resp.Body.String(), "</rss>")
+
 	// create a new file
 	req = NewRequest(t, "GET", "/user30/empty/_new/"+setting.Repository.DefaultBranch)
 	resp = session.MakeRequest(t, req, http.StatusOK)