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>

1 files changed, 106 insertions, 0 deletions

diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go
index 0925a6ce6a..57132c00dc 100644
--- a/modules/git/commit_test.go
+++ b/modules/git/commit_test.go
@@ -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)
+	}
+
+}