Skip to content

Commit 1740415

Browse files
philip-petersonphilip-peterson
authored
Merge branch 'main' into peterson/unused-param
2 parents 84d18d3 + b0936f4 commit 1740415

File tree

20 files changed

+339
-301
lines changed

20 files changed

+339
-301
lines changed

‎models/user/search.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func (opts *SearchUserOptions) toSearchQueryBase(ctx context.Context) *xorm.Sess
137137

138138
// SearchUsers takes options i.e. keyword and part of user name to search,
139139
// it returns results in given range and number of total results.
140-
func SearchUsers(ctx context.Context, opts *SearchUserOptions) (users []*User, _ int64, _ error) {
140+
func SearchUsers(ctx context.Context, opts SearchUserOptions) (users []*User, _ int64, _ error) {
141141
sessCount := opts.toSearchQueryBase(ctx)
142142
defer sessCount.Close()
143143
count, err := sessCount.Count(new(User))
@@ -152,7 +152,7 @@ func SearchUsers(ctx context.Context, opts *SearchUserOptions) (users []*User, _
152152
sessQuery := opts.toSearchQueryBase(ctx).OrderBy(opts.OrderBy.String())
153153
defer sessQuery.Close()
154154
if opts.Page > 0 {
155-
sessQuery = db.SetSessionPagination(sessQuery, opts)
155+
sessQuery = db.SetSessionPagination(sessQuery, &opts)
156156
}
157157

158158
// the sql may contain JOIN, so we must only select User related columns

‎models/user/user_test.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func TestCanCreateOrganization(t *testing.T) {
8888

8989
func TestSearchUsers(t *testing.T) {
9090
assert.NoError(t, unittest.PrepareTestDatabase())
91-
testSuccess := func(opts *user_model.SearchUserOptions, expectedUserOrOrgIDs []int64) {
91+
testSuccess := func(opts user_model.SearchUserOptions, expectedUserOrOrgIDs []int64) {
9292
users, _, err := user_model.SearchUsers(db.DefaultContext, opts)
9393
assert.NoError(t, err)
9494
cassText := fmt.Sprintf("ids: %v, opts: %v", expectedUserOrOrgIDs, opts)
@@ -100,61 +100,61 @@ func TestSearchUsers(t *testing.T) {
100100
}
101101

102102
// test orgs
103-
testOrgSuccess := func(opts *user_model.SearchUserOptions, expectedOrgIDs []int64) {
103+
testOrgSuccess := func(opts user_model.SearchUserOptions, expectedOrgIDs []int64) {
104104
opts.Type = user_model.UserTypeOrganization
105105
testSuccess(opts, expectedOrgIDs)
106106
}
107107

108-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1, PageSize: 2}},
108+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1, PageSize: 2}},
109109
[]int64{3, 6})
110110

111-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 2, PageSize: 2}},
111+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 2, PageSize: 2}},
112112
[]int64{7, 17})
113113

114-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 3, PageSize: 2}},
114+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 3, PageSize: 2}},
115115
[]int64{19, 25})
116116

117-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 4, PageSize: 2}},
117+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 4, PageSize: 2}},
118118
[]int64{26, 41})
119119

120-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 5, PageSize: 2}},
120+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 5, PageSize: 2}},
121121
[]int64{42})
122122

123-
testOrgSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 6, PageSize: 2}},
123+
testOrgSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 6, PageSize: 2}},
124124
[]int64{})
125125

126126
// test users
127-
testUserSuccess := func(opts *user_model.SearchUserOptions, expectedUserIDs []int64) {
127+
testUserSuccess := func(opts user_model.SearchUserOptions, expectedUserIDs []int64) {
128128
opts.Type = user_model.UserTypeIndividual
129129
testSuccess(opts, expectedUserIDs)
130130
}
131131

132-
testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
132+
testUserSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
133133
[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
134134

135-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(false)},
135+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(false)},
136136
[]int64{9})
137137

138-
testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
138+
testUserSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
139139
[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
140140

141-
testUserSuccess(&user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
141+
testUserSuccess(user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
142142
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
143143

144144
// order by name asc default
145-
testUserSuccess(&user_model.SearchUserOptions{Keyword: "user1", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
145+
testUserSuccess(user_model.SearchUserOptions{Keyword: "user1", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
146146
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
147147

148-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsAdmin: optional.Some(true)},
148+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsAdmin: optional.Some(true)},
149149
[]int64{1})
150150

151-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsRestricted: optional.Some(true)},
151+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsRestricted: optional.Some(true)},
152152
[]int64{29})
153153

154-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: optional.Some(true)},
154+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: optional.Some(true)},
155155
[]int64{37})
156156

