diff options
author | Pavlo Strokov <pstrokov@gitlab.com> | 2022-10-20 15:55:49 +0300 |
---|---|---|
committer | Pavlo Strokov <pstrokov@gitlab.com> | 2022-10-24 09:20:15 +0300 |
commit | eed1c3e6be735efec79626d4c5c5c3f566a05934 (patch) | |
tree | 68e97b8ba598c200f9b565b4069ab0bd232ae754 | |
parent | 007e1f7f1cdb4a677ed1f756473a34257057ad3a (diff) |
service/wiki: Improve validation of input
Gitaly should return the same error for all RPCs where the
Repository input parameter is missing.
The test coverage extended to cover changed code.
The error verification is done not only for the code, but for
the message as well. It gives us confidence that a proper path
is tested.
-rw-r--r-- | internal/gitaly/service/wiki/find_page.go | 4 | ||||
-rw-r--r-- | internal/gitaly/service/wiki/find_page_test.go | 31 | ||||
-rw-r--r-- | internal/gitaly/service/wiki/update_page.go | 4 | ||||
-rw-r--r-- | internal/gitaly/service/wiki/write_page.go | 4 |
4 files changed, 29 insertions, 14 deletions
diff --git a/internal/gitaly/service/wiki/find_page.go b/internal/gitaly/service/wiki/find_page.go index ef88be76f..2abb0d231 100644 --- a/internal/gitaly/service/wiki/find_page.go +++ b/internal/gitaly/service/wiki/find_page.go @@ -3,6 +3,7 @@ package wiki import ( "errors" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -44,6 +45,9 @@ func (s *server) WikiFindPage(request *gitalypb.WikiFindPageRequest, stream gita } func validateWikiFindPage(request *gitalypb.WikiFindPageRequest) error { + if request.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevisionAllowEmpty(request.Revision); err != nil { return err } diff --git a/internal/gitaly/service/wiki/find_page_test.go b/internal/gitaly/service/wiki/find_page_test.go index d2222118f..1556955b9 100644 --- a/internal/gitaly/service/wiki/find_page_test.go +++ b/internal/gitaly/service/wiki/find_page_test.go @@ -14,6 +14,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func testSuccessfulWikiFindPageRequest(t *testing.T, cfg config.Cfg, client gitalypb.WikiServiceClient, rubySrv *rubyserver.Server) { @@ -388,29 +389,31 @@ func TestFailedWikiFindPageDueToValidation(t *testing.T) { wikiRepo, _ := setupWikiRepo(t, ctx, cfg) testCases := []struct { - desc string - title string - code codes.Code + desc string + req *gitalypb.WikiFindPageRequest + expErr error }{ { - desc: "empty page path", - title: "", - code: codes.InvalidArgument, + desc: "empty page path", + req: &gitalypb.WikiFindPageRequest{Repository: wikiRepo, Title: nil}, + expErr: status.Error(codes.InvalidArgument, "WikiFindPage: empty Title"), + }, + { + desc: "repository not provided", + req: &gitalypb.WikiFindPageRequest{Repository: nil}, + expErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefect( + "WikiFindPage: empty Repository", + "repo scoped: empty Repository", + )), }, } for _, testCase := range testCases { t.Run(testCase.desc, func(t *testing.T) { - request := &gitalypb.WikiFindPageRequest{ - Repository: wikiRepo, - Title: []byte(testCase.title), - } - - c, err := client.WikiFindPage(ctx, request) + c, err := client.WikiFindPage(ctx, testCase.req) require.NoError(t, err) - err = drainWikiFindPageResponse(c) - testhelper.RequireGrpcCode(t, err, testCase.code) + testhelper.RequireGrpcError(t, testCase.expErr, err) }) } } diff --git a/internal/gitaly/service/wiki/update_page.go b/internal/gitaly/service/wiki/update_page.go index 8849a477a..9122c8d4e 100644 --- a/internal/gitaly/service/wiki/update_page.go +++ b/internal/gitaly/service/wiki/update_page.go @@ -3,6 +3,7 @@ package wiki import ( "fmt" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -61,6 +62,9 @@ func (s *server) WikiUpdatePage(stream gitalypb.WikiService_WikiUpdatePageServer } func validateWikiUpdatePageRequest(request *gitalypb.WikiUpdatePageRequest) error { + if request.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if len(request.GetPagePath()) == 0 { return fmt.Errorf("empty Page Path") } diff --git a/internal/gitaly/service/wiki/write_page.go b/internal/gitaly/service/wiki/write_page.go index db1318e2b..5316e3b51 100644 --- a/internal/gitaly/service/wiki/write_page.go +++ b/internal/gitaly/service/wiki/write_page.go @@ -3,6 +3,7 @@ package wiki import ( "fmt" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/rubyserver" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -61,6 +62,9 @@ func (s *server) WikiWritePage(stream gitalypb.WikiService_WikiWritePageServer) } func validateWikiWritePageRequest(request *gitalypb.WikiWritePageRequest) error { + if request.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if len(request.GetName()) == 0 { return fmt.Errorf("empty Name") } |