Skip to content

Commit f549842

Browse files
GiteaBotwxiaoguang
andauthored
Refactor commit reader (#34542) (#34549)
Backport #34542 by wxiaoguang Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent a6a14c9 commit f549842

File tree

4 files changed

+68
-84
lines changed

4 files changed

+68
-84
lines changed

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 <amajer@suse.de> 1698676906 +0100
6766
committer Adam Majer <amajer@suse.de> 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 <amajer@suse.de> 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 <me@silverwind.io> 1563741793 +0200
6665
committer silverwind <me@silverwind.io> 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 <me@silverwind.io> 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 <admin@oldschoolhack.me> 1711702962 +0100
133130
committer KN4CK3R <admin@oldschoolhack.me> 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 <admin@oldschoolhack.me> 1711702962 +0100

0 commit comments

Comments
 (0)