Colons are not allowed in refs

2 files changed, 19 insertions, 9 deletions

diff --git a/internal/service/commit/find_commit.go b/internal/service/commit/find_commit.go
index 6c73687a3..4bd4ba744 100644
--- a/internal/service/commit/find_commit.go
+++ b/internal/service/commit/find_commit.go
@@ -30,12 +30,12 @@ func init() {
 }
 
 func (s *server) FindCommit(ctx context.Context, in *pb.FindCommitRequest) (*pb.FindCommitResponse, error) {
-	if err := git.ValidateRevision(in.GetRevision()); err != nil {
+	revision := in.GetRevision()
+	if err := git.ValidateRevision(revision); err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "FindCommit: revision: %v", err)
 	}
 
 	repo := in.GetRepository()
-	revision := in.GetRevision()
 
 	if featureflag.IsEnabled(ctx, "gogit-findcommit") {
 		commit, err := gogitFindCommit(repo, revision)
diff --git a/internal/service/commit/find_commit_test.go b/internal/service/commit/find_commit_test.go
index b0874a1df..003f53f5f 100644
--- a/internal/service/commit/find_commit_test.go
+++ b/internal/service/commit/find_commit_test.go
@@ -15,6 +15,7 @@ import (
 	"golang.org/x/net/context"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
 )
 
 func TestSuccessfulFindCommitRequest(t *testing.T) {
@@ -247,7 +248,7 @@ func TestSuccessfulFindCommitRequest(t *testing.T) {
 		})
 	}
 
-	ctx = metadata.NewOutgoingContext(
+	gogitCtx := metadata.NewOutgoingContext(
 		ctx,
 		metadata.New(map[string]string{featureflag.HeaderKey("gogit-findcommit"): "true"}),
 	)
@@ -258,9 +259,9 @@ func TestSuccessfulFindCommitRequest(t *testing.T) {
 			Revision:   []byte(testCase.revision),
 		}
 
-		response, err := client.FindCommit(ctx, request)
-		require.NoError(t, err)
-		require.Equal(t, allCommits[i], response.Commit)
+		response, err := client.FindCommit(gogitCtx, request)
+		require.NoError(t, err, "request with go-git should succeed")
+		require.Equal(t, allCommits[i], response.Commit, "commit fetched with go-git should match default")
 	}
 }
 
@@ -284,8 +285,16 @@ func TestFailedFindCommitRequest(t *testing.T) {
 		{repo: invalidRepo, revision: []byte("master"), description: "Invalid repo"},
 		{repo: testRepo, revision: []byte(""), description: "Empty revision"},
 		{repo: testRepo, revision: []byte("-master"), description: "Invalid revision"},
+		{repo: testRepo, revision: []byte("mas:ter"), description: "Invalid revision"},
 	}
 
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	gogitCtx := metadata.NewOutgoingContext(
+		ctx,
+		metadata.New(map[string]string{featureflag.HeaderKey("gogit-findcommit"): "true"}),
+	)
+
 	for _, testCase := range testCases {
 		t.Run(testCase.description, func(t *testing.T) {
 			request := &pb.FindCommitRequest{
@@ -293,10 +302,11 @@ func TestFailedFindCommitRequest(t *testing.T) {
 				Revision:   testCase.revision,
 			}
 
-			ctx, cancel := context.WithCancel(context.Background())
-			defer cancel()
 			_, err := client.FindCommit(ctx, request)
-			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, "")
+			require.Equal(t, codes.InvalidArgument, status.Code(err), "default lookup should fail")
+
+			_, err = client.FindCommit(gogitCtx, request)
+			require.Equal(t, codes.InvalidArgument, status.Code(err), "go-git lookup should fail")
 		})
 	}
 }