Fix path bug in CommitService::FindCommits

3 files changed, 53 insertions, 0 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fb2b83a39..5592f91d6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Gitaly changelog
 
+UNRELEASED
+
+- Fix path bug in CommitService::FindCommits
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/364
+
 v0.42.0
 
 - Implement UserCreateTag RPC
diff --git a/internal/service/commit/find_commits.go b/internal/service/commit/find_commits.go
index 87bb25637..dd489d904 100644
--- a/internal/service/commit/find_commits.go
+++ b/internal/service/commit/find_commits.go
@@ -22,6 +22,13 @@ func (s *server) FindCommits(req *pb.FindCommitsRequest, stream pb.CommitService
 		}
 	}
 
+	// Clients might send empty paths. That is an error
+	for _, path := range req.Paths {
+		if len(path) == 0 {
+			return grpc.Errorf(codes.InvalidArgument, "path is empty string")
+		}
+	}
+
 	client, err := s.CommitServiceClient(ctx)
 	if err != nil {
 		return err
diff --git a/internal/service/commit/find_commits_test.go b/internal/service/commit/find_commits_test.go
index 54f8e83e3..788e3b1ac 100644
--- a/internal/service/commit/find_commits_test.go
+++ b/internal/service/commit/find_commits_test.go
@@ -5,6 +5,8 @@ import (
 	"io"
 	"testing"
 
+	"google.golang.org/grpc/codes"
+
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 
@@ -215,3 +217,42 @@ func TestSuccessfulFindCommitsRequest(t *testing.T) {
 		})
 	}
 }
+
+func TestFailureFindCommitsRequest(t *testing.T) {
+	server := startTestServices(t)
+	defer server.Stop()
+
+	client, conn := newCommitServiceClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testCases := []struct {
+		desc    string
+		request *pb.FindCommitsRequest
+		code    codes.Code
+	}{
+		{
+			desc: "empty path string",
+			request: &pb.FindCommitsRequest{
+				Repository: testRepo,
+				Paths:      [][]byte{[]byte("")},
+			},
+			code: codes.InvalidArgument,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+
+			stream, err := client.FindCommits(ctx, tc.request)
+			require.NoError(t, err)
+
+			for err == nil {
+				_, err = stream.Recv()
+			}
+
+			testhelper.AssertGrpcError(t, err, tc.code, "")
+		})
+	}
+}