Skip to content

Commit 0db554f

Browse files
wxiaoguangwxiaoguang
authored
Refactor commit signature parser (go-gitea#30228)
To make it more flexible and support SSH signature. The existing tests are not changed, there are also tests covering `parseTagRef` which also calls `parsePayloadSignature` now. Add some new tests to `Test_parseTagData`
1 parent ca297a9 commit 0db554f

File tree

7 files changed

+134
-98
lines changed

7 files changed

+134
-98
lines changed

‎modules/git/commit.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@ type Commit struct {
2626
Author *Signature
2727
Committer *Signature
2828
CommitMessage string
29-
Signature *CommitGPGSignature
29+
Signature *CommitSignature
3030

3131
Parents []ObjectID // ID strings
3232
submoduleCache *ObjectCache
3333
}
3434

35-
// CommitGPGSignature represents a git commit signature part.
36-
type CommitGPGSignature struct {
35+
// CommitSignature represents a git commit signature part.
36+
type CommitSignature struct {
3737
Signature string
3838
Payload string // TODO check if can be reconstruct from the rest of commit information to not have duplicate data
3939
}

‎modules/git/commit_convert_gogit.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
"github.com/go-git/go-git/v5/plumbing/object"
1414
)
1515

16-
func convertPGPSignature(c *object.Commit) *CommitGPGSignature {
16+
func convertPGPSignature(c *object.Commit) *CommitSignature {
1717
if c.PGPSignature == "" {
1818
return nil
1919
}
@@ -57,7 +57,7 @@ func convertPGPSignature(c *object.Commit) *CommitGPGSignature {
5757
return nil
5858
}
5959

60-
return &CommitGPGSignature{
60+
return &CommitSignature{
6161
Signature: c.PGPSignature,
6262
Payload: w.String(),
6363
}

‎modules/git/commit_reader.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ readLoop:
9999
}
100100
}
101101
commit.CommitMessage = messageSB.String()
102-
commit.Signature = &CommitGPGSignature{
102+
commit.Signature = &CommitSignature{
103103
Signature: signatureSB.String(),
104104
Payload: payloadSB.String(),
105105
}

‎modules/git/repo_tag.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,17 +185,15 @@ func parseTagRef(ref map[string]string) (tag *Tag, err error) {
185185

186186
tag.Tagger = parseSignatureFromCommitLine(ref["creator"])
187187
tag.Message = ref["contents"]
188-
// strip PGP signature if present in contents field
189-
pgpStart := strings.Index(tag.Message, beginpgp)
190-
if pgpStart >= 0 {
191-
tag.Message = tag.Message[0:pgpStart]
192-
}
188+
189+
// strip any signature if present in contents field
190+
_, tag.Message, _ = parsePayloadSignature(util.UnsafeStringToBytes(tag.Message), 0)
193191

194192
// annotated tag with GPG signature
195193
if tag.Type == "tag" && ref["contents:signature"] != "" {
196194
payload := fmt.Sprintf("object %s\ntype commit\ntag %s\ntagger %s\n\n%s\n",
197195
tag.Object, tag.Name, ref["creator"], strings.TrimSpace(tag.Message))
198-
tag.Signature = &CommitGPGSignature{
196+
tag.Signature = &CommitSignature{
199197
Signature: ref["contents:signature"],
200198
Payload: payload,
201199
}

‎modules/git/repo_tag_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ qbHDASXl
315315
Type: "tag",
316316
Tagger: parseSignatureFromCommitLine("Foo Bar <[email protected]> 1565789218 +0300"),
317317
Message: "Add changelog of v1.9.1 (#7859)\n\n* add changelog of v1.9.1\n* Update CHANGELOG.md",
318-
Signature: &CommitGPGSignature{
318+
Signature: &CommitSignature{
319319
Signature: `-----BEGIN PGP SIGNATURE-----
320320
321321
aBCGzBAABCgAdFiEEyWRwv/q1Q6IjSv+D4IPOwzt33PoFAmI8jbIACgkQ4IPOwzt3

‎modules/git/tag.go‎

Lines changed: 58 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,10 @@ package git
66
import (
77
"bytes"
88
"sort"
9-
"strings"
109

1110
"code.gitea.io/gitea/modules/util"
1211
)
1312

14-
const (
15-
beginpgp = "\n-----BEGIN PGP SIGNATURE-----\n"
16-
endpgp = "\n-----END PGP SIGNATURE-----"
17-
)
18-
1913
// Tag represents a Git tag.
2014
type Tag struct {
2115
Name string
@@ -24,14 +18,44 @@ type Tag struct {
2418
Type string
2519
Tagger *Signature
2620
Message string
27-
Signature *CommitGPGSignature
21+
Signature *CommitSignature
2822
}
2923

3024
// Commit return the commit of the tag reference
3125
func (tag *Tag) Commit(gitRepo *Repository) (*Commit, error) {
3226
return gitRepo.getCommit(tag.Object)
3327
}
3428

29+
func parsePayloadSignature(data []byte, messageStart int) (payload, msg, sign string) {
30+
pos := messageStart
31+
signStart, signEnd := -1, -1
32+
for {
33+
eol := bytes.IndexByte(data[pos:], '\n')
34+
if eol < 0 {
35+
break
36+
}
37+
line := data[pos : pos+eol]
38+
signType, hasPrefix := bytes.CutPrefix(line, []byte("-----BEGIN "))
39+
signType, hasSuffix := bytes.CutSuffix(signType, []byte(" SIGNATURE-----"))
40+
if hasPrefix && hasSuffix {
41+
signEndBytes := append([]byte("\n-----END "), signType...)
42+
signEndBytes = append(signEndBytes, []byte(" SIGNATURE-----")...)
43+
signEnd = bytes.Index(data[pos:], signEndBytes)
44+
if signEnd != -1 {
45+
signStart = pos
46+
signEnd = pos + signEnd + len(signEndBytes)
47+
}
48+
}
49+
pos += eol + 1
50+
}
51+
52+
if signStart != -1 && signEnd != -1 {
53+
msgEnd := max(messageStart, signStart-1)
54+
return string(data[:msgEnd]), string(data[messageStart:msgEnd]), string(data[signStart:signEnd])
55+
}
56+
return string(data), string(data[messageStart:]), ""
57+
}
58+
3559
// Parse commit information from the (uncompressed) raw
3660
// data from the commit object.
3761
// \n\n separate headers from message
@@ -40,47 +64,37 @@ func parseTagData(objectFormat ObjectFormat, data []byte) (*Tag, error) {
4064
tag.ID = objectFormat.EmptyObjectID()
4165
tag.Object = objectFormat.EmptyObjectID()
4266
tag.Tagger = &Signature{}
43-
// we now have the contents of the commit object. Let's investigate...
44-
nextline := 0
45-
l:
67+
68+
pos := 0
4669
for {
47-
eol := bytes.IndexByte(data[nextline:], '\n')
48-
switch {
49-
case eol > 0:
50-
line := data[nextline : nextline+eol]
51-
spacepos := bytes.IndexByte(line, ' ')
52-
reftype := line[:spacepos]
53-
switch string(reftype) {
54-
case "object":
55-
id, err := NewIDFromString(string(line[spacepos+1:]))
56-
if err != nil {
57-
return nil, err
58-
}
59-
tag.Object = id
60-
case "type":
61-
// A commit can have one or more parents
62-
tag.Type = string(line[spacepos+1:])
63-
case "tagger":
64-
tag.Tagger = parseSignatureFromCommitLine(util.UnsafeBytesToString(line[spacepos+1:]))
65-
}
66-
nextline += eol + 1
67-
case eol == 0:
68-
tag.Message = string(data[nextline+1:])
69-
break l
70-
default:
71-
break l
70+
eol := bytes.IndexByte(data[pos:], '\n')
71+
if eol == -1 {
72+
break // shouldn't happen, but could just tolerate it
7273
}
73-
}
74-
idx := strings.LastIndex(tag.Message, beginpgp)
75-
if idx > 0 {
76-
endSigIdx := strings.Index(tag.Message[idx:], endpgp)
77-
if endSigIdx > 0 {
78-
tag.Signature = &CommitGPGSignature{
79-
Signature: tag.Message[idx+1 : idx+endSigIdx+len(endpgp)],
80-
Payload: string(data[:bytes.LastIndex(data, []byte(beginpgp))+1]),
74+
if eol == 0 {
75+
pos++
76+
break // end of headers
77+
}
78+
line := data[pos : pos+eol]
79+
key, val, _ := bytes.Cut(line, []byte(" "))
80+
switch string(key) {
81+
case "object":
82+
id, err := NewIDFromString(string(val))
83+
if err != nil {
84+
return nil, err
8185
}
82-
tag.Message = tag.Message[:idx+1]
86+
tag.Object = id
87+
case "type":
88+
tag.Type = string(val) // A commit can have one or more parents
89+
case "tagger":
90+
tag.Tagger = parseSignatureFromCommitLine(util.UnsafeBytesToString(val))
8391
}
92+
pos += eol + 1
93+
}
94+
payload, msg, sign := parsePayloadSignature(data, pos)
95+
tag.Message = msg
96+
if len(sign) > 0 {
97+
tag.Signature = &CommitSignature{Signature: sign, Payload: payload}
8498
}
8599
return tag, nil
86100
}

‎modules/git/tag_test.go‎

Lines changed: 65 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -12,62 +12,86 @@ import (
1212

1313
func Test_parseTagData(t *testing.T) {
1414
testData := []struct {
15-
data []byte
16-
tag Tag
15+
data string
16+
expected Tag
1717
}{
18-
{data: []byte(`object 3b114ab800c6432ad42387ccf6bc8d4388a2885a
18+
{
19+
data: `object 3b114ab800c6432ad42387ccf6bc8d4388a2885a
1920
type commit
2021
tag 1.22.0
2122
tagger Lucas Michot <[email protected]> 1484491741 +0100
2223
23-
`), tag: Tag{
24-
Name: "",
25-
ID: Sha1ObjectFormat.EmptyObjectID(),
26-
Object: &Sha1Hash{0x3b, 0x11, 0x4a, 0xb8, 0x0, 0xc6, 0x43, 0x2a, 0xd4, 0x23, 0x87, 0xcc, 0xf6, 0xbc, 0x8d, 0x43, 0x88, 0xa2, 0x88, 0x5a},
27-
Type: "commit",
28-
Tagger: &Signature{Name: "Lucas Michot", Email: "[email protected]", When: time.Unix(1484491741, 0)},
29-
Message: "",
30-
Signature: nil,
31-
}},
32-
{data: []byte(`object 7cdf42c0b1cc763ab7e4c33c47a24e27c66bfccc
24+
`,
25+
expected: Tag{
26+
Name: "",
27+
ID: Sha1ObjectFormat.EmptyObjectID(),
28+
Object: MustIDFromString("3b114ab800c6432ad42387ccf6bc8d4388a2885a"),
29+
Type: "commit",
30+
Tagger: &Signature{Name: "Lucas Michot", Email: "[email protected]", When: time.Unix(1484491741, 0).In(time.FixedZone("", 3600))},
31+
Message: "",
32+
Signature: nil,
33+
},
34+
},
35+
{
36+
data: `object 7cdf42c0b1cc763ab7e4c33c47a24e27c66bfccc
3337
type commit
3438
tag 1.22.1
3539
tagger Lucas Michot <[email protected]> 1484553735 +0100
3640
3741
test message
3842
o
3943
40-
ono`), tag: Tag{
41-
Name: "",
42-
ID: Sha1ObjectFormat.EmptyObjectID(),
43-
Object: &Sha1Hash{0x7c, 0xdf, 0x42, 0xc0, 0xb1, 0xcc, 0x76, 0x3a, 0xb7, 0xe4, 0xc3, 0x3c, 0x47, 0xa2, 0x4e, 0x27, 0xc6, 0x6b, 0xfc, 0xcc},
44-
Type: "commit",
45-
Tagger: &Signature{Name: "Lucas Michot", Email: "[email protected]", When: time.Unix(1484553735, 0)},
46-
Message: "test message\no\n\nono",
47-
Signature: nil,
48-
}},
44+
ono`,
45+
expected: Tag{
46+
Name: "",
47+
ID: Sha1ObjectFormat.EmptyObjectID(),
48+
Object: MustIDFromString("7cdf42c0b1cc763ab7e4c33c47a24e27c66bfccc"),
49+
Type: "commit",
50+
Tagger: &Signature{Name: "Lucas Michot", Email: "[email protected]", When: time.Unix(1484553735, 0).In(time.FixedZone("", 3600))},
51+
Message: "test message\no\n\nono",
52+
Signature: nil,
53+
},
54+
},
55+
{
56+
data: `object 7cdf42c0b1cc763ab7e4c33c47a24e27c66bfaaa
57+
type commit
58+
tag v0
59+
tagger dummy user <[email protected]> 1484491741 +0100
60+
61+
dummy message
62+
-----BEGIN SSH SIGNATURE-----
63+
dummy signature
64+
-----END SSH SIGNATURE-----
65+
`,
66+
expected: Tag{
67+
Name: "",
68+
ID: Sha1ObjectFormat.EmptyObjectID(),
69+
Object: MustIDFromString("7cdf42c0b1cc763ab7e4c33c47a24e27c66bfaaa"),
70+
Type: "commit",
71+
Tagger: &Signature{Name: "dummy user", Email: "[email protected]", When: time.Unix(1484491741, 0).In(time.FixedZone("", 3600))},
72+
Message: "dummy message",
73+
Signature: &CommitSignature{
74+
Signature: `-----BEGIN SSH SIGNATURE-----
75+
dummy signature
76+
-----END SSH SIGNATURE-----`,
77+
Payload: `object 7cdf42c0b1cc763ab7e4c33c47a24e27c66bfaaa
78+
type commit
79+
tag v0
80+
tagger dummy user <[email protected]> 1484491741 +0100
81+
82+
dummy message`,
83+
},
84+
},
85+
},
4986
}
5087

5188
for _, test := range testData {
52-
tag, err := parseTagData(Sha1ObjectFormat, test.data)
89+
tag, err := parseTagData(Sha1ObjectFormat, []byte(test.data))
5390
assert.NoError(t, err)
54-
assert.EqualValues(t, test.tag.ID, tag.ID)
55-
assert.EqualValues(t, test.tag.Object, tag.Object)
56-
assert.EqualValues(t, test.tag.Name, tag.Name)
57-
assert.EqualValues(t, test.tag.Message, tag.Message)
58-
assert.EqualValues(t, test.tag.Type, tag.Type)
59-
if test.tag.Signature != nil && assert.NotNil(t, tag.Signature) {
60-
assert.EqualValues(t, test.tag.Signature.Signature, tag.Signature.Signature)
61-
assert.EqualValues(t, test.tag.Signature.Payload, tag.Signature.Payload)
62-
} else {
63-
assert.Nil(t, tag.Signature)
64-
}
65-
if test.tag.Tagger != nil && assert.NotNil(t, tag.Tagger) {
66-
assert.EqualValues(t, test.tag.Tagger.Name, tag.Tagger.Name)
67-
assert.EqualValues(t, test.tag.Tagger.Email, tag.Tagger.Email)
68-
assert.EqualValues(t, test.tag.Tagger.When.Unix(), tag.Tagger.When.Unix())
69-
} else {
70-
assert.Nil(t, tag.Tagger)
71-
}
91+
assert.Equal(t, test.expected, *tag)
7292
}
93+
94+
tag, err := parseTagData(Sha1ObjectFormat, []byte("type commit\n\nfoo\n-----BEGIN SSH SIGNATURE-----\ncorrupted..."))
95+
assert.NoError(t, err)
96+
assert.Equal(t, "foo\n-----BEGIN SSH SIGNATURE-----\ncorrupted...", tag.Message)
7397
}

0 commit comments

Comments
 (0)