Skip to content

Commit 094104b

Browse files
wxiaoguangwxiaoguang
authored
Make "commit statuses" API accept slashes in "ref" (#36264)
Fix #36253 Support slashes in `{ref}` (follow GitHub's behavior)
1 parent 1771569 commit 094104b

6 files changed

Lines changed: 99 additions & 150 deletions

File tree

‎routers/api/v1/api.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,19 +1384,19 @@ func Routes() *web.Router {
13841384
})
13851385
m.Get("/{base}/*", repo.GetPullRequestByBaseHead)
13861386
}, mustAllowPulls, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo())
1387-
m.Group("/statuses", func() {
1387+
m.Group("/statuses", func() { // "/statuses/{sha}" only accepts commit ID
13881388
m.Combo("/{sha}").Get(repo.GetCommitStatuses).
13891389
Post(reqToken(), reqRepoWriter(unit.TypeCode), bind(api.CreateStatusOption{}), repo.NewCommitStatus)
13901390
}, reqRepoReader(unit.TypeCode))
13911391
m.Group("/commits", func() {
13921392
m.Get("", context.ReferencesGitRepo(), repo.GetAllCommits)
1393-
m.Group("/{ref}", func() {
1394-
m.Get("/status", repo.GetCombinedCommitStatusByRef)
1395-
m.Get("/statuses", repo.GetCommitStatusesByRef)
1396-
}, context.ReferencesGitRepo())
1397-
m.Group("/{sha}", func() {
1398-
m.Get("/pull", repo.GetCommitPullRequest)
1399-
}, context.ReferencesGitRepo())
1393+
m.PathGroup("/*", func(g *web.RouterPathGroup) {
1394+
// Mis-configured reverse proxy might decode the `%2F` to slash ahead, so we need to support both formats (escaped, unescaped) here.
1395+
// It also matches GitHub's behavior
1396+
g.MatchPath("GET", "/<ref:*>/status", repo.GetCombinedCommitStatusByRef)
1397+
g.MatchPath("GET", "/<ref:*>/statuses", repo.GetCommitStatusesByRef)
1398+
g.MatchPath("GET", "/<sha>/pull", repo.GetCommitPullRequest)
1399+
})
14001400
}, reqRepoReader(unit.TypeCode))
14011401
m.Group("/git", func() {
14021402
m.Group("/commits", func() {

‎tests/integration/git_general_test.go‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -732,17 +732,8 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
732732

733733
commitID := path.Base(commitURL)
734734

735-
addCommitStatus := func(status commitstatus.CommitStatusState) func(*testing.T) {
736-
return doAPICreateCommitStatus(ctx, commitID, api.CreateStatusOption{
737-
State: status,
738-
TargetURL: "http://test.ci/",
739-
Description: "",
740-
Context: "testci",
741-
})
742-
}
743-
744735
// Call API to add Pending status for commit
745-
t.Run("CreateStatus", addCommitStatus(commitstatus.CommitStatusPending))
736+
t.Run("CreateStatus", doAPICreateCommitStatusTest(ctx, commitID, commitstatus.CommitStatusPending, "testci"))
746737

747738
// Cancel not existing auto merge
748739
ctx.ExpectedCode = http.StatusNotFound
@@ -771,16 +762,15 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
771762
assert.False(t, pr.HasMerged)
772763

773764
// Call API to add Failure status for commit
774-
t.Run("CreateStatus", addCommitStatus(commitstatus.CommitStatusFailure))
765+
t.Run("CreateStatus", doAPICreateCommitStatusTest(ctx, commitID, commitstatus.CommitStatusFailure, "testci"))
775766

776767
// Check pr status
777768
pr, err = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
778769
assert.NoError(t, err)
779770
assert.False(t, pr.HasMerged)
780771

781772
// Call API to add Success status for commit
782-
t.Run("CreateStatus", addCommitStatus(commitstatus.CommitStatusSuccess))
783-
773+
t.Run("CreateStatus", doAPICreateCommitStatusTest(ctx, commitID, commitstatus.CommitStatusSuccess, "testci"))
784774
// wait to let gitea merge stuff
785775
time.Sleep(time.Second)
786776

‎tests/integration/integration_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ func NewRequestWithBody(t testing.TB, method, urlStr string, body io.Reader) *Re
332332
return &RequestWrapper{req}
333333
}
334334

335-
const NoExpectedStatus = -1
335+
const NoExpectedStatus = 0
336336

337337
func MakeRequest(t testing.TB, rw *RequestWrapper, expectedStatus int) *httptest.ResponseRecorder {
338338
t.Helper()

‎tests/integration/pull_status_test.go‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,7 @@ func TestPullCreate_CommitStatus(t *testing.T) {
7676
// Update commit status, and check if icon is updated as well
7777
for _, status := range statusList {
7878
// Call API to add status for commit
79-
t.Run("CreateStatus", doAPICreateCommitStatus(testCtx, commitID, api.CreateStatusOption{
80-
State: status,
81-
TargetURL: "http://test.ci/",
82-
Description: "",
83-
Context: "testci",
84-
}))
79+
t.Run("CreateStatus", doAPICreateCommitStatusTest(testCtx, commitID, status, "testci"))
8580

8681
req = NewRequest(t, "GET", "/user1/repo1/pulls/1/commits")
8782
resp = session.MakeRequest(t, req, http.StatusOK)
@@ -103,19 +98,15 @@ func TestPullCreate_CommitStatus(t *testing.T) {
10398
})
10499
}
105100

106-
func doAPICreateCommitStatus(ctx APITestContext, commitID string, data api.CreateStatusOption) func(*testing.T) {
101+
func doAPICreateCommitStatusTest(ctx APITestContext, ref string, state commitstatus.CommitStatusState, statusContext string) func(*testing.T) {
107102
return func(t *testing.T) {
108-
req := NewRequestWithJSON(
109-
t,
110-
http.MethodPost,
111-
fmt.Sprintf("/api/v1/repos/%s/%s/statuses/%s", ctx.Username, ctx.Reponame, commitID),
112-
data,
113-
).AddTokenAuth(ctx.Token)
114-
if ctx.ExpectedCode != 0 {
115-
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
116-
return
117-
}
118-
ctx.Session.MakeRequest(t, req, http.StatusCreated)
103+
link := fmt.Sprintf("/api/v1/repos/%s/%s/statuses/%s", ctx.Username, ctx.Reponame, url.PathEscape(ref))
104+
req := NewRequestWithJSON(t, http.MethodPost, link, api.CreateStatusOption{
105+
State: state,
106+
TargetURL: "http://test.ci/",
107+
Context: statusContext,
108+
}).AddTokenAuth(ctx.Token)
109+
ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
119110
}
120111
}
121112

‎tests/integration/repo_commits_test.go‎

Lines changed: 77 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@ package integration
66
import (
77
"fmt"
88
"net/http"
9-
"net/http/httptest"
109
"path"
1110
"sync"
1211
"testing"
1312

1413
auth_model "code.gitea.io/gitea/models/auth"
14+
"code.gitea.io/gitea/models/db"
15+
git_model "code.gitea.io/gitea/models/git"
1516
"code.gitea.io/gitea/modules/commitstatus"
1617
"code.gitea.io/gitea/modules/json"
1718
"code.gitea.io/gitea/modules/setting"
@@ -20,6 +21,7 @@ import (
2021

2122
"github.com/PuerkitoBio/goquery"
2223
"github.com/stretchr/testify/assert"
24+
"github.com/stretchr/testify/require"
2325
)
2426

2527
func TestRepoCommits(t *testing.T) {
@@ -79,98 +81,87 @@ func TestRepoCommits(t *testing.T) {
7981
})
8082
}
8183

82-
func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {
84+
func TestRepoCommitsWithStatus(t *testing.T) {
8385
defer tests.PrepareTestEnv(t)()
8486

8587
session := loginUser(t, "user2")
86-
87-
// Request repository commits page
88-
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
89-
resp := session.MakeRequest(t, req, http.StatusOK)
90-
91-
doc := NewHTMLParser(t, resp.Body)
92-
// Get first commit URL
93-
commitURL, exists := doc.doc.Find("#commits-table .commit-id-short").Attr("href")
94-
assert.True(t, exists)
95-
assert.NotEmpty(t, commitURL)
96-
97-
// Call API to add status for commit
9888
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository)
99-
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
100-
State: commitstatus.CommitStatusState(state),
101-
TargetURL: "http://test.ci/",
102-
Description: "",
103-
Context: "testci",
104-
}))
105-
106-
req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
107-
resp = session.MakeRequest(t, req, http.StatusOK)
108-
109-
doc = NewHTMLParser(t, resp.Body)
110-
// Check if commit status is displayed in message column (.tippy-target to ignore the tippy trigger)
111-
sel := doc.doc.Find("#commits-table .message .tippy-target .commit-status")
112-
assert.Equal(t, 1, sel.Length())
113-
for _, class := range classes {
114-
assert.True(t, sel.HasClass(class))
115-
}
116-
117-
// By SHA
118-
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)+"/statuses")
119-
reqOne := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)+"/status")
120-
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)
121-
122-
// By short SHA
123-
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)[:10]+"/statuses")
124-
reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)[:10]+"/status")
125-
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)
126-
127-
// By Ref
128-
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/statuses")
129-
reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/status")
130-
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)
131-
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/v1.1/statuses")
132-
reqOne = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/v1.1/status")
133-
testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), session.MakeRequest(t, reqOne, http.StatusOK), state)
134-
}
13589

