Skip to content

Commit 789a3d3

Browse files
GiteaBotMohit25022005wxiaoguang
authored
fix(api): handle fork-only commits in compare API (#37185) (#37199)
Backport #37185 by @Mohit25022005 Fix 500 error when comparing branches across fork repositories ## Problem The compare API returns a 500 Internal Server Error when comparing branches where the head commit exists only in the fork repository. ## Cause The API was using the base repository's GitRepo and repository context when converting commits. This fails when the commit does not exist in the base repository, resulting in a "fatal: bad object" error. ## Solution Use the head repository and HeadGitRepo when available to ensure commits are resolved in the correct repository context. ## Result * Fixes "fatal: bad object" error * Enables proper comparison between base and fork repositories * Prevents 500 Internal Server Error Fixes #37168 Co-authored-by: Mohit Swarnkar <mohitswarnkar13@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent b37e098 commit 789a3d3

2 files changed

Lines changed: 45 additions & 24 deletions

File tree

routers/api/v1/repo/compare.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,20 @@ func CompareDiff(ctx *context.APIContext) {
6262

6363
apiCommits := make([]*api.Commit, 0, len(compareInfo.Commits))
6464
userCache := make(map[string]*user_model.User)
65+
6566
for i := 0; i < len(compareInfo.Commits); i++ {
66-
apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, compareInfo.Commits[i], userCache,
67+
apiCommit, err := convert.ToCommit(
68+
ctx,
69+
compareInfo.HeadRepo,
70+
compareInfo.HeadGitRepo,
71+
compareInfo.Commits[i],
72+
userCache,
6773
convert.ToCommitOptions{
6874
Stat: true,
6975
Verification: verification,
7076
Files: files,
71-
})
77+
},
78+
)
7279
if err != nil {
7380
ctx.APIErrorInternal(err)
7481
return

tests/integration/api_repo_compare_test.go

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,46 +5,60 @@ package integration
55

66
import (
77
"net/http"
8+
"net/url"
89
"testing"
910

1011
auth_model "code.gitea.io/gitea/models/auth"
12+
repo_model "code.gitea.io/gitea/models/repo"
1113
"code.gitea.io/gitea/models/unittest"
1214
user_model "code.gitea.io/gitea/models/user"
1315
api "code.gitea.io/gitea/modules/structs"
1416
"code.gitea.io/gitea/tests"
1517

1618
"github.com/stretchr/testify/assert"
19+
"github.com/stretchr/testify/require"
1720
)
1821

1922
func TestAPICompareBranches(t *testing.T) {
20-
defer tests.PrepareTestEnv(t)()
23+
onGiteaRun(t, func(t *testing.T, _ *url.URL) {
24+
session2 := loginUser(t, "user2")
25+
token2 := getTokenForLoggedInUser(t, session2, auth_model.AccessTokenScopeWriteRepository)
2126

22-
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
23-
// Login as User2.
24-
session := loginUser(t, user.Name)
25-
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
27+
t.Run("CompareBranches", func(t *testing.T) {
28+
defer tests.PrintCurrentTest(t)()
2629

27-
t.Run("CompareBranches", func(t *testing.T) {
28-
defer tests.PrintCurrentTest(t)()
29-
req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b").AddTokenAuth(token)
30-
resp := MakeRequest(t, req, http.StatusOK)
30+
req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b").AddTokenAuth(token2)
31+
resp := MakeRequest(t, req, http.StatusOK)
32+
apiResp := DecodeJSON(t, resp, &api.Compare{})
33+
assert.Equal(t, 2, apiResp.TotalCommits)
34+
assert.Len(t, apiResp.Commits, 2)
35+
})
3136

32-
var apiResp *api.Compare
33-
DecodeJSON(t, resp, &apiResp)
37+
t.Run("CompareCommits", func(t *testing.T) {
38+
defer tests.PrintCurrentTest(t)()
3439

35-
assert.Equal(t, 2, apiResp.TotalCommits)
36-
assert.Len(t, apiResp.Commits, 2)
37-
})
40+
req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/808038d2f71b0ab02099...c8e31bc7688741a5287f").AddTokenAuth(token2)
41+
resp := MakeRequest(t, req, http.StatusOK)
42+
apiResp := DecodeJSON(t, resp, &api.Compare{})
43+
assert.Equal(t, 1, apiResp.TotalCommits)
44+
assert.Len(t, apiResp.Commits, 1)
45+
})
3846

39-
t.Run("CompareCommits", func(t *testing.T) {
40-
defer tests.PrintCurrentTest(t)()
41-
req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/808038d2f71b0ab02099...c8e31bc7688741a5287f").AddTokenAuth(token)
42-
resp := MakeRequest(t, req, http.StatusOK)
47+
t.Run("CompareForkOnlyCommit", func(t *testing.T) {
48+
defer tests.PrintCurrentTest(t)()
4349

44-
var apiResp *api.Compare
45-
DecodeJSON(t, resp, &apiResp)
50+
user13 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 13})
51+
repo11 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 11})
52+
user13Sess := loginUser(t, "user13")
53+
user13Token := getTokenForLoggedInUser(t, user13Sess, auth_model.AccessTokenScopeWriteRepository)
4654

47-
assert.Equal(t, 1, apiResp.TotalCommits)
48-
assert.Len(t, apiResp.Commits, 1)
55+
_, err := createFileInBranch(user13, repo11, createFileInBranchOptions{OldBranch: "master", NewBranch: "new-branch"}, map[string]string{"file.txt": "content"})
56+
require.NoError(t, err)
57+
req := NewRequestf(t, "GET", "/api/v1/repos/user12/repo10/compare/master...user13:new-branch").AddTokenAuth(user13Token)
58+
resp := MakeRequest(t, req, http.StatusOK)
59+
apiResp := DecodeJSON(t, resp, &api.Compare{})
60+
assert.Equal(t, 1, apiResp.TotalCommits)
61+
assert.Len(t, apiResp.Commits, 1)
62+
})
4963
})
5064
}

0 commit comments

Comments
 (0)