157-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: optional.Some(true)},
157+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: optional.Some(true)},
158158
[]int64{24})
159159
}
160160

‎modules/git/commit.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type Commit struct {
3434
// CommitSignature represents a git commit signature part.
3535
type CommitSignature struct {
3636
Signature string
37-
Payload string // TODO check if can be reconstruct from the rest of commit information to not have duplicate data
37+
Payload string
3838
}
3939

4040
// Message returns the commit message. Same as retrieving CommitMessage directly.

‎modules/git/commit_reader.go‎

Lines changed: 61 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,44 @@ package git
66
import (
77
"bufio"
88
"bytes"
9+
"fmt"
910
"io"
10-
"strings"
1111
)
1212

13+
const (
14+
commitHeaderGpgsig = "gpgsig"
15+
commitHeaderGpgsigSha256 = "gpgsig-sha256"
16+
)
17+
18+
func assignCommitFields(gitRepo *Repository, commit *Commit, headerKey string, headerValue []byte) error {
19+
if len(headerValue) > 0 && headerValue[len(headerValue)-1] == '\n' {
20+
headerValue = headerValue[:len(headerValue)-1] // remove trailing newline
21+
}
22+
switch headerKey {
23+
case "tree":
24+
objID, err := NewIDFromString(string(headerValue))
25+
if err != nil {
26+
return fmt.Errorf("invalid tree ID %q: %w", string(headerValue), err)
27+
}
28+
commit.Tree = *NewTree(gitRepo, objID)
29+
case "parent":
30+
objID, err := NewIDFromString(string(headerValue))
31+
if err != nil {
32+
return fmt.Errorf("invalid parent ID %q: %w", string(headerValue), err)
33+
}
34+
commit.Parents = append(commit.Parents, objID)
35+
case "author":
36+
commit.Author.Decode(headerValue)
37+
case "committer":
38+
commit.Committer.Decode(headerValue)
39+
case commitHeaderGpgsig, commitHeaderGpgsigSha256:
40+
// if there are duplicate "gpgsig" and "gpgsig-sha256" headers, then the signature must have already been invalid
41+
// so we don't need to handle duplicate headers here
42+
commit.Signature = &CommitSignature{Signature: string(headerValue)}
43+
}
44+
return nil
45+
}
46+
1347
// CommitFromReader will generate a Commit from a provided reader
1448
// We need this to interpret commits from cat-file or cat-file --batch
1549
//
@@ -21,90 +55,46 @@ func CommitFromReader(gitRepo *Repository, objectID ObjectID, reader io.Reader)
2155
Committer: &Signature{},
2256
}
2357

