Skip to content

Commit eec8ee0

Browse files
authored
Fix some trivial problems (#36336)
1. correctly parse git protocol's "OldCommit NewCommit RefName" line, it should be explicitly split by space 2. add missing "return" in CreatePullRequest 3. add comments for "/user.keys" and "/user.gpg" outputs 4. trim space for the "commit status context name" to follow the same behavior of git_model.NewCommitStatus
1 parent f6d3c70 commit eec8ee0

6 files changed

Lines changed: 50 additions & 20 deletions

File tree

cmd/hook.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,14 @@ func (n *nilWriter) WriteString(s string) (int, error) {
163163
return len(s), nil
164164
}
165165

166+
func parseGitHookCommitRefLine(line string) (oldCommitID, newCommitID string, refFullName git.RefName, ok bool) {
167+
fields := strings.Split(line, " ")
168+
if len(fields) != 3 {
169+
return "", "", "", false
170+
}
171+
return fields[0], fields[1], git.RefName(fields[2]), true
172+
}
173+
166174
func runHookPreReceive(ctx context.Context, c *cli.Command) error {
167175
if isInternal, _ := strconv.ParseBool(os.Getenv(repo_module.EnvIsInternal)); isInternal {
168176
return nil
@@ -228,14 +236,11 @@ Gitea or set your environment appropriately.`, "")
228236
continue
229237
}
230238

231-
fields := bytes.Fields(scanner.Bytes())
232-
if len(fields) != 3 {
239+
oldCommitID, newCommitID, refFullName, ok := parseGitHookCommitRefLine(scanner.Text())
240+
if !ok {
233241
continue
234242
}
235243

236-
oldCommitID := string(fields[0])
237-
newCommitID := string(fields[1])
238-
refFullName := git.RefName(fields[2])
239244
total++
240245
lastline++
241246

@@ -378,16 +383,13 @@ Gitea or set your environment appropriately.`, "")
378383
continue
379384
}
380385

381-
fields := bytes.Fields(scanner.Bytes())
382-
if len(fields) != 3 {
386+
var ok bool
387+
oldCommitIDs[count], newCommitIDs[count], refFullNames[count], ok = parseGitHookCommitRefLine(scanner.Text())
388+
if !ok {
383389
continue
384390
}
385391

386392
fmt.Fprintf(out, ".")
387-
oldCommitIDs[count] = string(fields[0])
388-
newCommitIDs[count] = string(fields[1])
389-
refFullNames[count] = git.RefName(fields[2])
390-
391393
commitID, _ := git.NewIDFromString(newCommitIDs[count])
392394
if refFullNames[count] == git.BranchPrefix+"master" && !commitID.IsZero() && count == total {
393395
masterPushed = true

cmd/hook_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,17 @@ func TestPktLine(t *testing.T) {
3939
assert.NoError(t, err)
4040
assert.Equal(t, []byte("0007a\nb"), w.Bytes())
4141
}
42+
43+
func TestParseGitHookCommitRefLine(t *testing.T) {
44+
oldCommitID, newCommitID, refName, ok := parseGitHookCommitRefLine("a b c")
45+
assert.True(t, ok)
46+
assert.Equal(t, "a", oldCommitID)
47+
assert.Equal(t, "b", newCommitID)
48+
assert.Equal(t, "c", string(refName))
49+
50+
_, _, _, ok = parseGitHookCommitRefLine("a\tb\tc")
51+
assert.False(t, ok)
52+
53+
_, _, _, ok = parseGitHookCommitRefLine("a b")
54+
assert.False(t, ok)
55+
}

routers/api/v1/repo/pull.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,7 @@ func CreatePullRequest(ctx *context.APIContext) {
500500
unitPullRequest, err := ctx.Repo.Repository.GetUnit(ctx, unit.TypePullRequests)
501501
if err != nil {
502502
ctx.APIErrorInternal(err)
503+
return
503504
}
504505

505506
prIssue := &issues_model.Issue{

routers/web/user/home.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,8 @@ func ShowSSHKeys(ctx *context.Context) {
660660
}
661661

662662
var buf bytes.Buffer
663+
// "authorized_keys" file format: "#" followed by comment line per key
664+
buf.WriteString("# Gitea isn't a key server. The keys are exported as the user uploaded and might not have been fully verified.\n")
663665
for i := range keys {
664666
buf.WriteString(keys[i].OmitEmail())
665667
buf.WriteString("\n")
@@ -695,6 +697,8 @@ func ShowGPGKeys(ctx *context.Context) {
695697
var buf bytes.Buffer
696698

697699
headers := make(map[string]string)
700+
// https://www.rfc-editor.org/rfc/rfc4880
701+
headers["Comment"] = "Gitea isn't a key server. The keys are exported as the user uploaded and might not have been fully verified."
698702
if len(failedEntitiesID) > 0 { // If some key need re-import to be exported
699703
headers["Note"] = "The keys with the following IDs couldn't be exported and need to be reuploaded " + strings.Join(failedEntitiesID, ", ")
700704
} else if len(entities) == 0 {

services/actions/commit_status.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"path"
1111
"strconv"
12+
"strings"
1213

1314
actions_model "code.gitea.io/gitea/models/actions"
1415
"code.gitea.io/gitea/models/db"
@@ -129,6 +130,7 @@ func createCommitStatus(ctx context.Context, repo *repo_model.Repository, event,
129130
runName = wfs[0].Name
130131
}
131132
ctxName := fmt.Sprintf("%s / %s (%s)", runName, job.Name, event)
133+
ctxName = strings.TrimSpace(ctxName) // git_model.NewCommitStatus also trims spaces
132134
state := toCommitStatus(job.Status)
133135
if statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commitID, db.ListOptionsAll); err == nil {
134136
for _, v := range statuses {

tests/integration/user_test.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ func TestViewUser(t *testing.T) {
2727

2828
req := NewRequest(t, "GET", "/user2")
2929
MakeRequest(t, req, http.StatusOK)
30+
31+
req = NewRequest(t, "GET", "/user2.keys")
32+
resp := MakeRequest(t, req, http.StatusOK)
33+
assert.Equal(t, `# Gitea isn't a key server. The keys are exported as the user uploaded and might not have been fully verified.
34+
ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM=
35+
`, resp.Body.String())
3036
}
3137

3238
func TestRenameUsername(t *testing.T) {
@@ -194,8 +200,17 @@ func TestRenameReservedUsername(t *testing.T) {
194200

195201
func TestExportUserGPGKeys(t *testing.T) {
196202
defer tests.PrepareTestEnv(t)()
203+
testExportUserGPGKeys := func(t *testing.T, user, expected string) {
204+
session := loginUser(t, user)
205+
t.Logf("Testing username %s export gpg keys", user)
206+
req := NewRequest(t, "GET", "/"+user+".gpg")
207+
resp := session.MakeRequest(t, req, http.StatusOK)
208+
assert.Equal(t, expected, resp.Body.String())
209+
}
210+
197211
// Export empty key list
198212
testExportUserGPGKeys(t, "user1", `-----BEGIN PGP PUBLIC KEY BLOCK-----
213+
Comment: Gitea isn't a key server. The keys are exported as the user uploaded and might not have been fully verified.
199214
Note: This user hasn't uploaded any GPG keys.
200215
201216
@@ -237,6 +252,7 @@ GrE0MHOxUbc9tbtyk0F1SuzREUBH
237252
-----END PGP PUBLIC KEY BLOCK-----`)
238253
// Export new key
239254
testExportUserGPGKeys(t, "user1", `-----BEGIN PGP PUBLIC KEY BLOCK-----
255+
Comment: Gitea isn't a key server. The keys are exported as the user uploaded and might not have been fully verified.
240256
241257
xsBNBFyy/VUBCADJ7zbM20Z1RWmFoVgp5WkQfI2rU1Vj9cQHes9i42wVLLtcbPeo
242258
QzubgzvMPITDy7nfWxgSf83E23DoHQ1ACFbQh/6eFSRrjsusp3YQ/08NSfPPbcu8
@@ -268,15 +284,6 @@ GrE0MHOxUbc9tbtyk0F1SuzREUBH
268284
-----END PGP PUBLIC KEY BLOCK-----`)
269285
}
270286

271-
func testExportUserGPGKeys(t *testing.T, user, expected string) {
272-
session := loginUser(t, user)
273-
t.Logf("Testing username %s export gpg keys", user)
274-
req := NewRequest(t, "GET", "/"+user+".gpg")
275-
resp := session.MakeRequest(t, req, http.StatusOK)
276-
// t.Log(resp.Body.String())
277-
assert.Equal(t, expected, resp.Body.String())
278-
}
279-
280287
func TestGetUserRss(t *testing.T) {
281288
defer tests.PrepareTestEnv(t)()
282289

0 commit comments

Comments
 (0)