Fix webhook commits wrong hash on HEAD reset (#16283)

Use `..` instead of `...` with `rev-list`. In combination with #16282 the receiver can get the correct commit. The behaviour is now like Github.

fixes #11802

16 files changed, 48 insertions, 5 deletions

diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
index 815aa141e5..5b417cd774 100644
--- a/modules/git/repo_commit.go
+++ b/modules/git/repo_commit.go
@@ -264,14 +264,15 @@ func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (in
 	return len(strings.Split(stdout, "\n")) - 1, nil
 }
 
-// CommitsBetween returns a list that contains commits between [last, before).
+// CommitsBetween returns a list that contains commits between [before, last).
+// If before is detached (removed by reset + push) it is not included.
 func (repo *Repository) CommitsBetween(last *Commit, before *Commit) (*list.List, error) {
 	var stdout []byte
 	var err error
 	if before == nil {
 		stdout, err = NewCommand("rev-list", last.ID.String()).RunInDirBytes(repo.Path)
 	} else {
-		stdout, err = NewCommand("rev-list", before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path)
+		stdout, err = NewCommand("rev-list", before.ID.String()+".."+last.ID.String()).RunInDirBytes(repo.Path)
 		if err != nil && strings.Contains(err.Error(), "no merge base") {
 			// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
 			// previously it would return the results of git rev-list before last so let's try that...
@@ -284,14 +285,14 @@ func (repo *Repository) CommitsBetween(last *Commit, before *Commit) (*list.List
 	return repo.parsePrettyFormatLogToList(bytes.TrimSpace(stdout))
 }
 
-// CommitsBetweenLimit returns a list that contains at most limit commits skipping the first skip commits between [last, before)
+// CommitsBetweenLimit returns a list that contains at most limit commits skipping the first skip commits between [before, last)
 func (repo *Repository) CommitsBetweenLimit(last *Commit, before *Commit, limit, skip int) (*list.List, error) {
 	var stdout []byte
 	var err error
 	if before == nil {
 		stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), last.ID.String()).RunInDirBytes(repo.Path)
 	} else {
-		stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path)
+		stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), before.ID.String()+".."+last.ID.String()).RunInDirBytes(repo.Path)
 		if err != nil && strings.Contains(err.Error(), "no merge base") {
 			// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
 			// previously it would return the results of git rev-list --max-count n before last so let's try that...
@@ -322,7 +323,7 @@ func (repo *Repository) CommitsBetweenIDs(last, before string) (*list.List, erro
 
 // CommitsCountBetween return numbers of commits between two commits
 func (repo *Repository) CommitsCountBetween(start, end string) (int64, error) {
-	count, err := CommitsCountFiles(repo.Path, []string{start + "..." + end}, []string{})
+	count, err := CommitsCountFiles(repo.Path, []string{start + ".." + end}, []string{})
 	if err != nil && strings.Contains(err.Error(), "no merge base") {
 		// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
 		// previously it would return the results of git rev-list before last so let's try that...
diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go
index 8f8acbdfed..a6c27ea4d5 100644
--- a/modules/git/repo_commit_test.go
+++ b/modules/git/repo_commit_test.go
@@ -78,3 +78,25 @@ func TestIsCommitInBranch(t *testing.T) {
 	assert.NoError(t, err)
 	assert.False(t, result)
 }
+
+func TestRepository_CommitsBetweenIDs(t *testing.T) {
+	bareRepo1Path := filepath.Join(testReposDir, "repo4_commitsbetween")
+	bareRepo1, err := OpenRepository(bareRepo1Path)
+	assert.NoError(t, err)
+	defer bareRepo1.Close()
+
+	cases := []struct {
+		OldID           string
+		NewID           string
+		ExpectedCommits int
+	}{
+		{"fdc1b615bdcff0f0658b216df0c9209e5ecb7c78", "78a445db1eac62fe15e624e1137965969addf344", 1}, //com1 -> com2
+		{"78a445db1eac62fe15e624e1137965969addf344", "fdc1b615bdcff0f0658b216df0c9209e5ecb7c78", 0}, //reset HEAD~, com2 -> com1
+		{"78a445db1eac62fe15e624e1137965969addf344", "a78e5638b66ccfe7e1b4689d3d5684e42c97d7ca", 1}, //com2 -> com2_new
+	}
+	for i, c := range cases {
+		commits, err := bareRepo1.CommitsBetweenIDs(c.NewID, c.OldID)
+		assert.NoError(t, err)
+		assert.Equal(t, c.ExpectedCommits, commits.Len(), "case %d", i)
+	}
+}
diff --git a/modules/git/tests/repos/repo4_commitsbetween/HEAD b/modules/git/tests/repos/repo4_commitsbetween/HEAD
new file mode 100644
index 0000000000..b870d82622
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/HEAD
@@ -0,0 +1 @@
+ref: refs/heads/main
diff --git a/modules/git/tests/repos/repo4_commitsbetween/config b/modules/git/tests/repos/repo4_commitsbetween/config
new file mode 100644
index 0000000000..d545cdabdb
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/config
@@ -0,0 +1,7 @@
+[core]
+	repositoryformatversion = 0
+	filemode = false
+	bare = false
+	logallrefupdates = true
+	symlinks = false
+	ignorecase = true
diff --git a/modules/git/tests/repos/repo4_commitsbetween/logs/HEAD b/modules/git/tests/repos/repo4_commitsbetween/logs/HEAD
new file mode 100644
index 0000000000..24cc684bae
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/logs/HEAD
@@ -0,0 +1,4 @@
+0000000000000000000000000000000000000000 fdc1b615bdcff0f0658b216df0c9209e5ecb7c78 KN4CK3R <admin@oldschoolhack.me> 1624915979 +0200	commit (initial): com1
+fdc1b615bdcff0f0658b216df0c9209e5ecb7c78 78a445db1eac62fe15e624e1137965969addf344 KN4CK3R <admin@oldschoolhack.me> 1624915993 +0200	commit: com2
+78a445db1eac62fe15e624e1137965969addf344 fdc1b615bdcff0f0658b216df0c9209e5ecb7c78 KN4CK3R <admin@oldschoolhack.me> 1624916008 +0200	reset: moving to HEAD~1
+fdc1b615bdcff0f0658b216df0c9209e5ecb7c78 a78e5638b66ccfe7e1b4689d3d5684e42c97d7ca KN4CK3R <admin@oldschoolhack.me> 1624916029 +0200	commit: com2_new
diff --git a/modules/git/tests/repos/repo4_commitsbetween/logs/refs/heads/main b/modules/git/tests/repos/repo4_commitsbetween/logs/refs/heads/main
new file mode 100644
index 0000000000..24cc684bae
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/logs/refs/heads/main
@@ -0,0 +1,4 @@
+0000000000000000000000000000000000000000 fdc1b615bdcff0f0658b216df0c9209e5ecb7c78 KN4CK3R <admin@oldschoolhack.me> 1624915979 +0200	commit (initial): com1
+fdc1b615bdcff0f0658b216df0c9209e5ecb7c78 78a445db1eac62fe15e624e1137965969addf344 KN4CK3R <admin@oldschoolhack.me> 1624915993 +0200	commit: com2
+78a445db1eac62fe15e624e1137965969addf344 fdc1b615bdcff0f0658b216df0c9209e5ecb7c78 KN4CK3R <admin@oldschoolhack.me> 1624916008 +0200	reset: moving to HEAD~1
+fdc1b615bdcff0f0658b216df0c9209e5ecb7c78 a78e5638b66ccfe7e1b4689d3d5684e42c97d7ca KN4CK3R <admin@oldschoolhack.me> 1624916029 +0200	commit: com2_new
diff --git a/modules/git/tests/repos/repo4_commitsbetween/objects/27/734c860ab19650d48e71f9f12d9bd194ed82ea b/modules/git/tests/repos/repo4_commitsbetween/objects/27/734c860ab19650d48e71f9f12d9bd194ed82ea
new file mode 100644
index 0000000000..5b26f8b3a8
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/objects/27/734c860ab19650d48e71f9f12d9bd194ed82ea
diff --git a/modules/git/tests/repos/repo4_commitsbetween/objects/56/a6051ca2b02b04ef92d5150c9ef600403cb1de b/modules/git/tests/repos/repo4_commitsbetween/objects/56/a6051ca2b02b04ef92d5150c9ef600403cb1de
new file mode 100644
index 0000000000..b17dfe30e6
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/objects/56/a6051ca2b02b04ef92d5150c9ef600403cb1de
diff --git a/modules/git/tests/repos/repo4_commitsbetween/objects/78/a445db1eac62fe15e624e1137965969addf344 b/modules/git/tests/repos/repo4_commitsbetween/objects/78/a445db1eac62fe15e624e1137965969addf344
new file mode 100644
index 0000000000..6d23de052e
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/objects/78/a445db1eac62fe15e624e1137965969addf344
@@ -0,0 +1,3 @@
+x
��M
+�0@a�=E��̤I������$�Zl�����|�G���)�îm̊uO�"����&`�8Gt�I�7�#n�6%�09�)�8�F�(hl��@���MuS��\����1�y=�%?i�u�"��O
+��Dm�ڃ��w���ź{p�C_
\ No newline at end of file
diff --git a/modules/git/tests/repos/repo4_commitsbetween/objects/a7/8e5638b66ccfe7e1b4689d3d5684e42c97d7ca b/modules/git/tests/repos/repo4_commitsbetween/objects/a7/8e5638b66ccfe7e1b4689d3d5684e42c97d7ca
new file mode 100644
index 0000000000..d5c554a542
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/objects/a7/8e5638b66ccfe7e1b4689d3d5684e42c97d7ca
diff --git a/modules/git/tests/repos/repo4_commitsbetween/objects/ad/74ceca1b8fde10c7d933bd2e56d347dddb4ab5 b/modules/git/tests/repos/repo4_commitsbetween/objects/ad/74ceca1b8fde10c7d933bd2e56d347dddb4ab5
new file mode 100644
index 0000000000..26ed785006
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/objects/ad/74ceca1b8fde10c7d933bd2e56d347dddb4ab5
diff --git a/modules/git/tests/repos/repo4_commitsbetween/objects/b5/d8dd0ddd9d8d752bb47b5f781f09f478316098 b/modules/git/tests/repos/repo4_commitsbetween/objects/b5/d8dd0ddd9d8d752bb47b5f781f09f478316098
new file mode 100644
index 0000000000..8060b57df0
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/objects/b5/d8dd0ddd9d8d752bb47b5f781f09f478316098
diff --git a/modules/git/tests/repos/repo4_commitsbetween/objects/d8/263ee9860594d2806b0dfd1bfd17528b0ba2a4 b/modules/git/tests/repos/repo4_commitsbetween/objects/d8/263ee9860594d2806b0dfd1bfd17528b0ba2a4
new file mode 100644
index 0000000000..4b1baefffb
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/objects/d8/263ee9860594d2806b0dfd1bfd17528b0ba2a4
diff --git a/modules/git/tests/repos/repo4_commitsbetween/objects/e2/3cc6a008501f1491b0480cedaef160e41cf684 b/modules/git/tests/repos/repo4_commitsbetween/objects/e2/3cc6a008501f1491b0480cedaef160e41cf684
new file mode 100644
index 0000000000..0a70530845
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/objects/e2/3cc6a008501f1491b0480cedaef160e41cf684
diff --git a/modules/git/tests/repos/repo4_commitsbetween/objects/fd/c1b615bdcff0f0658b216df0c9209e5ecb7c78 b/modules/git/tests/repos/repo4_commitsbetween/objects/fd/c1b615bdcff0f0658b216df0c9209e5ecb7c78
new file mode 100644
index 0000000000..2e6d94584c
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/objects/fd/c1b615bdcff0f0658b216df0c9209e5ecb7c78
diff --git a/modules/git/tests/repos/repo4_commitsbetween/refs/heads/main b/modules/git/tests/repos/repo4_commitsbetween/refs/heads/main
new file mode 100644
index 0000000000..9e1b981a6e
--- /dev/null
+++ b/modules/git/tests/repos/repo4_commitsbetween/refs/heads/main
@@ -0,0 +1 @@
+a78e5638b66ccfe7e1b4689d3d5684e42c97d7ca