136-
func testRepoCommitsWithStatus(t *testing.T, resp, respOne *httptest.ResponseRecorder, state string) {
137-
var statuses []*api.CommitStatus
138-
assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), &statuses))
139-
var status api.CombinedStatus
140-
assert.NoError(t, json.Unmarshal(respOne.Body.Bytes(), &status))
141-
assert.NotNil(t, status)
142-
143-
if assert.Len(t, statuses, 1) {
144-
assert.Equal(t, commitstatus.CommitStatusState(state), statuses[0].State)
145-
assert.Equal(t, setting.AppURL+"api/v1/repos/user2/repo1/statuses/65f1bf27bc3bf70f64657658635e66094edbcb4d", statuses[0].URL)
146-
assert.Equal(t, "http://test.ci/", statuses[0].TargetURL)
147-
assert.Empty(t, statuses[0].Description)
148-
assert.Equal(t, "testci", statuses[0].Context)
149-
150-
assert.Len(t, status.Statuses, 1)
151-
assert.Equal(t, statuses[0], status.Statuses[0])
152-
assert.Equal(t, "65f1bf27bc3bf70f64657658635e66094edbcb4d", status.SHA)
90+
requestCommitStatuses := func(t *testing.T, linkList, linkCombined string) (statuses []*api.CommitStatus, status api.CombinedStatus) {
91+
assert.NoError(t, json.Unmarshal(session.MakeRequest(t, NewRequest(t, "GET", linkList), http.StatusOK).Body.Bytes(), &statuses))
92+
assert.NoError(t, json.Unmarshal(session.MakeRequest(t, NewRequest(t, "GET", linkCombined), http.StatusOK).Body.Bytes(), &status))
93+
return statuses, status
15394
}
154-
}
155-
156-
func TestRepoCommitsWithStatusPending(t *testing.T) {
157-
doTestRepoCommitWithStatus(t, "pending", "octicon-dot-fill", "yellow")
158-
}
15995