24-
payloadSB := new(strings.Builder)
25-
signatureSB := new(strings.Builder)
26-
messageSB := new(strings.Builder)
27-
message := false
28-
pgpsig := false
29-
30-
bufReader, ok := reader.(*bufio.Reader)
31-
if !ok {
32-
bufReader = bufio.NewReader(reader)
33-
}
34-
35-
readLoop:
58+
bufReader := bufio.NewReader(reader)
59+
inHeader := true
60+
var payloadSB, messageSB bytes.Buffer
61+
var headerKey string
62+
var headerValue []byte
3663
for {
3764
line, err := bufReader.ReadBytes('\n')
38-
if err != nil {
39-
if err == io.EOF {
40-
if message {
41-
_, _ = messageSB.Write(line)
42-
}
43-
_, _ = payloadSB.Write(line)
44-
break readLoop
45-
}
46-
return nil, err
65+
if err != nil && err != io.EOF {
66+
return nil, fmt.Errorf("unable to read commit %q: %w", objectID.String(), err)
4767
}
48-
if pgpsig {
49-
if len(line) > 0 && line[0] == ' ' {
50-
_, _ = signatureSB.Write(line[1:])
51-
continue
52-
}
53-
pgpsig = false
68+
if len(line) == 0 {
69+
break
5470
}
5571

56-
if !message {
57-
// This is probably not correct but is copied from go-gits interpretation...
58-
trimmed := bytes.TrimSpace(line)
59-
if len(trimmed) == 0 {
60-
message = true
61-
_, _ = payloadSB.Write(line)
62-
continue
63-
}
64-
65-
split := bytes.SplitN(trimmed, []byte{' '}, 2)
66-
var data []byte
67-
if len(split) > 1 {
68-
data = split[1]
72+
if inHeader {
73+
inHeader = !(len(line) == 1 && line[0] == '\n') // still in header if line is not just a newline
74+
k, v, _ := bytes.Cut(line, []byte{' '})
75+
if len(k) != 0 || !inHeader {
76+
if headerKey != "" {
77+
if err = assignCommitFields(gitRepo, commit, headerKey, headerValue); err != nil {
78+
return nil, fmt.Errorf("unable to parse commit %q: %w", objectID.String(), err)
79+
}
80+
}
81+
headerKey = string(k) // it also resets the headerValue to empty string if not inHeader
82+
headerValue = v
83+
} else {
84+
headerValue = append(headerValue, v...)
6985
}
70-
71-
switch string(split[0]) {
72-
case "tree":
73-
commit.Tree = *NewTree(gitRepo, MustIDFromString(string(data)))
86+
if headerKey != commitHeaderGpgsig && headerKey != commitHeaderGpgsigSha256 {
7487
_, _ = payloadSB.Write(line)
75-
case "parent":
76-
commit.Parents = append(commit.Parents, MustIDFromString(string(data)))
77-
_, _ = payloadSB.Write(line)
78-
case "author":
79-
commit.Author = &Signature{}
80-
commit.Author.Decode(data)
81-
_, _ = payloadSB.Write(line)
82-
case "committer":
83-
commit.Committer = &Signature{}
84-
commit.Committer.Decode(data)
85-
_, _ = payloadSB.Write(line)
86-
case "encoding":
87-
_, _ = payloadSB.Write(line)
88-
case "gpgsig":
89-
fallthrough
90-
case "gpgsig-sha256": // FIXME: no intertop, so only 1 exists at present.
91-
_, _ = signatureSB.Write(data)
92-
_ = signatureSB.WriteByte('\n')
93-
pgpsig = true
9488
}
9589
} else {
9690
_, _ = messageSB.Write(line)
9791
_, _ = payloadSB.Write(line)
9892
}
9993
}
94+
10095
commit.CommitMessage = messageSB.String()
101-
commit.Signature = &CommitSignature{
102-
Signature: signatureSB.String(),
103-
Payload: payloadSB.String(),
104-
}
105-
if len(commit.Signature.Signature) == 0 {
106-
commit.Signature = nil
96+
if commit.Signature != nil {
97+
commit.Signature.Payload = payloadSB.String()
10798
}
108-
10999
return commit, nil
110100
}

‎modules/git/commit_sha256_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ func TestGetFullCommitIDErrorSha256(t *testing.T) {
6060
}
6161

6262
func TestCommitFromReaderSha256(t *testing.T) {
63-
commitString := `9433b2a62b964c17a4485ae180f45f595d3e69d31b786087775e28c6b6399df0 commit 1114
64-
tree e7f9e96dd79c09b078cac8b303a7d3b9d65ff9b734e86060a4d20409fd379f9e
63+
commitString := `tree e7f9e96dd79c09b078cac8b303a7d3b9d65ff9b734e86060a4d20409fd379f9e
6564
parent 26e9ccc29fad747e9c5d9f4c9ddeb7eff61cc45ef6a8dc258cbeb181afc055e8
6665
author Adam Majer <[email protected]> 1698676906 +0100
6766
committer Adam Majer <[email protected]> 1698676906 +0100
@@ -112,8 +111,7 @@ VAEUo6ecdDxSpyt2naeg9pKus/BRi7P6g4B1hkk/zZstUX/QP4IQuAJbXjkvsC+X
112111
HKRr3NlRM/DygzTyj0gN74uoa0goCIbyAQhiT42nm0cuhM7uN/W0ayrlZjGF1cbR
113112
8NCJUL2Nwj0ywKIavC99Ipkb8AsFwpVT6U6effs6
114113
=xybZ
115-
-----END PGP SIGNATURE-----
116-
`, commitFromReader.Signature.Signature)
114+
-----END PGP SIGNATURE-----`, commitFromReader.Signature.Signature)
117115
assert.Equal(t, `tree e7f9e96dd79c09b078cac8b303a7d3b9d65ff9b734e86060a4d20409fd379f9e
118116
parent 26e9ccc29fad747e9c5d9f4c9ddeb7eff61cc45ef6a8dc258cbeb181afc055e8
119117
author Adam Majer <[email protected]> 1698676906 +0100

‎modules/git/commit_test.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,7 @@ func TestGetFullCommitIDError(t *testing.T) {
5959
}
6060

6161
func TestCommitFromReader(t *testing.T) {
62-
commitString := `feaf4ba6bc635fec442f46ddd4512416ec43c2c2 commit 1074
63-
tree f1a6cb52b2d16773290cefe49ad0684b50a4f930
62+
commitString := `tree f1a6cb52b2d16773290cefe49ad0684b50a4f930
6463
parent 37991dec2c8e592043f47155ce4808d4580f9123
6564
author silverwind <[email protected]> 1563741793 +0200
6665
committer silverwind <[email protected]> 1563741793 +0200
@@ -108,8 +107,7 @@ sD53z/f0J+We4VZjY+pidvA9BGZPFVdR3wd3xGs8/oH6UWaLJAMGkLG6dDb3qDLm
108107
mfeFhT57UbE4qukTDIQ0Y0WM40UYRTakRaDY7ubhXgLgx09Cnp9XTVMsHgT6j9/i
109108
1pxsB104XLWjQHTjr1JtiaBQEwFh9r2OKTcpvaLcbNtYpo7CzOs=
110109
=FRsO
111-
-----END PGP SIGNATURE-----
112-
`, commitFromReader.Signature.Signature)
110+
-----END PGP SIGNATURE-----`, commitFromReader.Signature.Signature)
113111
assert.Equal(t, `tree f1a6cb52b2d16773290cefe49ad0684b50a4f930
114112
parent 37991dec2c8e592043f47155ce4808d4580f9123
115113
author silverwind <[email protected]> 1563741793 +0200
@@ -126,8 +124,7 @@ empty commit`, commitFromReader.Signature.Payload)
126124
}
127125

128126
func TestCommitWithEncodingFromReader(t *testing.T) {
129-
commitString := `feaf4ba6bc635fec442f46ddd4512416ec43c2c2 commit 1074
130-
tree ca3fad42080dd1a6d291b75acdfc46e5b9b307e5
127+
commitString := `tree ca3fad42080dd1a6d291b75acdfc46e5b9b307e5
131128
parent 47b24e7ab977ed31c5a39989d570847d6d0052af
132129
author KN4CK3R <[email protected]> 1711702962 +0100
133130
committer KN4CK3R <[email protected]> 1711702962 +0100
@@ -172,8 +169,7 @@ SONRzusmu5n3DgV956REL7x62h7JuqmBz/12HZkr0z0zgXkcZ04q08pSJATX5N1F
172169
yN+tWxTsWg+zhDk96d5Esdo9JMjcFvPv0eioo30GAERaz1hoD7zCMT4jgUFTQwgz
173170
jw4YcO5u
174171
=r3UU
175-
-----END PGP SIGNATURE-----
176-
`, commitFromReader.Signature.Signature)
172+
-----END PGP SIGNATURE-----`, commitFromReader.Signature.Signature)
177173
assert.Equal(t, `tree ca3fad42080dd1a6d291b75acdfc46e5b9b307e5
178174
parent 47b24e7ab977ed31c5a39989d570847d6d0052af
179175
author KN4CK3R <[email protected]> 1711702962 +0100

‎routers/api/v1/admin/org.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func GetAllOrgs(ctx *context.APIContext) {
101101

102102
listOptions := utils.GetListOptions(ctx)
103103

104-
users, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
104+
users, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
105105
Actor: ctx.Doer,
106106
Type: user_model.UserTypeOrganization,
107107
OrderBy: db.SearchOrderByAlphabetically,

‎routers/api/v1/admin/user.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ func SearchUsers(ctx *context.APIContext) {
423423

424424
listOptions := utils.GetListOptions(ctx)
425425

426-
users, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
426+
users, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
427427
Actor: ctx.Doer,
428428
Type: user_model.UserTypeIndividual,
429429
LoginName: ctx.FormTrim("login_name"),

‎routers/api/v1/org/org.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func GetAll(ctx *context.APIContext) {
201201

202202
listOptions := utils.GetListOptions(ctx)
203203

204-
publicOrgs, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
204+
publicOrgs, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
205205
Actor: ctx.Doer,
206206
ListOptions: listOptions,
207207
Type: user_model.UserTypeOrganization,

‎routers/api/v1/user/user.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func Search(ctx *context.APIContext) {
7373
if ctx.PublicOnly {
7474
visible = []structs.VisibleType{structs.VisibleTypePublic}
7575
}
76-
users, maxResults, err = user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
76+
users, maxResults, err = user_model.SearchUsers(ctx, user_model.SearchUserOptions{
7777
Actor: ctx.Doer,
7878
Keyword: ctx.FormTrim("q"),
7979
UID: uid,

0 commit comments

Comments
 (0)