Do not assume all 40 char strings are SHA1s (#14624) (#14648)

Backport #14624

GetCommit() assumes that all 40 char strings are SHA1s. This leads to an
error if you try to do a PR on a branch which is 40 characters long.

This PR attempts the SHA first - and if it fails will switch to using rev-parse.

Fix #14470

Signed-off-by: Andrew Thornton <art27@cantab.net>

1 files changed, 15 insertions, 11 deletions

diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
index c9a5efb24e..a00e59edb8 100644
--- a/modules/git/repo_commit.go
+++ b/modules/git/repo_commit.go
@@ -129,19 +129,23 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) {
 
 // ConvertToSHA1 returns a Hash object from a potential ID string
 func (repo *Repository) ConvertToSHA1(commitID string) (SHA1, error) {
-	if len(commitID) != 40 {
-		var err error
-		actualCommitID, err := NewCommand("rev-parse", "--verify", commitID).RunInDir(repo.Path)
-		if err != nil {
-			if strings.Contains(err.Error(), "unknown revision or path") ||
-				strings.Contains(err.Error(), "fatal: Needed a single revision") {
-				return SHA1{}, ErrNotExist{commitID, ""}
-			}
-			return SHA1{}, err
+	if len(commitID) == 40 {
+		sha1, err := NewIDFromString(commitID)
+		if err == nil {
+			return sha1, nil
+		}
+	}
+
+	actualCommitID, err := NewCommand("rev-parse", "--verify", commitID).RunInDir(repo.Path)
+	if err != nil {
+		if strings.Contains(err.Error(), "unknown revision or path") ||
+			strings.Contains(err.Error(), "fatal: Needed a single revision") {
+			return SHA1{}, ErrNotExist{commitID, ""}
 		}
-		commitID = actualCommitID
+		return SHA1{}, err
 	}
-	return NewIDFromString(commitID)
+
+	return NewIDFromString(actualCommitID)
 }
 
 // GetCommit returns commit object of by ID string.