160-
func TestRepoCommitsWithStatusSuccess(t *testing.T) {
161-
doTestRepoCommitWithStatus(t, "success", "octicon-check", "green")
162-
}
96+
testRefMaster := func(t *testing.T, state commitstatus.CommitStatusState, classes ...string) {
97+
_ = db.TruncateBeans(t.Context(), &git_model.CommitStatus{})
16398

164-
func TestRepoCommitsWithStatusError(t *testing.T) {
165-
doTestRepoCommitWithStatus(t, "error", "gitea-exclamation", "red")
166-
}
99+
// Request repository commits page
100+
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
101+
resp := session.MakeRequest(t, req, http.StatusOK)
167102

168-
func TestRepoCommitsWithStatusFailure(t *testing.T) {
169-
doTestRepoCommitWithStatus(t, "failure", "octicon-x", "red")
170-
}
103+
doc := NewHTMLParser(t, resp.Body)
104+
// Get first commit URL
105+
commitURL, _ := doc.doc.Find("#commits-table .commit-id-short").Attr("href")
106+
require.NotEmpty(t, commitURL)
107+
commitID := path.Base(commitURL)
108+
109+
// Call API to add status for commit
110+
doAPICreateCommitStatusTest(ctx, path.Base(commitURL), state, "testci")(t)
111+
112+
req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
113+
resp = session.MakeRequest(t, req, http.StatusOK)
114+
115+
doc = NewHTMLParser(t, resp.Body)
116+
// Check if commit status is displayed in message column (.tippy-target to ignore the tippy trigger)
117+
sel := doc.doc.Find("#commits-table .message .tippy-target .commit-status")
118+
assert.Equal(t, 1, sel.Length())
119+
for _, class := range classes {
120+
assert.True(t, sel.HasClass(class))
121+
}
122+
123+
testRepoCommitsWithStatus := func(t *testing.T, linkList, linkCombined string, state commitstatus.CommitStatusState) {
124+
statuses, status := requestCommitStatuses(t, linkList, linkCombined)
125+
require.Len(t, statuses, 1)
126+
require.NotNil(t, status)
127+
128+
assert.Equal(t, state, statuses[0].State)
129+
assert.Equal(t, setting.AppURL+"api/v1/repos/user2/repo1/statuses/"+commitID, statuses[0].URL)
130+
assert.Equal(t, "http://test.ci/", statuses[0].TargetURL)
131+
assert.Empty(t, statuses[0].Description)
132+
assert.Equal(t, "testci", statuses[0].Context)
133+
134+
assert.Len(t, status.Statuses, 1)
135+
assert.Equal(t, statuses[0], status.Statuses[0])
136+
assert.Equal(t, commitID, status.SHA)
137+
}
138+
// By SHA
139+
testRepoCommitsWithStatus(t, "/api/v1/repos/user2/repo1/commits/"+commitID+"/statuses", "/api/v1/repos/user2/repo1/commits/"+commitID+"/status", state)
140+
// By short SHA
141+
testRepoCommitsWithStatus(t, "/api/v1/repos/user2/repo1/commits/"+commitID[:7]+"/statuses", "/api/v1/repos/user2/repo1/commits/"+commitID[:7]+"/status", state)
142+
// By Ref
143+
testRepoCommitsWithStatus(t, "/api/v1/repos/user2/repo1/commits/master/statuses", "/api/v1/repos/user2/repo1/commits/master/status", state)
144+
// Tag "v1.1" points to master
145+
testRepoCommitsWithStatus(t, "/api/v1/repos/user2/repo1/commits/v1.1/statuses", "/api/v1/repos/user2/repo1/commits/v1.1/status", state)
146+
}
171147

