Fix modified files list in webhooks when there is a space (#16288)
* Fix modified files list in webhooks when there is a space There is an unfortunate bug with GetCommitFileStatus where files with spaces are misparsed and split at the space. There is a second bug because modern gits detect renames meaning that this function no longer works correctly. There is a third bug in that merge commits don't have their modified files detected correctly. Fix #15865 Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath
GitHub
parent
836884429a
commit
62c278e4ab
2 changed files with 152 additions and 18 deletions
|
|
@ -15,6 +15,8 @@ import (
|
|||
"os/exec"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"code.gitea.io/gitea/modules/log"
|
||||
)
|
||||
|
||||
// Commit represents a git commit.
|
||||
|
|
@ -432,33 +434,59 @@ func NewCommitFileStatus() *CommitFileStatus {
|
|||
}
|
||||
}
|
||||
|
||||
func parseCommitFileStatus(fileStatus *CommitFileStatus, stdout io.Reader) {
|
||||
rd := bufio.NewReader(stdout)
|
||||
peek, err := rd.Peek(1)
|
||||
if err != nil {
|
||||
if err != io.EOF {
|
||||
log.Error("Unexpected error whilst reading from git log --name-status. Error: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
if peek[0] == '\n' || peek[0] == '\x00' {
|
||||
_, _ = rd.Discard(1)
|
||||
}
|
||||
for {
|
||||
modifier, err := rd.ReadSlice('\x00')
|
||||
if err != nil {
|
||||
if err != io.EOF {
|
||||
log.Error("Unexpected error whilst reading from git log --name-status. Error: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
file, err := rd.ReadString('\x00')
|
||||
if err != nil {
|
||||
if err != io.EOF {
|
||||
log.Error("Unexpected error whilst reading from git log --name-status. Error: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
file = file[:len(file)-1]
|
||||
switch modifier[0] {
|
||||
case 'A':
|
||||
fileStatus.Added = append(fileStatus.Added, file)
|
||||
case 'D':
|
||||
fileStatus.Removed = append(fileStatus.Removed, file)
|
||||
case 'M':
|
||||
fileStatus.Modified = append(fileStatus.Modified, file)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// GetCommitFileStatus returns file status of commit in given repository.
|
||||
func GetCommitFileStatus(repoPath, commitID string) (*CommitFileStatus, error) {
|
||||
stdout, w := io.Pipe()
|
||||
done := make(chan struct{})
|
||||
fileStatus := NewCommitFileStatus()
|
||||
go func() {
|
||||
scanner := bufio.NewScanner(stdout)
|
||||
for scanner.Scan() {
|
||||
fields := strings.Fields(scanner.Text())
|
||||
if len(fields) < 2 {
|
||||
continue
|
||||
}
|
||||
|
||||
switch fields[0][0] {
|
||||
case 'A':
|
||||
fileStatus.Added = append(fileStatus.Added, fields[1])
|
||||
case 'D':
|
||||
fileStatus.Removed = append(fileStatus.Removed, fields[1])
|
||||
case 'M':
|
||||
fileStatus.Modified = append(fileStatus.Modified, fields[1])
|
||||
}
|
||||
}
|
||||
done <- struct{}{}
|
||||
parseCommitFileStatus(fileStatus, stdout)
|
||||
close(done)
|
||||
}()
|
||||
|
||||
stderr := new(bytes.Buffer)
|
||||
err := NewCommand("show", "--name-status", "--pretty=format:''", commitID).RunInDirPipeline(repoPath, w, stderr)
|
||||
args := []string{"log", "--name-status", "-c", "--pretty=format:", "--parents", "--no-renames", "-z", "-1", commitID}
|
||||
|
||||
err := NewCommand(args...).RunInDirPipeline(repoPath, w, stderr)
|
||||
w.Close() // Close writer to exit parsing goroutine
|
||||
if err != nil {
|
||||
return nil, ConcatenateError(err, stderr.String())
|
||||
|
|
|
|||
|
|
@ -130,3 +130,109 @@ func TestHasPreviousCommit(t *testing.T) {
|
|||
assert.NoError(t, err)
|
||||
assert.False(t, selfNot)
|
||||
}
|
||||
|
||||
func TestParseCommitFileStatus(t *testing.T) {
|
||||
type testcase struct {
|
||||
output string
|
||||
added []string
|
||||
removed []string
|
||||
modified []string
|
||||
}
|
||||
|
||||
kases := []testcase{
|
||||
{
|
||||
// Merge commit
|
||||
output: "MM\x00options/locale/locale_en-US.ini\x00",
|
||||
modified: []string{
|
||||
"options/locale/locale_en-US.ini",
|
||||
},
|
||||
added: []string{},
|
||||
removed: []string{},
|
||||
},
|
||||
{
|
||||
// Spaces commit
|
||||
output: "D\x00b\x00D\x00b b/b\x00A\x00b b/b b/b b/b\x00A\x00b b/b b/b b/b b/b\x00",
|
||||
removed: []string{
|
||||
"b",
|
||||
"b b/b",
|
||||
},
|
||||
modified: []string{},
|
||||
added: []string{
|
||||
"b b/b b/b b/b",
|
||||
"b b/b b/b b/b b/b",
|
||||
},
|
||||
},
|
||||
{
|
||||
// larger commit
|
||||
output: "M\x00go.mod\x00M\x00go.sum\x00M\x00modules/ssh/ssh.go\x00M\x00vendor/github.com/gliderlabs/ssh/circle.yml\x00M\x00vendor/github.com/gliderlabs/ssh/context.go\x00A\x00vendor/github.com/gliderlabs/ssh/go.mod\x00A\x00vendor/github.com/gliderlabs/ssh/go.sum\x00M\x00vendor/github.com/gliderlabs/ssh/server.go\x00M\x00vendor/github.com/gliderlabs/ssh/session.go\x00M\x00vendor/github.com/gliderlabs/ssh/ssh.go\x00M\x00vendor/golang.org/x/sys/unix/mkerrors.sh\x00M\x00vendor/golang.org/x/sys/unix/syscall_darwin.go\x00M\x00vendor/golang.org/x/sys/unix/zerrors_darwin_amd64.go\x00M\x00vendor/golang.org/x/sys/unix/zerrors_darwin_arm64.go\x00M\x00vendor/golang.org/x/sys/unix/zerrors_freebsd_386.go\x00M\x00vendor/golang.org/x/sys/unix/zerrors_freebsd_amd64.go\x00M\x00vendor/golang.org/x/sys/unix/zerrors_freebsd_arm.go\x00M\x00vendor/golang.org/x/sys/unix/zerrors_freebsd_arm64.go\x00M\x00vendor/golang.org/x/sys/unix/zerrors_linux.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_darwin_amd64.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_darwin_arm64.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_dragonfly_amd64.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_freebsd_386.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_freebsd_amd64.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_freebsd_arm.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_freebsd_arm64.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_netbsd_386.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_netbsd_amd64.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_netbsd_arm.go\x00M\x00vendor/golang.org/x/sys/unix/ztypes_netbsd_arm64.go\x00M\x00vendor/modules.txt\x00",
|
||||
modified: []string{
|
||||
"go.mod",
|
||||
"go.sum",
|
||||
"modules/ssh/ssh.go",
|
||||
"vendor/github.com/gliderlabs/ssh/circle.yml",
|
||||
"vendor/github.com/gliderlabs/ssh/context.go",
|
||||
"vendor/github.com/gliderlabs/ssh/server.go",
|
||||
"vendor/github.com/gliderlabs/ssh/session.go",
|
||||
"vendor/github.com/gliderlabs/ssh/ssh.go",
|
||||
"vendor/golang.org/x/sys/unix/mkerrors.sh",
|
||||
"vendor/golang.org/x/sys/unix/syscall_darwin.go",
|
||||
"vendor/golang.org/x/sys/unix/zerrors_darwin_amd64.go",
|
||||
"vendor/golang.org/x/sys/unix/zerrors_darwin_arm64.go",
|
||||
"vendor/golang.org/x/sys/unix/zerrors_freebsd_386.go",
|
||||
"vendor/golang.org/x/sys/unix/zerrors_freebsd_amd64.go",
|
||||
"vendor/golang.org/x/sys/unix/zerrors_freebsd_arm.go",
|
||||
"vendor/golang.org/x/sys/unix/zerrors_freebsd_arm64.go",
|
||||
"vendor/golang.org/x/sys/unix/zerrors_linux.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_darwin_amd64.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_darwin_arm64.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_dragonfly_amd64.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_freebsd_386.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_freebsd_amd64.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_freebsd_arm.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_freebsd_arm64.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_netbsd_386.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_netbsd_amd64.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_netbsd_arm.go",
|
||||
"vendor/golang.org/x/sys/unix/ztypes_netbsd_arm64.go",
|
||||
"vendor/modules.txt",
|
||||
},
|
||||
added: []string{
|
||||
"vendor/github.com/gliderlabs/ssh/go.mod",
|
||||
"vendor/github.com/gliderlabs/ssh/go.sum",
|
||||
},
|
||||
removed: []string{},
|
||||
},
|
||||
{
|
||||
// git 1.7.2 adds an unnecessary \x00 on merge commit
|
||||
output: "\x00MM\x00options/locale/locale_en-US.ini\x00",
|
||||
modified: []string{
|
||||
"options/locale/locale_en-US.ini",
|
||||
},
|
||||
added: []string{},
|
||||
removed: []string{},
|
||||
},
|
||||
{
|
||||
// git 1.7.2 adds an unnecessary \n on normal commit
|
||||
output: "\nD\x00b\x00D\x00b b/b\x00A\x00b b/b b/b b/b\x00A\x00b b/b b/b b/b b/b\x00",
|
||||
removed: []string{
|
||||
"b",
|
||||
"b b/b",
|
||||
},
|
||||
modified: []string{},
|
||||
added: []string{
|
||||
"b b/b b/b b/b",
|
||||
"b b/b b/b b/b b/b",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, kase := range kases {
|
||||
fileStatus := NewCommitFileStatus()
|
||||
parseCommitFileStatus(fileStatus, strings.NewReader(kase.output))
|
||||
|
||||
assert.Equal(t, kase.added, fileStatus.Added)
|
||||
assert.Equal(t, kase.removed, fileStatus.Removed)
|
||||
assert.Equal(t, kase.modified, fileStatus.Modified)
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Reference in a new issue