Handle edge case of annotated tags without messages

2 files changed, 60 insertions, 8 deletions

diff --git a/internal/git/log/tag.go b/internal/git/log/tag.go
index a117ddac5..5465d8a09 100644
--- a/internal/git/log/tag.go
+++ b/internal/git/log/tag.go
@@ -3,7 +3,6 @@ package log
 import (
 	"bufio"
 	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -48,16 +47,15 @@ func splitRawTag(r io.Reader) (*tagHeader, []byte, error) {
 		return nil, nil, err
 	}
 
+	var body []byte
 	split := bytes.SplitN(raw, []byte("\n\n"), 2)
-	if len(split) != 2 {
-		return nil, nil, errors.New("invalid tag object")
+	if len(split) == 2 {
+		// Remove trailing newline, if any, to preserve existing behavior the old GitLab tag finding code.
+		// See https://gitlab.com/gitlab-org/gitaly/blob/5e94dc966ac1900c11794b107a77496552591f9b/ruby/lib/gitlab/git/repository.rb#L211.
+		// Maybe this belongs in the FindAllTags handler, or even on the gitlab-ce client side, instead of here?
+		body = bytes.TrimRight(split[1], "\n")
 	}
 
-	// Remove trailing newline, if any, to preserve existing behavior the old GitLab tag finding code.
-	// See https://gitlab.com/gitlab-org/gitaly/blob/5e94dc966ac1900c11794b107a77496552591f9b/ruby/lib/gitlab/git/repository.rb#L211.
-	// Maybe this belongs in the FindAllTags handler, or even on the gitlab-ce client side, instead of here?
-	body := bytes.TrimRight(split[1], "\n")
-
 	var header tagHeader
 	s := bufio.NewScanner(bytes.NewReader(split[0]))
 	for s.Scan() {
diff --git a/internal/git/log/tag_test.go b/internal/git/log/tag_test.go
index 6c702910e..13ef06166 100644
--- a/internal/git/log/tag_test.go
+++ b/internal/git/log/tag_test.go
@@ -1,6 +1,7 @@
 package log
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 	"testing"
@@ -8,6 +9,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 )
@@ -53,3 +55,55 @@ func TestGetTag(t *testing.T) {
 		})
 	}
 }
+
+func TestSplitRawTag(t *testing.T) {
+	testCases := []struct {
+		description string
+		tagContent  string
+		header      tagHeader
+		body        []byte
+	}{
+		{
+			description: "tag without a message",
+			tagContent:  "object c92faf3e0a557270141be67f206d7cdb99bfc3a2\ntype commit\ntag v2.6.16.28\ntagger Adrian Bunk <bunk@stusta.de> 1156539089 +0200",
+			header: tagHeader{
+				oid:     "c92faf3e0a557270141be67f206d7cdb99bfc3a2",
+				tagType: "commit",
+			},
+			body: nil,
+		},
+		{
+			description: "tag with message",
+			tagContent:  "object c92faf3e0a557270141be67f206d7cdb99bfc3a2\ntype commit\ntag v2.6.16.28\ntagger Adrian Bunk <bunk@stusta.de> 1156539089 +0200\n\nmessage",
+			header: tagHeader{
+				oid:     "c92faf3e0a557270141be67f206d7cdb99bfc3a2",
+				tagType: "commit",
+			},
+			body: []byte("message"),
+		},
+		{
+			description: "tag with empty message",
+			tagContent:  "object c92faf3e0a557270141be67f206d7cdb99bfc3a2\ntype commit\ntag v2.6.16.28\ntagger Adrian Bunk <bunk@stusta.de> 1156539089 +0200\n\n",
+			header: tagHeader{
+				oid:     "c92faf3e0a557270141be67f206d7cdb99bfc3a2",
+				tagType: "commit",
+			},
+			body: []byte{},
+		},
+		{
+			description: "tag with message with empty line",
+			tagContent:  "object c92faf3e0a557270141be67f206d7cdb99bfc3a2\ntype commit\ntag v2.6.16.28\ntagger Adrian Bunk <bunk@stusta.de> 1156539089 +0200\n\nHello world\n\nThis is a message",
+			header: tagHeader{
+				oid:     "c92faf3e0a557270141be67f206d7cdb99bfc3a2",
+				tagType: "commit",
+			},
+			body: []byte("Hello world\n\nThis is a message"),
+		},
+	}
+	for _, tc := range testCases {
+		header, body, err := splitRawTag(bytes.NewReader([]byte(tc.tagContent)))
+		assert.Equal(t, tc.header, *header)
+		assert.Equal(t, tc.body, body)
+		assert.NoError(t, err)
+	}
+}