172-
func TestRepoCommitsWithStatusWarning(t *testing.T) {
173-
doTestRepoCommitWithStatus(t, "warning", "gitea-exclamation", "yellow")
148+
t.Run("pending", func(t *testing.T) { testRefMaster(t, "pending", "octicon-dot-fill", "yellow") })
149+
t.Run("success", func(t *testing.T) { testRefMaster(t, "success", "octicon-check", "green") })
150+
t.Run("error", func(t *testing.T) { testRefMaster(t, "error", "gitea-exclamation", "red") })
151+
t.Run("failure", func(t *testing.T) { testRefMaster(t, "failure", "octicon-x", "red") })
152+
t.Run("warning", func(t *testing.T) { testRefMaster(t, "warning", "gitea-exclamation", "yellow") })
153+
t.Run("BranchWithSlash", func(t *testing.T) {
154+
_ = db.TruncateBeans(t.Context(), &git_model.CommitStatus{})
155+
156+
linkList, linkCombined := "/api/v1/repos/user2/repo1/commits/feature%2F1/statuses", "/api/v1/repos/user2/repo1/commits/feature/1/status"
157+
statuses, status := requestCommitStatuses(t, linkList, linkCombined)
158+
assert.Empty(t, statuses)
159+
assert.Empty(t, status.Statuses)
160+
doAPICreateCommitStatusTest(ctx, "feature/1", commitstatus.CommitStatusSuccess, "testci")(t)
161+
statuses, status = requestCommitStatuses(t, linkList, linkCombined)
162+
assert.NotEmpty(t, statuses)
163+
assert.NotEmpty(t, status.Statuses)
164+
})
174165
}
175166

176167
func TestRepoCommitsStatusParallel(t *testing.T) {
@@ -194,13 +185,7 @@ func TestRepoCommitsStatusParallel(t *testing.T) {
194185
go func(parentT *testing.T, i int) {
195186
parentT.Run(fmt.Sprintf("ParallelCreateStatus_%d", i), func(t *testing.T) {
196187
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository)
197-
runBody := doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
198-
State: commitstatus.CommitStatusPending,
199-
TargetURL: "http://test.ci/",
200-
Description: "",
201-
Context: "testci",
202-
})
203-
runBody(t)
188+
doAPICreateCommitStatusTest(ctx, path.Base(commitURL), commitstatus.CommitStatusPending, "testci")(t)
204189
wg.Done()
205190
})
206191
}(t, i)
@@ -225,20 +210,8 @@ func TestRepoCommitsStatusMultiple(t *testing.T) {
225210

226211
// Call API to add status for commit
227212
ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository)
228-
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
229-
State: commitstatus.CommitStatusSuccess,
230-
TargetURL: "http://test.ci/",
231-
Description: "",
232-
Context: "testci",
233-
}))
234-
235-
t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{
236-
State: commitstatus.CommitStatusSuccess,
237-
TargetURL: "http://test.ci/",
238-
Description: "",
239-
Context: "other_context",
240-
}))
241-
213+
t.Run("CreateStatus", doAPICreateCommitStatusTest(ctx, path.Base(commitURL), commitstatus.CommitStatusSuccess, "testci"))
214+
t.Run("CreateStatus", doAPICreateCommitStatusTest(ctx, path.Base(commitURL), commitstatus.CommitStatusSuccess, "other_context"))
242215
req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
243216
resp = session.MakeRequest(t, req, http.StatusOK)
244217

‎tests/integration/repo_webhook_test.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -930,12 +930,7 @@ func Test_WebhookStatus(t *testing.T) {
930930
testCtx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeAll)
931931

932932
// update a status for a commit via API
933-
doAPICreateCommitStatus(testCtx, commitID, api.CreateStatusOption{
934-
State: commitstatus.CommitStatusSuccess,
935-
TargetURL: "http://test.ci/",
936-
Description: "",
937-
Context: "testci",
938-
})(t)
933+
doAPICreateCommitStatusTest(testCtx, commitID, commitstatus.CommitStatusSuccess, "testci")(t)
939934

940935
// 3. validate the webhook is triggered
941936
assert.Equal(t, "status", triggeredEvent)

0 commit comments

Comments
 (0)