diff options
115 files changed, 533 insertions, 146 deletions
diff --git a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go index 84b39aca9..9b65eae67 100644 --- a/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go +++ b/internal/gitaly/service/cleanup/apply_bfg_object_map_stream.go @@ -1,9 +1,9 @@ package cleanup import ( - "fmt" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/cleanup/internalrefs" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/cleanup/notifier" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -66,7 +66,7 @@ func (s *server) ApplyBfgObjectMapStream(server gitalypb.CleanupService_ApplyBfg func validateFirstRequest(req *gitalypb.ApplyBfgObjectMapStreamRequest) error { if repo := req.GetRepository(); repo == nil { - return fmt.Errorf("first request: repository not set") + return gitalyerrors.ErrEmptyRepository } return nil diff --git a/internal/gitaly/service/commit/check_objects_exist.go b/internal/gitaly/service/commit/check_objects_exist.go index 04b92be89..2a43ed0fc 100644 --- a/internal/gitaly/service/commit/check_objects_exist.go +++ b/internal/gitaly/service/commit/check_objects_exist.go @@ -4,6 +4,7 @@ import ( "context" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -29,7 +30,7 @@ func (s *server) CheckObjectsExist( } if request.GetRepository() == nil { - return helper.ErrInvalidArgumentf("empty Repository") + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } objectInfoReader, cancel, err := s.catfileCache.ObjectInfoReader( diff --git a/internal/gitaly/service/commit/commit_messages.go b/internal/gitaly/service/commit/commit_messages.go index da82f92d6..3417f509a 100644 --- a/internal/gitaly/service/commit/commit_messages.go +++ b/internal/gitaly/service/commit/commit_messages.go @@ -5,6 +5,7 @@ import ( "fmt" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -56,7 +57,7 @@ func (s *server) getAndStreamCommitMessages(request *gitalypb.GetCommitMessagesR func validateGetCommitMessagesRequest(request *gitalypb.GetCommitMessagesRequest) error { if request.GetRepository() == nil { - return fmt.Errorf("empty Repository") + return gitalyerrors.ErrEmptyRepository } return nil diff --git a/internal/gitaly/service/commit/commit_signatures.go b/internal/gitaly/service/commit/commit_signatures.go index 9045e428c..ffcb0adc8 100644 --- a/internal/gitaly/service/commit/commit_signatures.go +++ b/internal/gitaly/service/commit/commit_signatures.go @@ -7,20 +7,19 @@ import ( "fmt" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) var gpgSiganturePrefix = []byte("gpgsig") func (s *server) GetCommitSignatures(request *gitalypb.GetCommitSignaturesRequest, stream gitalypb.CommitService_GetCommitSignaturesServer) error { if err := validateGetCommitSignaturesRequest(request); err != nil { - return status.Errorf(codes.InvalidArgument, "GetCommitSignatures: %v", err) + return helper.ErrInvalidArgumentf("GetCommitSignatures: %w", err) } return s.getCommitSignatures(request, stream) @@ -129,7 +128,7 @@ func sendResponse(commitID string, signatureKey []byte, commitText []byte, strea func validateGetCommitSignaturesRequest(request *gitalypb.GetCommitSignaturesRequest) error { if request.GetRepository() == nil { - return errors.New("empty Repository") + return gitalyerrors.ErrEmptyRepository } if len(request.GetCommitIds()) == 0 { diff --git a/internal/gitaly/service/commit/commits_by_message.go b/internal/gitaly/service/commit/commits_by_message.go index 021141304..1eaeb52c7 100644 --- a/internal/gitaly/service/commit/commits_by_message.go +++ b/internal/gitaly/service/commit/commits_by_message.go @@ -3,6 +3,7 @@ package commit import ( "fmt" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -69,6 +70,10 @@ func (s *server) commitsByMessage(in *gitalypb.CommitsByMessageRequest, stream g } func validateCommitsByMessageRequest(in *gitalypb.CommitsByMessageRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if err := git.ValidateRevisionAllowEmpty(in.Revision); err != nil { return err } diff --git a/internal/gitaly/service/commit/count_commits.go b/internal/gitaly/service/commit/count_commits.go index 12c0072e1..1d2807193 100644 --- a/internal/gitaly/service/commit/count_commits.go +++ b/internal/gitaly/service/commit/count_commits.go @@ -9,6 +9,7 @@ import ( "time" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -76,6 +77,10 @@ func (s *server) CountCommits(ctx context.Context, in *gitalypb.CountCommitsRequ } func validateCountCommitsRequest(in *gitalypb.CountCommitsRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if err := git.ValidateRevisionAllowEmpty(in.Revision); err != nil { return err } diff --git a/internal/gitaly/service/commit/count_diverging_commits.go b/internal/gitaly/service/commit/count_diverging_commits.go index a6d6ea27a..d47671ee9 100644 --- a/internal/gitaly/service/commit/count_diverging_commits.go +++ b/internal/gitaly/service/commit/count_diverging_commits.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -36,7 +37,7 @@ func (s *server) validateCountDivergingCommitsRequest(req *gitalypb.CountDivergi } if req.GetRepository() == nil { - return errors.New("repository is empty") + return gitalyerrors.ErrEmptyRepository } if _, err := s.locator.GetRepoPath(req.GetRepository()); err != nil { diff --git a/internal/gitaly/service/commit/filter_shas_with_signatures.go b/internal/gitaly/service/commit/filter_shas_with_signatures.go index 02e7b14df..b71f7fd54 100644 --- a/internal/gitaly/service/commit/filter_shas_with_signatures.go +++ b/internal/gitaly/service/commit/filter_shas_with_signatures.go @@ -2,9 +2,9 @@ package commit import ( "context" - "errors" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -28,8 +28,8 @@ func (s *server) FilterShasWithSignatures(bidi gitalypb.CommitService_FilterShas } func validateFirstFilterShasWithSignaturesRequest(in *gitalypb.FilterShasWithSignaturesRequest) error { - if in.Repository == nil { - return errors.New("no repository given") + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository } return nil } diff --git a/internal/gitaly/service/commit/filter_shas_with_signatures_test.go b/internal/gitaly/service/commit/filter_shas_with_signatures_test.go index ddd59f018..56ad29770 100644 --- a/internal/gitaly/service/commit/filter_shas_with_signatures_test.go +++ b/internal/gitaly/service/commit/filter_shas_with_signatures_test.go @@ -62,7 +62,7 @@ func TestFilterShasWithSignaturesSuccessful(t *testing.T) { func TestFilterShasWithSignaturesValidationError(t *testing.T) { t.Parallel() err := validateFirstFilterShasWithSignaturesRequest(&gitalypb.FilterShasWithSignaturesRequest{}) - require.Contains(t, err.Error(), "no repository given") + require.Contains(t, err.Error(), "empty Repository") } func recvFSWS(stream gitalypb.CommitService_FilterShasWithSignaturesClient) ([][]byte, error) { diff --git a/internal/gitaly/service/commit/find_all_commits.go b/internal/gitaly/service/commit/find_all_commits.go index 837ff1318..573894744 100644 --- a/internal/gitaly/service/commit/find_all_commits.go +++ b/internal/gitaly/service/commit/find_all_commits.go @@ -3,6 +3,7 @@ package commit import ( "fmt" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -10,7 +11,7 @@ import ( func (s *server) FindAllCommits(in *gitalypb.FindAllCommitsRequest, stream gitalypb.CommitService_FindAllCommitsServer) error { if err := validateFindAllCommitsRequest(in); err != nil { - return err + return helper.ErrInvalidArgument(err) } ctx := stream.Context() @@ -39,8 +40,12 @@ func (s *server) FindAllCommits(in *gitalypb.FindAllCommitsRequest, stream gital } func validateFindAllCommitsRequest(in *gitalypb.FindAllCommitsRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if err := git.ValidateRevisionAllowEmpty(in.Revision); err != nil { - return helper.ErrInvalidArgument(err) + return err } return nil diff --git a/internal/gitaly/service/commit/find_commit.go b/internal/gitaly/service/commit/find_commit.go index 05705a0ae..4b736452c 100644 --- a/internal/gitaly/service/commit/find_commit.go +++ b/internal/gitaly/service/commit/find_commit.go @@ -4,18 +4,27 @@ import ( "context" "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/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) +func validateFindCommitRequest(in *gitalypb.FindCommitRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if err := git.ValidateRevision(in.GetRevision()); err != nil { + return err + } + return nil +} + func (s *server) FindCommit(ctx context.Context, in *gitalypb.FindCommitRequest) (*gitalypb.FindCommitResponse, error) { - revision := in.GetRevision() - if err := git.ValidateRevision(revision); err != nil { + if err := validateFindCommitRequest(in); err != nil { return nil, helper.ErrInvalidArgument(err) } - repo := s.localrepo(in.GetRepository()) var opts []localrepo.ReadCommitOpt @@ -23,7 +32,7 @@ func (s *server) FindCommit(ctx context.Context, in *gitalypb.FindCommitRequest) opts = []localrepo.ReadCommitOpt{localrepo.WithTrailers()} } - commit, err := repo.ReadCommit(ctx, git.Revision(revision), opts...) + commit, err := repo.ReadCommit(ctx, git.Revision(in.GetRevision()), opts...) if err != nil { if errors.Is(err, localrepo.ErrObjectNotFound) { return &gitalypb.FindCommitResponse{}, nil diff --git a/internal/gitaly/service/commit/find_commits.go b/internal/gitaly/service/commit/find_commits.go index 13907b10a..1b2aa7117 100644 --- a/internal/gitaly/service/commit/find_commits.go +++ b/internal/gitaly/service/commit/find_commits.go @@ -12,6 +12,7 @@ import ( "strings" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/trailerparser" @@ -22,10 +23,20 @@ import ( var statsPattern = regexp.MustCompile(`\s(\d+)\sfiles? changed(,\s(\d+)\sinsertions?\(\+\))?(,\s(\d+)\sdeletions?\(-\))?`) +func validateFindCommitsRequest(in *gitalypb.FindCommitsRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if err := git.ValidateRevisionAllowEmpty(in.GetRevision()); err != nil { + return err + } + return nil +} + func (s *server) FindCommits(req *gitalypb.FindCommitsRequest, stream gitalypb.CommitService_FindCommitsServer) error { ctx := stream.Context() - if err := git.ValidateRevisionAllowEmpty(req.Revision); err != nil { + if err := validateFindCommitsRequest(req); err != nil { return helper.ErrInvalidArgument(err) } diff --git a/internal/gitaly/service/commit/isancestor.go b/internal/gitaly/service/commit/isancestor.go index 3fa8bdccc..a4b993da5 100644 --- a/internal/gitaly/service/commit/isancestor.go +++ b/internal/gitaly/service/commit/isancestor.go @@ -2,21 +2,34 @@ package commit import ( "context" + "errors" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" log "github.com/sirupsen/logrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) -func (s *server) CommitIsAncestor(ctx context.Context, in *gitalypb.CommitIsAncestorRequest) (*gitalypb.CommitIsAncestorResponse, error) { - if in.AncestorId == "" { - return nil, status.Errorf(codes.InvalidArgument, "Bad Request (empty ancestor sha)") +func validateCommitIsAncestorRequest(in *gitalypb.CommitIsAncestorRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if in.GetAncestorId() == "" { + return errors.New("Bad Request (empty ancestor sha)") //nolint:stylecheck + } + if in.GetChildId() == "" { + return errors.New("Bad Request (empty child sha)") //nolint:stylecheck } - if in.ChildId == "" { - return nil, status.Errorf(codes.InvalidArgument, "Bad Request (empty child sha)") + return nil +} + +func (s *server) CommitIsAncestor(ctx context.Context, in *gitalypb.CommitIsAncestorRequest) (*gitalypb.CommitIsAncestorResponse, error) { + if err := validateCommitIsAncestorRequest(in); err != nil { + return nil, helper.ErrInvalidArgument(err) } ret, err := s.commitIsAncestorName(ctx, in.Repository, in.AncestorId, in.ChildId) diff --git a/internal/gitaly/service/commit/languages.go b/internal/gitaly/service/commit/languages.go index 4363011c0..67b9d04a7 100644 --- a/internal/gitaly/service/commit/languages.go +++ b/internal/gitaly/service/commit/languages.go @@ -9,6 +9,7 @@ import ( "sort" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" @@ -17,8 +18,18 @@ import ( var errAmbigRef = errors.New("ambiguous reference") -func (s *server) CommitLanguages(ctx context.Context, req *gitalypb.CommitLanguagesRequest) (*gitalypb.CommitLanguagesResponse, error) { +func (s *server) validateCommitLanguagesRequest(req *gitalypb.CommitLanguagesRequest) error { + if req.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevisionAllowEmpty(req.Revision); err != nil { + return err + } + return nil +} + +func (s *server) CommitLanguages(ctx context.Context, req *gitalypb.CommitLanguagesRequest) (*gitalypb.CommitLanguagesResponse, error) { + if err := s.validateCommitLanguagesRequest(req); err != nil { return nil, helper.ErrInvalidArgument(err) } diff --git a/internal/gitaly/service/commit/last_commit_for_path.go b/internal/gitaly/service/commit/last_commit_for_path.go index 48e87c726..0ed2eb3e3 100644 --- a/internal/gitaly/service/commit/last_commit_for_path.go +++ b/internal/gitaly/service/commit/last_commit_for_path.go @@ -3,6 +3,7 @@ package commit import ( "context" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/log" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -57,8 +58,11 @@ func (s *server) lastCommitForPath(ctx context.Context, in *gitalypb.LastCommitF } func validateLastCommitForPathRequest(in *gitalypb.LastCommitForPathRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevision(in.Revision); err != nil { - return helper.ErrInvalidArgument(err) + return err } return nil } diff --git a/internal/gitaly/service/commit/last_commit_for_path_test.go b/internal/gitaly/service/commit/last_commit_for_path_test.go index 84a7297e5..7cc171413 100644 --- a/internal/gitaly/service/commit/last_commit_for_path_test.go +++ b/internal/gitaly/service/commit/last_commit_for_path_test.go @@ -103,7 +103,7 @@ func TestFailedLastCommitForPathRequest(t *testing.T) { Revision: []byte("some-branch"), }, expectedErr: helper.ErrInvalidArgumentf(gitalyOrPraefect( - "GetStorageByName: no such storage: \"\"", + "empty Repository", "repo scoped: empty Repository", )), }, diff --git a/internal/gitaly/service/commit/list_all_commits.go b/internal/gitaly/service/commit/list_all_commits.go index ff368f6d7..9cba824b8 100644 --- a/internal/gitaly/service/commit/list_all_commits.go +++ b/internal/gitaly/service/commit/list_all_commits.go @@ -1,9 +1,9 @@ package commit import ( - "errors" "fmt" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" @@ -14,7 +14,7 @@ import ( func verifyListAllCommitsRequest(request *gitalypb.ListAllCommitsRequest) error { if request.GetRepository() == nil { - return errors.New("empty repository") + return gitalyerrors.ErrEmptyRepository } return nil } diff --git a/internal/gitaly/service/commit/list_commits.go b/internal/gitaly/service/commit/list_commits.go index 7fc5858fe..cff5cd83a 100644 --- a/internal/gitaly/service/commit/list_commits.go +++ b/internal/gitaly/service/commit/list_commits.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" @@ -15,7 +16,7 @@ import ( func verifyListCommitsRequest(request *gitalypb.ListCommitsRequest) error { if request.GetRepository() == nil { - return errors.New("empty repository") + return gitalyerrors.ErrEmptyRepository } if len(request.GetRevisions()) == 0 { return errors.New("missing revisions") diff --git a/internal/gitaly/service/commit/list_commits_by_oid.go b/internal/gitaly/service/commit/list_commits_by_oid.go index 9bd423a8e..fa830121b 100644 --- a/internal/gitaly/service/commit/list_commits_by_oid.go +++ b/internal/gitaly/service/commit/list_commits_by_oid.go @@ -3,8 +3,10 @@ package commit import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/protobuf/proto" @@ -23,6 +25,9 @@ var listCommitsbyOidHistogram = promauto.NewHistogram( func (s *server) ListCommitsByOid(in *gitalypb.ListCommitsByOidRequest, stream gitalypb.CommitService_ListCommitsByOidServer) error { ctx := stream.Context() + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repo := s.localrepo(in.GetRepository()) objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) diff --git a/internal/gitaly/service/commit/list_commits_by_ref_name.go b/internal/gitaly/service/commit/list_commits_by_ref_name.go index 77c8b3e61..12b866730 100644 --- a/internal/gitaly/service/commit/list_commits_by_ref_name.go +++ b/internal/gitaly/service/commit/list_commits_by_ref_name.go @@ -1,6 +1,7 @@ package commit import ( + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -11,6 +12,9 @@ import ( func (s *server) ListCommitsByRefName(in *gitalypb.ListCommitsByRefNameRequest, stream gitalypb.CommitService_ListCommitsByRefNameServer) error { ctx := stream.Context() + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repo := s.localrepo(in.GetRepository()) objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) diff --git a/internal/gitaly/service/commit/list_files.go b/internal/gitaly/service/commit/list_files.go index 9a1da2197..5ba492fe4 100644 --- a/internal/gitaly/service/commit/list_files.go +++ b/internal/gitaly/service/commit/list_files.go @@ -5,6 +5,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" log "github.com/sirupsen/logrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/lstree" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -19,7 +20,7 @@ func (s *server) ListFiles(in *gitalypb.ListFilesRequest, stream gitalypb.Commit }).Debug("ListFiles") if err := validateListFilesRequest(in); err != nil { - return err + return helper.ErrInvalidArgument(err) } repo := s.localrepo(in.GetRepository()) @@ -58,8 +59,11 @@ func (s *server) ListFiles(in *gitalypb.ListFilesRequest, stream gitalypb.Commit } func validateListFilesRequest(in *gitalypb.ListFilesRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevisionAllowEmpty(in.Revision); err != nil { - return helper.ErrInvalidArgument(err) + return err } return nil } diff --git a/internal/gitaly/service/commit/list_last_commits_for_tree.go b/internal/gitaly/service/commit/list_last_commits_for_tree.go index 7e1c35b83..ba3f6535e 100644 --- a/internal/gitaly/service/commit/list_last_commits_for_tree.go +++ b/internal/gitaly/service/commit/list_last_commits_for_tree.go @@ -6,6 +6,7 @@ import ( "sort" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/log" "gitlab.com/gitlab-org/gitaly/v15/internal/git/lstree" @@ -139,6 +140,9 @@ func sendCommitsForTree(batch []*gitalypb.ListLastCommitsForTreeResponse_CommitF } func validateListLastCommitsForTreeRequest(in *gitalypb.ListLastCommitsForTreeRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevision([]byte(in.Revision)); err != nil { return err } diff --git a/internal/gitaly/service/commit/raw_blame.go b/internal/gitaly/service/commit/raw_blame.go index 41c3c83c8..ebaf02c70 100644 --- a/internal/gitaly/service/commit/raw_blame.go +++ b/internal/gitaly/service/commit/raw_blame.go @@ -6,6 +6,7 @@ import ( "regexp" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -60,6 +61,9 @@ func (s *server) RawBlame(in *gitalypb.RawBlameRequest, stream gitalypb.CommitSe } func validateRawBlameRequest(in *gitalypb.RawBlameRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevision(in.Revision); err != nil { return err } diff --git a/internal/gitaly/service/commit/stats.go b/internal/gitaly/service/commit/stats.go index 1b9ba0286..1a9873f4a 100644 --- a/internal/gitaly/service/commit/stats.go +++ b/internal/gitaly/service/commit/stats.go @@ -7,13 +7,24 @@ import ( "strconv" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) -func (s *server) CommitStats(ctx context.Context, in *gitalypb.CommitStatsRequest) (*gitalypb.CommitStatsResponse, error) { +func validateCommitStatsRequest(in *gitalypb.CommitStatsRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevision(in.Revision); err != nil { + return err + } + return nil +} + +func (s *server) CommitStats(ctx context.Context, in *gitalypb.CommitStatsRequest) (*gitalypb.CommitStatsResponse, error) { + if err := validateCommitStatsRequest(in); err != nil { return nil, helper.ErrInvalidArgument(err) } diff --git a/internal/gitaly/service/commit/tree_entries.go b/internal/gitaly/service/commit/tree_entries.go index 2a2489526..ceb52cdcf 100644 --- a/internal/gitaly/service/commit/tree_entries.go +++ b/internal/gitaly/service/commit/tree_entries.go @@ -8,6 +8,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" log "github.com/sirupsen/logrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" @@ -24,6 +25,9 @@ const ( ) func validateGetTreeEntriesRequest(in *gitalypb.GetTreeEntriesRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevision(in.Revision); err != nil { return err } diff --git a/internal/gitaly/service/commit/tree_entries_test.go b/internal/gitaly/service/commit/tree_entries_test.go index e763619a4..c9ba3b40a 100644 --- a/internal/gitaly/service/commit/tree_entries_test.go +++ b/internal/gitaly/service/commit/tree_entries_test.go @@ -674,7 +674,7 @@ func TestGetTreeEntries_validation(t *testing.T) { Path: path, }, expectedErr: helper.ErrInvalidArgumentf(gitalyOrPraefect( - "GetStorageByName: no such storage: \"\"", + "TreeEntry: empty Repository", "repo scoped: empty Repository", )), }, diff --git a/internal/gitaly/service/commit/tree_entry.go b/internal/gitaly/service/commit/tree_entry.go index 4ab6afb3a..c3216c15f 100644 --- a/internal/gitaly/service/commit/tree_entry.go +++ b/internal/gitaly/service/commit/tree_entry.go @@ -5,6 +5,7 @@ import ( "io" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -148,6 +149,9 @@ func (s *server) TreeEntry(in *gitalypb.TreeEntryRequest, stream gitalypb.Commit } func validateRequest(in *gitalypb.TreeEntryRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevision(in.Revision); err != nil { return err } diff --git a/internal/gitaly/service/commit/tree_entry_test.go b/internal/gitaly/service/commit/tree_entry_test.go index 8d408fc48..d2b0c0047 100644 --- a/internal/gitaly/service/commit/tree_entry_test.go +++ b/internal/gitaly/service/commit/tree_entry_test.go @@ -177,7 +177,7 @@ func TestFailedTreeEntry(t *testing.T) { name: "Repository is nil", req: &gitalypb.TreeEntryRequest{Repository: nil, Revision: revision, Path: path}, expectedErr: helper.ErrInvalidArgumentf(gitalyOrPraefect( - "GetStorageByName: no such storage: \"\"", + "TreeEntry: empty Repository", "repo scoped: empty Repository", )), }, diff --git a/internal/gitaly/service/conflicts/list_conflict_files.go b/internal/gitaly/service/conflicts/list_conflict_files.go index c0316e5eb..427603524 100644 --- a/internal/gitaly/service/conflicts/list_conflict_files.go +++ b/internal/gitaly/service/conflicts/list_conflict_files.go @@ -7,6 +7,7 @@ import ( "io" "unicode/utf8" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -127,7 +128,7 @@ func (s *server) ListConflictFiles(request *gitalypb.ListConflictFilesRequest, s func validateListConflictFilesRequest(in *gitalypb.ListConflictFilesRequest) error { if in.GetRepository() == nil { - return fmt.Errorf("empty Repository") + return gitalyerrors.ErrEmptyRepository } if in.GetOurCommitOid() == "" { return fmt.Errorf("empty OurCommitOid") diff --git a/internal/gitaly/service/conflicts/resolve_conflicts.go b/internal/gitaly/service/conflicts/resolve_conflicts.go index 11de0412f..72ed7f94e 100644 --- a/internal/gitaly/service/conflicts/resolve_conflicts.go +++ b/internal/gitaly/service/conflicts/resolve_conflicts.go @@ -12,6 +12,7 @@ import ( "time" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/conflict" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" @@ -80,7 +81,7 @@ func validateResolveConflictsHeader(header *gitalypb.ResolveConflictsRequestHead return fmt.Errorf("empty OurCommitOid") } if header.GetRepository() == nil { - return fmt.Errorf("empty Repository") + return gitalyerrors.ErrEmptyRepository } if header.GetTargetRepository() == nil { return fmt.Errorf("empty TargetRepository") diff --git a/internal/gitaly/service/diff/commit.go b/internal/gitaly/service/diff/commit.go index de9258115..8c718dcc4 100644 --- a/internal/gitaly/service/diff/commit.go +++ b/internal/gitaly/service/diff/commit.go @@ -6,6 +6,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" log "github.com/sirupsen/logrus" + 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/diff" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -14,6 +15,7 @@ import ( ) type requestWithLeftRightCommitIds interface { + GetRepository() *gitalypb.Repository GetLeftCommitId() string GetRightCommitId() string } @@ -197,6 +199,9 @@ func (s *server) CommitDelta(in *gitalypb.CommitDeltaRequest, stream gitalypb.Di } func validateRequest(in requestWithLeftRightCommitIds) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if in.GetLeftCommitId() == "" { return fmt.Errorf("empty LeftCommitId") } diff --git a/internal/gitaly/service/diff/find_changed_paths.go b/internal/gitaly/service/diff/find_changed_paths.go index 93016168a..7420f9ab2 100644 --- a/internal/gitaly/service/diff/find_changed_paths.go +++ b/internal/gitaly/service/diff/find_changed_paths.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -195,6 +196,9 @@ func resolveObjectWithType(ctx context.Context, repo *localrepo.Repo, revision s func (s *server) validateFindChangedPathsRequestParams(ctx context.Context, in *gitalypb.FindChangedPathsRequest) error { repo := in.GetRepository() + if repo == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if _, err := s.locator.GetRepoPath(repo); err != nil { return err } diff --git a/internal/gitaly/service/diff/numstat.go b/internal/gitaly/service/diff/numstat.go index db6442ed0..ed9290ea2 100644 --- a/internal/gitaly/service/diff/numstat.go +++ b/internal/gitaly/service/diff/numstat.go @@ -3,8 +3,10 @@ package diff import ( "io" + 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/diff" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -81,6 +83,9 @@ func sendStats(batch []*gitalypb.DiffStats, stream gitalypb.DiffService_DiffStat func (s *server) validateDiffStatsRequestParams(in *gitalypb.DiffStatsRequest) error { repo := in.GetRepository() + if repo == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if _, err := s.locator.GetRepoPath(repo); err != nil { return err } diff --git a/internal/gitaly/service/hook/pack_objects.go b/internal/gitaly/service/hook/pack_objects.go index 3028a46bc..40afe5195 100644 --- a/internal/gitaly/service/hook/pack_objects.go +++ b/internal/gitaly/service/hook/pack_objects.go @@ -18,6 +18,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/sirupsen/logrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" @@ -293,7 +294,7 @@ func bufferStdin(r io.Reader, h hash.Hash) (_ io.ReadCloser, err error) { func (s *server) PackObjectsHookWithSidechannel(ctx context.Context, req *gitalypb.PackObjectsHookWithSidechannelRequest) (*gitalypb.PackObjectsHookWithSidechannelResponse, error) { if req.GetRepository() == nil { - return nil, helper.ErrInvalidArgument(errors.New("repository is empty")) + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } args, err := parsePackObjectsArgs(req.Args) diff --git a/internal/gitaly/service/hook/post_receive.go b/internal/gitaly/service/hook/post_receive.go index 88e2d15a0..713c65849 100644 --- a/internal/gitaly/service/hook/post_receive.go +++ b/internal/gitaly/service/hook/post_receive.go @@ -6,6 +6,7 @@ import ( "os/exec" "sync" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -67,7 +68,7 @@ func (s *server) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServ func validatePostReceiveHookRequest(in *gitalypb.PostReceiveHookRequest) error { if in.GetRepository() == nil { - return errors.New("repository is empty") + return gitalyerrors.ErrEmptyRepository } return nil diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go index 442af0e26..e30741737 100644 --- a/internal/gitaly/service/hook/pre_receive.go +++ b/internal/gitaly/service/hook/pre_receive.go @@ -6,6 +6,7 @@ import ( "os/exec" "sync" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -57,7 +58,7 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer func validatePreReceiveHookRequest(in *gitalypb.PreReceiveHookRequest) error { if in.GetRepository() == nil { - return errors.New("repository is empty") + return gitalyerrors.ErrEmptyRepository } return nil diff --git a/internal/gitaly/service/hook/reference_transaction.go b/internal/gitaly/service/hook/reference_transaction.go index e01841587..8078b5541 100644 --- a/internal/gitaly/service/hook/reference_transaction.go +++ b/internal/gitaly/service/hook/reference_transaction.go @@ -3,6 +3,7 @@ package hook import ( "errors" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -14,7 +15,7 @@ import ( func validateReferenceTransactionHookRequest(in *gitalypb.ReferenceTransactionHookRequest) error { if in.GetRepository() == nil { - return errors.New("repository is empty") + return gitalyerrors.ErrEmptyRepository } return nil diff --git a/internal/gitaly/service/hook/update.go b/internal/gitaly/service/hook/update.go index 81f0a6a46..dcbd032b7 100644 --- a/internal/gitaly/service/hook/update.go +++ b/internal/gitaly/service/hook/update.go @@ -5,6 +5,7 @@ import ( "os/exec" "sync" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -12,7 +13,7 @@ import ( func validateUpdateHookRequest(in *gitalypb.UpdateHookRequest) error { if in.GetRepository() == nil { - return errors.New("repository is empty") + return gitalyerrors.ErrEmptyRepository } return nil diff --git a/internal/gitaly/service/objectpool/alternates.go b/internal/gitaly/service/objectpool/alternates.go index 8f8703114..e7b3480b6 100644 --- a/internal/gitaly/service/objectpool/alternates.go +++ b/internal/gitaly/service/objectpool/alternates.go @@ -2,7 +2,6 @@ package objectpool import ( "context" - "errors" "fmt" "os" "path/filepath" @@ -11,6 +10,7 @@ import ( "time" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -27,7 +27,7 @@ import ( // backed-up copy of objects/info/alternates. func (s *server) DisconnectGitAlternates(ctx context.Context, req *gitalypb.DisconnectGitAlternatesRequest) (*gitalypb.DisconnectGitAlternatesResponse, error) { if req.GetRepository() == nil { - return nil, helper.ErrInvalidArgument(errors.New("no repository")) + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } repo := s.localrepo(req.GetRepository()) diff --git a/internal/gitaly/service/objectpool/get.go b/internal/gitaly/service/objectpool/get.go index 5a7ada7c6..2986a1eb0 100644 --- a/internal/gitaly/service/objectpool/get.go +++ b/internal/gitaly/service/objectpool/get.go @@ -2,9 +2,9 @@ package objectpool import ( "context" - "errors" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git/objectpool" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -12,7 +12,7 @@ import ( func (s *server) GetObjectPool(ctx context.Context, in *gitalypb.GetObjectPoolRequest) (*gitalypb.GetObjectPoolResponse, error) { if in.GetRepository() == nil { - return nil, helper.ErrInternal(errors.New("repository is empty")) + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } repo := s.localrepo(in.GetRepository()) diff --git a/internal/gitaly/service/operations/apply_patch.go b/internal/gitaly/service/operations/apply_patch.go index c86297abc..8d6a41ed2 100644 --- a/internal/gitaly/service/operations/apply_patch.go +++ b/internal/gitaly/service/operations/apply_patch.go @@ -10,6 +10,7 @@ import ( "time" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -197,7 +198,7 @@ func (s *Server) userApplyPatch(ctx context.Context, header *gitalypb.UserApplyP func validateUserApplyPatchHeader(header *gitalypb.UserApplyPatchRequest_Header) error { if header.GetRepository() == nil { - return fmt.Errorf("missing Repository") + return gitalyerrors.ErrEmptyRepository } if header.GetUser() == nil { diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go index e1a38ab82..fd62f7d87 100644 --- a/internal/gitaly/service/operations/apply_patch_test.go +++ b/internal/gitaly/service/operations/apply_patch_test.go @@ -709,7 +709,7 @@ func TestFailedValidationUserApplyPatch(t *testing.T) { }{ { desc: "missing Repository", - errorMessage: "missing Repository", + errorMessage: "empty Repository", branchName: "new-branch", user: gittest.TestUser, }, diff --git a/internal/gitaly/service/operations/branches.go b/internal/gitaly/service/operations/branches.go index adaa0442d..ac4e7fb7e 100644 --- a/internal/gitaly/service/operations/branches.go +++ b/internal/gitaly/service/operations/branches.go @@ -4,6 +4,7 @@ import ( "context" "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/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" @@ -14,20 +15,27 @@ import ( "google.golang.org/grpc/status" ) -//nolint: stylecheck // This is unintentionally missing documentation. -func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateBranchRequest) (*gitalypb.UserCreateBranchResponse, error) { - if len(req.BranchName) == 0 { - return nil, status.Errorf(codes.InvalidArgument, "Bad Request (empty branch name)") +func validateUserCreateBranchRequest(in *gitalypb.UserCreateBranchRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository } - - if req.User == nil { - return nil, status.Errorf(codes.InvalidArgument, "empty user") + if len(in.BranchName) == 0 { + return errors.New("Bad Request (empty branch name)") //nolint:stylecheck } - - if len(req.StartPoint) == 0 { - return nil, status.Errorf(codes.InvalidArgument, "empty start point") + if in.User == nil { + return errors.New("empty user") + } + if len(in.StartPoint) == 0 { + return errors.New("empty start point") } + return nil +} +//nolint: stylecheck // This is unintentionally missing documentation. +func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateBranchRequest) (*gitalypb.UserCreateBranchResponse, error) { + if err := validateUserCreateBranchRequest(req); err != nil { + return nil, helper.ErrInvalidArgument(err) + } quarantineDir, quarantineRepo, err := s.quarantinedRepo(ctx, req.GetRepository()) if err != nil { return nil, err @@ -105,20 +113,24 @@ func (s *Server) UserCreateBranch(ctx context.Context, req *gitalypb.UserCreateB } func validateUserUpdateBranchGo(req *gitalypb.UserUpdateBranchRequest) error { + if req.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if req.User == nil { - return status.Errorf(codes.InvalidArgument, "empty user") + return errors.New("empty user") } if len(req.BranchName) == 0 { - return status.Errorf(codes.InvalidArgument, "empty branch name") + return errors.New("empty branch name") } if len(req.Oldrev) == 0 { - return status.Errorf(codes.InvalidArgument, "empty oldrev") + return errors.New("empty oldrev") } if len(req.Newrev) == 0 { - return status.Errorf(codes.InvalidArgument, "empty newrev") + return errors.New("empty newrev") } return nil @@ -128,7 +140,7 @@ func validateUserUpdateBranchGo(req *gitalypb.UserUpdateBranchRequest) error { func (s *Server) UserUpdateBranch(ctx context.Context, req *gitalypb.UserUpdateBranchRequest) (*gitalypb.UserUpdateBranchResponse, error) { // Validate the request if err := validateUserUpdateBranchGo(req); err != nil { - return nil, err + return nil, helper.ErrInvalidArgument(err) } newOID, err := git.ObjectHashSHA1.FromHex(string(req.Newrev)) @@ -168,17 +180,25 @@ func (s *Server) UserUpdateBranch(ctx context.Context, req *gitalypb.UserUpdateB return &gitalypb.UserUpdateBranchResponse{}, nil } +func validateUserDeleteBranchRequest(in *gitalypb.UserDeleteBranchRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if len(in.GetBranchName()) == 0 { + return errors.New("bad request: empty branch name") + } + if in.GetUser() == nil { + return errors.New("bad request: empty user") + } + return nil +} + // UserDeleteBranch force-deletes a single branch in the context of a specific user. It executes // hooks and contacts Rails to verify that the user is indeed allowed to delete that branch. func (s *Server) UserDeleteBranch(ctx context.Context, req *gitalypb.UserDeleteBranchRequest) (*gitalypb.UserDeleteBranchResponse, error) { - if len(req.BranchName) == 0 { - return nil, helper.ErrInvalidArgumentf("bad request: empty branch name") - } - - if req.User == nil { - return nil, helper.ErrInvalidArgumentf("bad request: empty user") + if err := validateUserDeleteBranchRequest(req); err != nil { + return nil, helper.ErrInvalidArgument(err) } - referenceName := git.NewReferenceNameFromBranchName(string(req.BranchName)) referenceValue, err := s.localrepo(req.GetRepository()).ResolveRevision(ctx, referenceName.Revision()) diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go index 0637ee021..368100bbf 100644 --- a/internal/gitaly/service/operations/commit_files.go +++ b/internal/gitaly/service/operations/commit_files.go @@ -11,6 +11,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "github.com/sirupsen/logrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/remoterepo" @@ -420,7 +421,7 @@ func (s *Server) fetchMissingCommit( func validateUserCommitFilesHeader(header *gitalypb.UserCommitFilesRequestHeader) error { if header.GetRepository() == nil { - return fmt.Errorf("empty Repository") + return gitalyerrors.ErrEmptyRepository } if header.GetUser() == nil { return fmt.Errorf("empty User") diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index 8886340c7..f87783d49 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -8,6 +8,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "github.com/sirupsen/logrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" @@ -17,6 +18,10 @@ import ( ) func validateMergeBranchRequest(request *gitalypb.UserMergeBranchRequest) error { + if request.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if request.User == nil { return fmt.Errorf("empty user") } @@ -225,7 +230,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc func validateFFRequest(in *gitalypb.UserFFBranchRequest) error { if in.Repository == nil { - return fmt.Errorf("empty repository") + return gitalyerrors.ErrEmptyRepository } if len(in.Branch) == 0 { @@ -304,6 +309,10 @@ func (s *Server) UserFFBranch(ctx context.Context, in *gitalypb.UserFFBranchRequ } func validateUserMergeToRefRequest(in *gitalypb.UserMergeToRefRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if len(in.FirstParentRef) == 0 && len(in.Branch) == 0 { return fmt.Errorf("empty first parent ref and branch name") } diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go index 7e7ab424b..32c26a6a9 100644 --- a/internal/gitaly/service/operations/merge_test.go +++ b/internal/gitaly/service/operations/merge_test.go @@ -372,8 +372,8 @@ func TestUserMergeBranch_abort(t *testing.T) { closeSend bool desc string }{ - {req: &gitalypb.UserMergeBranchRequest{}, desc: "empty request, don't close"}, - {req: &gitalypb.UserMergeBranchRequest{}, closeSend: true, desc: "empty request and close"}, + {req: &gitalypb.UserMergeBranchRequest{Repository: &gitalypb.Repository{}}, desc: "empty request, don't close"}, + {req: &gitalypb.UserMergeBranchRequest{Repository: &gitalypb.Repository{}}, closeSend: true, desc: "empty request and close"}, {closeSend: true, desc: "no request just close"}, } diff --git a/internal/gitaly/service/operations/rebase.go b/internal/gitaly/service/operations/rebase.go index b1bbc7448..e036b2691 100644 --- a/internal/gitaly/service/operations/rebase.go +++ b/internal/gitaly/service/operations/rebase.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" @@ -161,7 +162,7 @@ var ErrInvalidBranch = errors.New("invalid branch name") func validateUserRebaseConfirmableHeader(header *gitalypb.UserRebaseConfirmableRequest_Header) error { if header.GetRepository() == nil { - return errors.New("empty Repository") + return gitalyerrors.ErrEmptyRepository } if header.GetUser() == nil { diff --git a/internal/gitaly/service/operations/rebase_test.go b/internal/gitaly/service/operations/rebase_test.go index d8e216d94..a56845645 100644 --- a/internal/gitaly/service/operations/rebase_test.go +++ b/internal/gitaly/service/operations/rebase_test.go @@ -414,9 +414,34 @@ func TestUserRebaseConfirmable_abortViaClose(t *testing.T) { desc string code codes.Code }{ - {req: &gitalypb.UserRebaseConfirmableRequest{}, desc: "empty request, don't close", code: codes.FailedPrecondition}, - {req: &gitalypb.UserRebaseConfirmableRequest{}, closeSend: true, desc: "empty request and close", code: codes.FailedPrecondition}, - {closeSend: true, desc: "no request just close", code: codes.Internal}, + { + req: &gitalypb.UserRebaseConfirmableRequest{ + UserRebaseConfirmableRequestPayload: &gitalypb.UserRebaseConfirmableRequest_Header_{ + Header: &gitalypb.UserRebaseConfirmableRequest_Header{ + Repository: &gitalypb.Repository{}, + }, + }, + }, + desc: "empty request, don't close", + code: codes.FailedPrecondition, + }, + { + req: &gitalypb.UserRebaseConfirmableRequest{ + UserRebaseConfirmableRequestPayload: &gitalypb.UserRebaseConfirmableRequest_Header_{ + Header: &gitalypb.UserRebaseConfirmableRequest_Header{ + Repository: &gitalypb.Repository{}, + }, + }, + }, + closeSend: true, + desc: "empty request and close", + code: codes.FailedPrecondition, + }, + { + closeSend: true, + desc: "no request just close", + code: codes.Internal, + }, } for i, tc := range testCases { diff --git a/internal/gitaly/service/operations/squash.go b/internal/gitaly/service/operations/squash.go index 7f2bc0881..5085bd540 100644 --- a/internal/gitaly/service/operations/squash.go +++ b/internal/gitaly/service/operations/squash.go @@ -5,6 +5,7 @@ import ( "errors" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" @@ -34,7 +35,7 @@ func (s *Server) UserSquash(ctx context.Context, req *gitalypb.UserSquashRequest func validateUserSquashRequest(req *gitalypb.UserSquashRequest) error { if req.GetRepository() == nil { - return errors.New("empty Repository") + return gitalyerrors.ErrEmptyRepository } if req.GetUser() == nil { diff --git a/internal/gitaly/service/operations/submodules.go b/internal/gitaly/service/operations/submodules.go index dd8e3ee70..2f5b290f8 100644 --- a/internal/gitaly/service/operations/submodules.go +++ b/internal/gitaly/service/operations/submodules.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/v15/internal/git2go" @@ -30,7 +31,7 @@ func (s *Server) UserUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpda func validateUserUpdateSubmoduleRequest(req *gitalypb.UserUpdateSubmoduleRequest) error { if req.GetRepository() == nil { - return fmt.Errorf("empty Repository") + return gitalyerrors.ErrEmptyRepository } if req.GetUser() == nil { diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go index 71909dfb1..f97715979 100644 --- a/internal/gitaly/service/operations/tags.go +++ b/internal/gitaly/service/operations/tags.go @@ -8,6 +8,7 @@ import ( "regexp" "time" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" @@ -20,16 +21,24 @@ import ( "google.golang.org/grpc/status" ) -//nolint: stylecheck // This is unintentionally missing documentation. -func (s *Server) UserDeleteTag(ctx context.Context, req *gitalypb.UserDeleteTagRequest) (*gitalypb.UserDeleteTagResponse, error) { - if len(req.TagName) == 0 { - return nil, status.Errorf(codes.InvalidArgument, "empty tag name") +func validateUserDeleteTagRequest(in *gitalypb.UserDeleteTagRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository } - - if req.User == nil { - return nil, status.Errorf(codes.InvalidArgument, "empty user") + if len(in.GetTagName()) == 0 { + return errors.New("empty tag name") + } + if in.GetUser() == nil { + return errors.New("empty user") } + return nil +} +//nolint: stylecheck // This is unintentionally missing documentation. +func (s *Server) UserDeleteTag(ctx context.Context, req *gitalypb.UserDeleteTagRequest) (*gitalypb.UserDeleteTagResponse, error) { + if err := validateUserDeleteTagRequest(req); err != nil { + return nil, helper.ErrInvalidArgument(err) + } referenceName := git.ReferenceName(fmt.Sprintf("refs/tags/%s", req.TagName)) revision, err := s.localrepo(req.GetRepository()).ResolveRevision(ctx, referenceName.Revision()) if err != nil { @@ -77,7 +86,7 @@ func validateUserCreateTag(req *gitalypb.UserCreateTagRequest) error { } if req.GetRepository() == nil { - return fmt.Errorf("empty repository") + return gitalyerrors.ErrEmptyRepository } return nil diff --git a/internal/gitaly/service/operations/utils.go b/internal/gitaly/service/operations/utils.go index 2052c9b3d..6d58e6f2d 100644 --- a/internal/gitaly/service/operations/utils.go +++ b/internal/gitaly/service/operations/utils.go @@ -4,11 +4,13 @@ import ( "fmt" "time" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/protobuf/types/known/timestamppb" ) type cherryPickOrRevertRequest interface { + GetRepository() *gitalypb.Repository GetUser() *gitalypb.User GetCommit() *gitalypb.GitCommit GetBranchName() []byte @@ -16,6 +18,10 @@ type cherryPickOrRevertRequest interface { } func validateCherryPickOrRevertRequest(req cherryPickOrRevertRequest) error { + if req.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if req.GetUser() == nil { return fmt.Errorf("empty User") } diff --git a/internal/gitaly/service/ref/branches.go b/internal/gitaly/service/ref/branches.go index 71daec916..e99c62f44 100644 --- a/internal/gitaly/service/ref/branches.go +++ b/internal/gitaly/service/ref/branches.go @@ -4,13 +4,18 @@ import ( "context" "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/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest) (*gitalypb.FindBranchResponse, error) { + if req.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if len(req.GetName()) == 0 { return nil, status.Errorf(codes.InvalidArgument, "Branch name cannot be empty") } diff --git a/internal/gitaly/service/ref/delete_refs.go b/internal/gitaly/service/ref/delete_refs.go index f8d7a7f03..1a2bb9fe1 100644 --- a/internal/gitaly/service/ref/delete_refs.go +++ b/internal/gitaly/service/ref/delete_refs.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" @@ -178,6 +179,10 @@ func hasAnyPrefix(s string, prefixes []string) bool { } func validateDeleteRefRequest(req *gitalypb.DeleteRefsRequest) error { + if req.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if len(req.ExceptWithPrefix) > 0 && len(req.Refs) > 0 { return fmt.Errorf("ExceptWithPrefix and Refs are mutually exclusive") } diff --git a/internal/gitaly/service/ref/find_all_tags.go b/internal/gitaly/service/ref/find_all_tags.go index 551083b30..a07d225e0 100644 --- a/internal/gitaly/service/ref/find_all_tags.go +++ b/internal/gitaly/service/ref/find_all_tags.go @@ -7,6 +7,7 @@ import ( "fmt" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" @@ -166,7 +167,7 @@ func (s *server) findAllTags(ctx context.Context, repo *localrepo.Repo, sortFiel func (s *server) validateFindAllTagsRequest(request *gitalypb.FindAllTagsRequest) error { if request.GetRepository() == nil { - return errors.New("empty Repository") + return gitalyerrors.ErrEmptyRepository } if _, err := s.locator.GetRepoPath(request.GetRepository()); err != nil { diff --git a/internal/gitaly/service/ref/find_refs_by_oid.go b/internal/gitaly/service/ref/find_refs_by_oid.go index 520acb3fa..bdadd7513 100644 --- a/internal/gitaly/service/ref/find_refs_by_oid.go +++ b/internal/gitaly/service/ref/find_refs_by_oid.go @@ -5,6 +5,7 @@ import ( "errors" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -52,7 +53,7 @@ func (s *server) FindRefsByOID(ctx context.Context, in *gitalypb.FindRefsByOIDRe func validateFindRefsReq(in *gitalypb.FindRefsByOIDRequest) error { if in.GetRepository() == nil { - return errors.New("empty Repository") + return gitalyerrors.ErrEmptyRepository } if in.GetOid() == "" { diff --git a/internal/gitaly/service/ref/find_tag.go b/internal/gitaly/service/ref/find_tag.go index b23b50b3e..bc4f1f465 100644 --- a/internal/gitaly/service/ref/find_tag.go +++ b/internal/gitaly/service/ref/find_tag.go @@ -7,6 +7,7 @@ import ( "fmt" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -125,7 +126,7 @@ func (s *server) findTag(ctx context.Context, repo git.RepositoryExecutor, tagNa func (s *server) validateFindTagRequest(in *gitalypb.FindTagRequest) error { if in.GetRepository() == nil { - return errors.New("repository is empty") + return gitalyerrors.ErrEmptyRepository } if _, err := s.locator.GetRepoPath(in.GetRepository()); err != nil { diff --git a/internal/gitaly/service/ref/list_refs.go b/internal/gitaly/service/ref/list_refs.go index 7b11f5a91..512f17d28 100644 --- a/internal/gitaly/service/ref/list_refs.go +++ b/internal/gitaly/service/ref/list_refs.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/lines" @@ -48,7 +49,7 @@ func (s *server) ListRefs(in *gitalypb.ListRefsRequest, stream gitalypb.RefServi func validateListRefsRequest(in *gitalypb.ListRefsRequest) error { if in.GetRepository() == nil { - return errors.New("repository is empty") + return gitalyerrors.ErrEmptyRepository } if len(in.GetPatterns()) < 1 { return errors.New("patterns must have at least one entry") diff --git a/internal/gitaly/service/ref/refexists.go b/internal/gitaly/service/ref/refexists.go index b081fbee2..aab56ca0e 100644 --- a/internal/gitaly/service/ref/refexists.go +++ b/internal/gitaly/service/ref/refexists.go @@ -6,6 +6,7 @@ import ( "strings" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -13,6 +14,10 @@ import ( // RefExists returns true if the given reference exists. The ref must start with the string `ref/` func (s *server) RefExists(ctx context.Context, in *gitalypb.RefExistsRequest) (*gitalypb.RefExistsResponse, error) { + if in.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } + ref := string(in.Ref) if !isValidRefName(ref) { diff --git a/internal/gitaly/service/ref/refnames.go b/internal/gitaly/service/ref/refnames.go index 491b31d14..958019d2c 100644 --- a/internal/gitaly/service/ref/refnames.go +++ b/internal/gitaly/service/ref/refnames.go @@ -4,7 +4,9 @@ import ( "bufio" "context" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/protobuf/proto" @@ -13,6 +15,10 @@ import ( // FindAllBranchNames creates a stream of ref names for all branches in the given repository func (s *server) FindAllBranchNames(in *gitalypb.FindAllBranchNamesRequest, stream gitalypb.RefService_FindAllBranchNamesServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } + chunker := chunk.New(&findAllBranchNamesSender{stream: stream}) return s.listRefNames(stream.Context(), chunker, "refs/heads", in.Repository, nil) @@ -34,6 +40,10 @@ func (ts *findAllBranchNamesSender) Send() error { // FindAllTagNames creates a stream of ref names for all tags in the given repository func (s *server) FindAllTagNames(in *gitalypb.FindAllTagNamesRequest, stream gitalypb.RefService_FindAllTagNamesServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } + chunker := chunk.New(&findAllTagNamesSender{stream: stream}) return s.listRefNames(stream.Context(), chunker, "refs/tags", in.Repository, nil) diff --git a/internal/gitaly/service/ref/refnames_containing.go b/internal/gitaly/service/ref/refnames_containing.go index 420dbda83..dc3db16f8 100644 --- a/internal/gitaly/service/ref/refnames_containing.go +++ b/internal/gitaly/service/ref/refnames_containing.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" @@ -15,6 +16,9 @@ import ( // ListBranchNamesContainingCommit returns a maximum of in.GetLimit() Branch names // which contain the SHA1 passed as argument func (s *server) ListBranchNamesContainingCommit(in *gitalypb.ListBranchNamesContainingCommitRequest, stream gitalypb.RefService_ListBranchNamesContainingCommitServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if err := git.ObjectHashSHA1.ValidateHex(in.GetCommitId()); err != nil { return helper.ErrInvalidArgument(err) } @@ -58,6 +62,9 @@ func (bs *branchNamesContainingCommitSender) Send() error { // ListTagNamesContainingCommit returns a maximum of in.GetLimit() Tag names // which contain the SHA1 passed as argument func (s *server) ListTagNamesContainingCommit(in *gitalypb.ListTagNamesContainingCommitRequest, stream gitalypb.RefService_ListTagNamesContainingCommitServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if err := git.ObjectHashSHA1.ValidateHex(in.GetCommitId()); err != nil { return helper.ErrInvalidArgument(err) } diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go index ca67ce8ed..d1b9a23f9 100644 --- a/internal/gitaly/service/ref/refs.go +++ b/internal/gitaly/service/ref/refs.go @@ -7,6 +7,7 @@ import ( "math" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/lines" @@ -69,6 +70,9 @@ func (s *server) findRefs(ctx context.Context, writer lines.Sender, repo git.Rep // FindDefaultBranchName returns the default branch name for the given repository func (s *server) FindDefaultBranchName(ctx context.Context, in *gitalypb.FindDefaultBranchNameRequest) (*gitalypb.FindDefaultBranchNameResponse, error) { + if in.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repo := s.localrepo(in.GetRepository()) defaultBranch, err := repo.GetDefaultBranch(ctx) @@ -94,6 +98,9 @@ func parseSortKey(sortKey gitalypb.FindLocalBranchesRequest_SortBy) string { // FindLocalBranches creates a stream of branches for all local branches in the given repository func (s *server) FindLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream gitalypb.RefService_FindLocalBranchesServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if err := s.findLocalBranches(in, stream); err != nil { return helper.ErrInternal(err) } @@ -123,6 +130,9 @@ func (s *server) findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream } func (s *server) FindAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefService_FindAllBranchesServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if err := s.findAllBranches(in, stream); err != nil { return helper.ErrInternal(err) } diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go index 1a09d9fa2..1b1b654ad 100644 --- a/internal/gitaly/service/ref/refs_test.go +++ b/internal/gitaly/service/ref/refs_test.go @@ -635,7 +635,7 @@ func testEmptyFindLocalBranchesRequest(t *testing.T, ctx context.Context) { testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf(gitalyOrPraefect( - "GetStorageByName: no such storage: \"\"", + "empty Repository", "repo scoped: empty Repository", )), recvError, @@ -790,7 +790,7 @@ func TestInvalidFindAllBranchesRequest(t *testing.T) { description: "Empty request", request: &gitalypb.FindAllBranchesRequest{}, expectedErr: helper.ErrInvalidArgumentf(gitalyOrPraefect( - "GetStorageByName: no such storage: \"\"", + "empty Repository", "repo scoped: empty Repository", )), }, diff --git a/internal/gitaly/service/ref/remote_branches.go b/internal/gitaly/service/ref/remote_branches.go index 617c7fa00..42543dc6a 100644 --- a/internal/gitaly/service/ref/remote_branches.go +++ b/internal/gitaly/service/ref/remote_branches.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -46,7 +47,7 @@ func (s *server) findAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesReques func validateFindAllRemoteBranchesRequest(req *gitalypb.FindAllRemoteBranchesRequest) error { if req.GetRepository() == nil { - return fmt.Errorf("empty Repository") + return gitalyerrors.ErrEmptyRepository } if len(req.GetRemoteName()) == 0 { diff --git a/internal/gitaly/service/ref/tag_messages.go b/internal/gitaly/service/ref/tag_messages.go index ccb18a26d..87273b988 100644 --- a/internal/gitaly/service/ref/tag_messages.go +++ b/internal/gitaly/service/ref/tag_messages.go @@ -5,6 +5,7 @@ import ( "fmt" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -27,7 +28,7 @@ func (s *server) GetTagMessages(request *gitalypb.GetTagMessagesRequest, stream func validateGetTagMessagesRequest(request *gitalypb.GetTagMessagesRequest) error { if request.GetRepository() == nil { - return fmt.Errorf("empty Repository") + return gitalyerrors.ErrEmptyRepository } return nil diff --git a/internal/gitaly/service/ref/tag_signatures.go b/internal/gitaly/service/ref/tag_signatures.go index b73c5df6a..6c1608ab3 100644 --- a/internal/gitaly/service/ref/tag_signatures.go +++ b/internal/gitaly/service/ref/tag_signatures.go @@ -6,6 +6,7 @@ import ( "io" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -16,7 +17,7 @@ import ( func verifyGetTagSignaturesRequest(req *gitalypb.GetTagSignaturesRequest) error { if req.GetRepository() == nil { - return errors.New("empty repository") + return gitalyerrors.ErrEmptyRepository } if len(req.GetTagRevisions()) == 0 { diff --git a/internal/gitaly/service/remote/find_remote_root_ref.go b/internal/gitaly/service/remote/find_remote_root_ref.go index 7ec4012e5..a43de7422 100644 --- a/internal/gitaly/service/remote/find_remote_root_ref.go +++ b/internal/gitaly/service/remote/find_remote_root_ref.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -76,7 +77,7 @@ func (s *server) FindRemoteRootRef(ctx context.Context, in *gitalypb.FindRemoteR return nil, status.Error(codes.InvalidArgument, "missing remote URL") } if in.Repository == nil { - return nil, status.Error(codes.InvalidArgument, "missing repository") + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } ref, err := s.findRemoteRootRef(ctx, in) diff --git a/internal/gitaly/service/remote/find_remote_root_ref_test.go b/internal/gitaly/service/remote/find_remote_root_ref_test.go index 193946176..ed201ee1d 100644 --- a/internal/gitaly/service/remote/find_remote_root_ref_test.go +++ b/internal/gitaly/service/remote/find_remote_root_ref_test.go @@ -104,7 +104,7 @@ func TestFindRemoteRootRefFailedDueToValidation(t *testing.T) { request: &gitalypb.FindRemoteRootRefRequest{ RemoteUrl: "remote-url", }, - expectedErr: helper.ErrInvalidArgumentf("missing repository"), + expectedErr: helper.ErrInvalidArgumentf("empty Repository"), }, { desc: "Remote URL is empty", diff --git a/internal/gitaly/service/remote/update_remote_mirror.go b/internal/gitaly/service/remote/update_remote_mirror.go index 9aeb5161c..343a48f8d 100644 --- a/internal/gitaly/service/remote/update_remote_mirror.go +++ b/internal/gitaly/service/remote/update_remote_mirror.go @@ -8,6 +8,7 @@ import ( "regexp" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -264,7 +265,7 @@ func newReferenceMatcher(branchMatchers [][]byte) (*regexp.Regexp, error) { func validateUpdateRemoteMirrorRequest(ctx context.Context, req *gitalypb.UpdateRemoteMirrorRequest) error { if req.GetRepository() == nil { - return fmt.Errorf("empty Repository") + return gitalyerrors.ErrEmptyRepository } if req.GetRemote() == nil { diff --git a/internal/gitaly/service/repository/apply_gitattributes.go b/internal/gitaly/service/repository/apply_gitattributes.go index 460a86ba5..52cd74e43 100644 --- a/internal/gitaly/service/repository/apply_gitattributes.go +++ b/internal/gitaly/service/repository/apply_gitattributes.go @@ -9,6 +9,7 @@ import ( "path/filepath" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" @@ -122,6 +123,9 @@ func (s *server) vote(ctx context.Context, oid git.ObjectID, phase voting.Phase) } func (s *server) ApplyGitattributes(ctx context.Context, in *gitalypb.ApplyGitattributesRequest) (*gitalypb.ApplyGitattributesResponse, error) { + if in.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repo := s.localrepo(in.GetRepository()) repoPath, err := s.locator.GetRepoPath(repo) if err != nil { diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go index b2688fcb7..be6ffdd36 100644 --- a/internal/gitaly/service/repository/archive.go +++ b/internal/gitaly/service/repository/archive.go @@ -11,6 +11,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/smudge" @@ -34,6 +35,9 @@ type archiveParams struct { func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.RepositoryService_GetArchiveServer) error { ctx := stream.Context() + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } compressArgs, format := parseArchiveFormat(in.GetFormat()) repo := s.localrepo(in.GetRepository()) diff --git a/internal/gitaly/service/repository/backup_custom_hooks.go b/internal/gitaly/service/repository/backup_custom_hooks.go index 9a44385d7..a9b49b8f0 100644 --- a/internal/gitaly/service/repository/backup_custom_hooks.go +++ b/internal/gitaly/service/repository/backup_custom_hooks.go @@ -5,6 +5,8 @@ import ( "path/filepath" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" "google.golang.org/grpc/codes" @@ -14,6 +16,9 @@ import ( const customHooksDir = "custom_hooks" func (s *server) BackupCustomHooks(in *gitalypb.BackupCustomHooksRequest, stream gitalypb.RepositoryService_BackupCustomHooksServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repoPath, err := s.locator.GetPath(in.Repository) if err != nil { return status.Errorf(codes.Internal, "BackupCustomHooks: getting repo path failed %v", err) diff --git a/internal/gitaly/service/repository/calculate_checksum.go b/internal/gitaly/service/repository/calculate_checksum.go index 3407936ec..84eb6f627 100644 --- a/internal/gitaly/service/repository/calculate_checksum.go +++ b/internal/gitaly/service/repository/calculate_checksum.go @@ -7,7 +7,9 @@ import ( "encoding/hex" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -15,7 +17,9 @@ import ( func (s *server) CalculateChecksum(ctx context.Context, in *gitalypb.CalculateChecksumRequest) (*gitalypb.CalculateChecksumResponse, error) { repo := in.GetRepository() - + if repo == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repoPath, err := s.locator.GetRepoPath(repo) if err != nil { return nil, err diff --git a/internal/gitaly/service/repository/config.go b/internal/gitaly/service/repository/config.go index b972a89a8..d99cc36dc 100644 --- a/internal/gitaly/service/repository/config.go +++ b/internal/gitaly/service/repository/config.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" @@ -17,6 +18,9 @@ func (s *server) GetConfig( request *gitalypb.GetConfigRequest, stream gitalypb.RepositoryService_GetConfigServer, ) error { + if request.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repoPath, err := s.locator.GetPath(request.GetRepository()) if err != nil { return err diff --git a/internal/gitaly/service/repository/create_bundle.go b/internal/gitaly/service/repository/create_bundle.go index 67127f0e9..3acbcca16 100644 --- a/internal/gitaly/service/repository/create_bundle.go +++ b/internal/gitaly/service/repository/create_bundle.go @@ -3,6 +3,7 @@ package repository import ( "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -14,7 +15,7 @@ import ( func (s *server) CreateBundle(req *gitalypb.CreateBundleRequest, stream gitalypb.RepositoryService_CreateBundleServer) error { repo := req.GetRepository() if repo == nil { - return status.Errorf(codes.InvalidArgument, "CreateBundle: empty Repository") + return helper.ErrInvalidArgumentf("CreateBundle: %w", gitalyerrors.ErrEmptyRepository) } ctx := stream.Context() diff --git a/internal/gitaly/service/repository/create_bundle_from_ref_list.go b/internal/gitaly/service/repository/create_bundle_from_ref_list.go index b8ba37c6c..428ea6749 100644 --- a/internal/gitaly/service/repository/create_bundle_from_ref_list.go +++ b/internal/gitaly/service/repository/create_bundle_from_ref_list.go @@ -5,7 +5,9 @@ import ( "io" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" "google.golang.org/grpc/codes" @@ -19,7 +21,7 @@ func (s *server) CreateBundleFromRefList(stream gitalypb.RepositoryService_Creat } if firstRequest.GetRepository() == nil { - return status.Errorf(codes.InvalidArgument, "empty Repository") + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } ctx := stream.Context() diff --git a/internal/gitaly/service/repository/create_fork.go b/internal/gitaly/service/repository/create_fork.go index 7566a9344..36265ed84 100644 --- a/internal/gitaly/service/repository/create_fork.go +++ b/internal/gitaly/service/repository/create_fork.go @@ -6,6 +6,7 @@ import ( "os" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -21,7 +22,7 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest return nil, status.Errorf(codes.InvalidArgument, "CreateFork: empty SourceRepository") } if targetRepository == nil { - return nil, status.Errorf(codes.InvalidArgument, "CreateFork: empty Repository") + return nil, helper.ErrInvalidArgumentf("CreateFork: %w", gitalyerrors.ErrEmptyRepository) } if err := s.createRepository(ctx, targetRepository, func(repo *gitalypb.Repository) error { diff --git a/internal/gitaly/service/repository/create_repository.go b/internal/gitaly/service/repository/create_repository.go index 41d207c8e..d44229c53 100644 --- a/internal/gitaly/service/repository/create_repository.go +++ b/internal/gitaly/service/repository/create_repository.go @@ -3,11 +3,15 @@ package repository import ( "context" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) func (s *server) CreateRepository(ctx context.Context, req *gitalypb.CreateRepositoryRequest) (*gitalypb.CreateRepositoryResponse, error) { + if req.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if err := s.createRepository( ctx, req.GetRepository(), diff --git a/internal/gitaly/service/repository/create_repository_from_bundle.go b/internal/gitaly/service/repository/create_repository_from_bundle.go index f7da9c43d..d5b5f3797 100644 --- a/internal/gitaly/service/repository/create_repository_from_bundle.go +++ b/internal/gitaly/service/repository/create_repository_from_bundle.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/tempdir" @@ -25,7 +26,7 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr repo := firstRequest.GetRepository() if repo == nil { - return status.Errorf(codes.InvalidArgument, "CreateRepositoryFromBundle: empty Repository") + return helper.ErrInvalidArgumentf("CreateRepositoryFromBundle: %w", gitalyerrors.ErrEmptyRepository) } ctx := stream.Context() diff --git a/internal/gitaly/service/repository/create_repository_from_snapshot.go b/internal/gitaly/service/repository/create_repository_from_snapshot.go index 4965fd3bb..48e2529ff 100644 --- a/internal/gitaly/service/repository/create_repository_from_snapshot.go +++ b/internal/gitaly/service/repository/create_repository_from_snapshot.go @@ -7,6 +7,7 @@ import ( "time" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/labkit/correlation" @@ -75,6 +76,9 @@ func untar(ctx context.Context, path string, in *gitalypb.CreateRepositoryFromSn } func (s *server) CreateRepositoryFromSnapshot(ctx context.Context, in *gitalypb.CreateRepositoryFromSnapshotRequest) (*gitalypb.CreateRepositoryFromSnapshotResponse, error) { + if in.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if err := s.createRepository(ctx, in.GetRepository(), func(repo *gitalypb.Repository) error { path, err := s.locator.GetPath(repo) if err != nil { diff --git a/internal/gitaly/service/repository/create_repository_from_url.go b/internal/gitaly/service/repository/create_repository_from_url.go index f7fd9b853..b96f57fde 100644 --- a/internal/gitaly/service/repository/create_repository_from_url.go +++ b/internal/gitaly/service/repository/create_repository_from_url.go @@ -9,6 +9,7 @@ import ( "os" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -120,7 +121,7 @@ func (s *server) CreateRepositoryFromURL(ctx context.Context, req *gitalypb.Crea func validateCreateRepositoryFromURLRequest(req *gitalypb.CreateRepositoryFromURLRequest) error { if req.GetRepository() == nil { - return fmt.Errorf("empty Repository") + return gitalyerrors.ErrEmptyRepository } if req.GetUrl() == "" { diff --git a/internal/gitaly/service/repository/fetch.go b/internal/gitaly/service/repository/fetch.go index ef10fee9b..08d7fd4a3 100644 --- a/internal/gitaly/service/repository/fetch.go +++ b/internal/gitaly/service/repository/fetch.go @@ -5,6 +5,7 @@ import ( "errors" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/remoterepo" @@ -12,12 +13,21 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) -func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourceBranchRequest) (*gitalypb.FetchSourceBranchResponse, error) { - if err := git.ValidateRevision(req.GetSourceBranch()); err != nil { - return nil, helper.ErrInvalidArgument(err) +func validateFetchSourceBranchRequest(in *gitalypb.FetchSourceBranchRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if err := git.ValidateRevision(in.GetSourceBranch()); err != nil { + return err } + if err := git.ValidateRevision(in.GetTargetRef()); err != nil { + return err + } + return nil +} - if err := git.ValidateRevision(req.GetTargetRef()); err != nil { +func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourceBranchRequest) (*gitalypb.FetchSourceBranchResponse, error) { + if err := validateFetchSourceBranchRequest(req); err != nil { return nil, helper.ErrInvalidArgument(err) } diff --git a/internal/gitaly/service/repository/fsck.go b/internal/gitaly/service/repository/fsck.go index db6f8eee6..b705e8a93 100644 --- a/internal/gitaly/service/repository/fsck.go +++ b/internal/gitaly/service/repository/fsck.go @@ -4,7 +4,9 @@ import ( "bytes" "context" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -12,6 +14,9 @@ func (s *server) Fsck(ctx context.Context, req *gitalypb.FsckRequest) (*gitalypb var stdout, stderr bytes.Buffer repo := req.GetRepository() + if repo == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } cmd, err := s.gitCmdFactory.New(ctx, repo, git.SubCmd{Name: "fsck"}, diff --git a/internal/gitaly/service/repository/fullpath.go b/internal/gitaly/service/repository/fullpath.go index 2f5194906..bc85a72a5 100644 --- a/internal/gitaly/service/repository/fullpath.go +++ b/internal/gitaly/service/repository/fullpath.go @@ -4,7 +4,7 @@ import ( "context" "strings" - "gitlab.com/gitlab-org/gitaly/v15/internal/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/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -19,7 +19,7 @@ func (s *server) SetFullPath( request *gitalypb.SetFullPathRequest, ) (*gitalypb.SetFullPathResponse, error) { if request.GetRepository() == nil { - return nil, helper.ErrInvalidArgumentf("empty Repository") + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } if len(request.GetPath()) == 0 { @@ -39,7 +39,7 @@ func (s *server) SetFullPath( // "gitlab.fullpath" key. func (s *server) FullPath(ctx context.Context, request *gitalypb.FullPathRequest) (*gitalypb.FullPathResponse, error) { if request.GetRepository() == nil { - return nil, helper.ErrInvalidArgument(errors.ErrEmptyRepository) + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } repo := s.localrepo(request.GetRepository()) diff --git a/internal/gitaly/service/repository/info_attributes.go b/internal/gitaly/service/repository/info_attributes.go index 023fc28fd..cf2c86031 100644 --- a/internal/gitaly/service/repository/info_attributes.go +++ b/internal/gitaly/service/repository/info_attributes.go @@ -5,6 +5,8 @@ import ( "os" "path/filepath" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" "google.golang.org/grpc/codes" @@ -12,6 +14,9 @@ import ( ) func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream gitalypb.RepositoryService_GetInfoAttributesServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repoPath, err := s.locator.GetRepoPath(in.GetRepository()) if err != nil { return err diff --git a/internal/gitaly/service/repository/license.go b/internal/gitaly/service/repository/license.go index 4d1b8fd43..2d9d64d11 100644 --- a/internal/gitaly/service/repository/license.go +++ b/internal/gitaly/service/repository/license.go @@ -13,6 +13,7 @@ import ( "github.com/go-enry/go-license-detector/v4/licensedb" "github.com/go-enry/go-license-detector/v4/licensedb/api" "github.com/go-enry/go-license-detector/v4/licensedb/filer" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/lstree" @@ -40,6 +41,9 @@ var nicknameByLicenseIdentifier = map[string]string{ } func (s *server) FindLicense(ctx context.Context, req *gitalypb.FindLicenseRequest) (*gitalypb.FindLicenseResponse, error) { + if req.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } if featureflag.GoFindLicense.IsEnabled(ctx) { repo := localrepo.New(s.locator, s.gitCmdFactory, s.catfileCache, req.GetRepository()) diff --git a/internal/gitaly/service/repository/merge_base.go b/internal/gitaly/service/repository/merge_base.go index 0879e7243..21486caf6 100644 --- a/internal/gitaly/service/repository/merge_base.go +++ b/internal/gitaly/service/repository/merge_base.go @@ -4,7 +4,9 @@ import ( "context" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "google.golang.org/grpc/codes" @@ -12,6 +14,9 @@ import ( ) func (s *server) FindMergeBase(ctx context.Context, req *gitalypb.FindMergeBaseRequest) (*gitalypb.FindMergeBaseResponse, error) { + if req.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } var revisions []string for _, rev := range req.GetRevisions() { revisions = append(revisions, string(rev)) diff --git a/internal/gitaly/service/repository/prune_unreachable_objects.go b/internal/gitaly/service/repository/prune_unreachable_objects.go index 2ba270830..2ae3f8394 100644 --- a/internal/gitaly/service/repository/prune_unreachable_objects.go +++ b/internal/gitaly/service/repository/prune_unreachable_objects.go @@ -3,6 +3,7 @@ package repository import ( "context" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/housekeeping" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" @@ -19,7 +20,7 @@ func (s *server) PruneUnreachableObjects( request *gitalypb.PruneUnreachableObjectsRequest, ) (*gitalypb.PruneUnreachableObjectsResponse, error) { if request.GetRepository() == nil { - return nil, helper.ErrInvalidArgumentf("missing repository") + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } repo := s.localrepo(request.GetRepository()) diff --git a/internal/gitaly/service/repository/prune_unreachable_objects_test.go b/internal/gitaly/service/repository/prune_unreachable_objects_test.go index 29bb18eac..3988d4fbc 100644 --- a/internal/gitaly/service/repository/prune_unreachable_objects_test.go +++ b/internal/gitaly/service/repository/prune_unreachable_objects_test.go @@ -32,7 +32,7 @@ func TestPruneUnreachableObjects(t *testing.T) { if testhelper.IsPraefectEnabled() { testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf("repo scoped: empty Repository"), err) } else { - testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf("missing repository"), err) + testhelper.RequireGrpcError(t, helper.ErrInvalidArgumentf("empty Repository"), err) } }) diff --git a/internal/gitaly/service/repository/raw_changes.go b/internal/gitaly/service/repository/raw_changes.go index 9a81e1637..d76d72a44 100644 --- a/internal/gitaly/service/repository/raw_changes.go +++ b/internal/gitaly/service/repository/raw_changes.go @@ -7,6 +7,7 @@ import ( "regexp" "strconv" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/rawdiff" @@ -18,6 +19,9 @@ import ( func (s *server) GetRawChanges(req *gitalypb.GetRawChangesRequest, stream gitalypb.RepositoryService_GetRawChangesServer) error { ctx := stream.Context() + if req.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repo := s.localrepo(req.GetRepository()) objectInfoReader, cancel, err := s.catfileCache.ObjectInfoReader(stream.Context(), repo) diff --git a/internal/gitaly/service/repository/raw_changes_test.go b/internal/gitaly/service/repository/raw_changes_test.go index b91d1ecdc..409202eab 100644 --- a/internal/gitaly/service/repository/raw_changes_test.go +++ b/internal/gitaly/service/repository/raw_changes_test.go @@ -185,7 +185,7 @@ func TestGetRawChangesFailures(t *testing.T) { ToRevision: "913c66a37b4a45b9769037c55c2d238bd0942d2e", }, expectedErr: helper.ErrInvalidArgumentf(gitalyOrPraefect( - "GetStorageByName: no such storage: \"\"", + "empty Repository", "repo scoped: empty Repository", )), }, diff --git a/internal/gitaly/service/repository/remove.go b/internal/gitaly/service/repository/remove.go index b166df6d4..3e786110a 100644 --- a/internal/gitaly/service/repository/remove.go +++ b/internal/gitaly/service/repository/remove.go @@ -8,6 +8,7 @@ import ( "path/filepath" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/safe" @@ -18,7 +19,9 @@ import ( func (s *server) RemoveRepository(ctx context.Context, in *gitalypb.RemoveRepositoryRequest) (*gitalypb.RemoveRepositoryResponse, error) { repo := in.GetRepository() - + if repo == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } path, err := s.locator.GetPath(repo) if err != nil { return nil, helper.ErrInternal(err) diff --git a/internal/gitaly/service/repository/repack.go b/internal/gitaly/service/repository/repack.go index 51164133d..3795493f4 100644 --- a/internal/gitaly/service/repository/repack.go +++ b/internal/gitaly/service/repository/repack.go @@ -54,7 +54,7 @@ func (s *server) RepackFull(ctx context.Context, in *gitalypb.RepackFullRequest) func (s *server) RepackIncremental(ctx context.Context, in *gitalypb.RepackIncrementalRequest) (*gitalypb.RepackIncrementalResponse, error) { if in.GetRepository() == nil { - return nil, helper.ErrInvalidArgumentf("empty repository") + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } repo := s.localrepo(in.GetRepository()) diff --git a/internal/gitaly/service/repository/repack_test.go b/internal/gitaly/service/repository/repack_test.go index fb42f8d04..cce7b610c 100644 --- a/internal/gitaly/service/repository/repack_test.go +++ b/internal/gitaly/service/repository/repack_test.go @@ -115,7 +115,7 @@ func TestRepackIncrementalFailure(t *testing.T) { { desc: "nil repo", repo: nil, - err: status.Error(codes.InvalidArgument, gitalyOrPraefect("empty repository", "repo scoped: empty Repository")), + err: status.Error(codes.InvalidArgument, gitalyOrPraefect("empty Repository", "repo scoped: empty Repository")), }, { desc: "invalid storage name", diff --git a/internal/gitaly/service/repository/replicate.go b/internal/gitaly/service/repository/replicate.go index 67a52eca3..13e798dca 100644 --- a/internal/gitaly/service/repository/replicate.go +++ b/internal/gitaly/service/repository/replicate.go @@ -13,6 +13,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/client" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/remoterepo" @@ -91,7 +92,7 @@ func (s *server) ReplicateRepository(ctx context.Context, in *gitalypb.Replicate func validateReplicateRepository(in *gitalypb.ReplicateRepositoryRequest) error { if in.GetRepository() == nil { - return errors.New("repository cannot be empty") + return gitalyerrors.ErrEmptyRepository } if in.GetSource() == nil { diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index 89ca307b8..bd661280a 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -323,7 +323,7 @@ func TestReplicateRepositoryInvalidArguments(t *testing.T) { RelativePath: "/ab/cd/abcdef1234", }, }, - expectedError: "repository cannot be empty", + expectedError: "empty Repository", }, { description: "empty source", @@ -334,7 +334,7 @@ func TestReplicateRepositoryInvalidArguments(t *testing.T) { }, Source: nil, }, - expectedError: "repository cannot be empty", + expectedError: "empty Repository", }, { description: "source and repository have different relative paths", diff --git a/internal/gitaly/service/repository/repository.go b/internal/gitaly/service/repository/repository.go index 1f49b13c8..02686fa2c 100644 --- a/internal/gitaly/service/repository/repository.go +++ b/internal/gitaly/service/repository/repository.go @@ -3,6 +3,7 @@ package repository import ( "context" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" @@ -16,6 +17,9 @@ func (s *server) Exists(ctx context.Context, in *gitalypb.RepositoryExistsReques } func (s *server) RepositoryExists(ctx context.Context, in *gitalypb.RepositoryExistsRequest) (*gitalypb.RepositoryExistsResponse, error) { + if in.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } path, err := s.locator.GetPath(in.Repository) if err != nil { return nil, err @@ -25,6 +29,9 @@ func (s *server) RepositoryExists(ctx context.Context, in *gitalypb.RepositoryEx } func (s *server) HasLocalBranches(ctx context.Context, in *gitalypb.HasLocalBranchesRequest) (*gitalypb.HasLocalBranchesResponse, error) { + if in.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } hasBranches, err := s.localrepo(in.GetRepository()).HasBranches(ctx) if err != nil { return nil, helper.ErrInternal(err) diff --git a/internal/gitaly/service/repository/restore_custom_hooks.go b/internal/gitaly/service/repository/restore_custom_hooks.go index a3e2289e7..fdf5a7700 100644 --- a/internal/gitaly/service/repository/restore_custom_hooks.go +++ b/internal/gitaly/service/repository/restore_custom_hooks.go @@ -9,6 +9,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" @@ -33,7 +34,7 @@ func (s *server) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCus repo := firstRequest.GetRepository() if repo == nil { - return status.Errorf(codes.InvalidArgument, "RestoreCustomHooks: empty Repository") + return helper.ErrInvalidArgumentf("RestoreCustomHooks: %w", gitalyerrors.ErrInvalidRepository) } reader := streamio.NewReader(func() ([]byte, error) { @@ -83,7 +84,7 @@ func (s *server) restoreCustomHooksWithVoting(stream gitalypb.RepositoryService_ repo := firstRequest.GetRepository() if repo == nil { - return helper.ErrInvalidArgumentf("RestoreCustomHooks: empty Repository") + return helper.ErrInvalidArgumentf("RestoreCustomHooks: %w", gitalyerrors.ErrEmptyRepository) } v := voting.NewVoteHash() diff --git a/internal/gitaly/service/repository/search_files.go b/internal/gitaly/service/repository/search_files.go index ba7598888..03a1f5cf7 100644 --- a/internal/gitaly/service/repository/search_files.go +++ b/internal/gitaly/service/repository/search_files.go @@ -9,6 +9,7 @@ import ( "regexp" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/lines" @@ -31,13 +32,8 @@ func (s *server) SearchFilesByContent(req *gitalypb.SearchFilesByContentRequest, return helper.ErrInvalidArgument(err) } - repo := req.GetRepository() - if repo == nil { - return helper.ErrInvalidArgumentf("SearchFilesByContent: empty Repository") - } - ctx := stream.Context() - cmd, err := s.gitCmdFactory.New(ctx, repo, + cmd, err := s.gitCmdFactory.New(ctx, req.GetRepository(), git.SubCmd{Name: "grep", Flags: []git.Option{ git.Flag{Name: "--ignore-case"}, git.Flag{Name: "-I"}, @@ -118,15 +114,10 @@ func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, strea } } - repo := req.GetRepository() - if repo == nil { - return helper.ErrInvalidArgumentf("SearchFilesByName: empty Repository") - } - ctx := stream.Context() cmd, err := s.gitCmdFactory.New( ctx, - repo, + req.GetRepository(), git.SubCmd{Name: "ls-tree", Flags: []git.Option{ git.Flag{Name: "--full-tree"}, git.Flag{Name: "--name-status"}, @@ -149,11 +140,16 @@ func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, strea } type searchFilesRequest interface { + GetRepository() *gitalypb.Repository GetRef() []byte GetQuery() string } func validateSearchFilesRequest(req searchFilesRequest) error { + if req.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if len(req.GetQuery()) == 0 { return errors.New("no query given") } diff --git a/internal/gitaly/service/repository/search_files_test.go b/internal/gitaly/service/repository/search_files_test.go index 092ef976a..39a32e30d 100644 --- a/internal/gitaly/service/repository/search_files_test.go +++ b/internal/gitaly/service/repository/search_files_test.go @@ -239,11 +239,13 @@ func TestSearchFilesByContentFailure(t *testing.T) { }{ { desc: "empty request", + repo: &gitalypb.Repository{}, code: codes.InvalidArgument, msg: "no query given", }, { desc: "only query given", + repo: &gitalypb.Repository{}, query: "foo", code: codes.InvalidArgument, msg: "no ref given", @@ -474,11 +476,13 @@ func TestSearchFilesByNameFailure(t *testing.T) { }{ { desc: "empty request", + repo: &gitalypb.Repository{}, code: codes.InvalidArgument, msg: "no query given", }, { desc: "only query given", + repo: &gitalypb.Repository{}, query: "foo", code: codes.InvalidArgument, msg: "no ref given", @@ -492,6 +496,7 @@ func TestSearchFilesByNameFailure(t *testing.T) { }, { desc: "invalid filter", + repo: &gitalypb.Repository{}, query: "foo", ref: "master", filter: "+*.", @@ -500,6 +505,7 @@ func TestSearchFilesByNameFailure(t *testing.T) { }, { desc: "filter longer than max", + repo: &gitalypb.Repository{}, query: "foo", ref: "master", filter: strings.Repeat(".", searchFilesFilterMaxLength+1), diff --git a/internal/gitaly/service/repository/size.go b/internal/gitaly/service/repository/size.go index df03e2300..8e7af1566 100644 --- a/internal/gitaly/service/repository/size.go +++ b/internal/gitaly/service/repository/size.go @@ -9,6 +9,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" @@ -17,6 +18,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/objectpool" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" ) @@ -32,6 +34,9 @@ import ( // flag can be enabled to return the alternative repository size calculation // instead of the size derived from the disk usage command. func (s *server) RepositorySize(ctx context.Context, in *gitalypb.RepositorySizeRequest) (*gitalypb.RepositorySizeResponse, error) { + if in.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repo := s.localrepo(in.GetRepository()) path, err := repo.Path() @@ -160,6 +165,9 @@ func calculateSizeWithRevlist(ctx context.Context, repo *localrepo.Repo) (int64, } func (s *server) GetObjectDirectorySize(ctx context.Context, in *gitalypb.GetObjectDirectorySizeRequest) (*gitalypb.GetObjectDirectorySizeResponse, error) { + if in.GetRepository() == nil { + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repo := s.localrepo(in.GetRepository()) path, err := repo.ObjectDirectoryPath() diff --git a/internal/gitaly/service/repository/snapshot.go b/internal/gitaly/service/repository/snapshot.go index 5144d941b..ffb54b717 100644 --- a/internal/gitaly/service/repository/snapshot.go +++ b/internal/gitaly/service/repository/snapshot.go @@ -8,6 +8,7 @@ import ( "regexp" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + 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/archive" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -21,6 +22,10 @@ var objectFiles = []*regexp.Regexp{ } func (s *server) GetSnapshot(in *gitalypb.GetSnapshotRequest, stream gitalypb.RepositoryService_GetSnapshotServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } + path, err := s.locator.GetRepoPath(in.Repository) if err != nil { return err diff --git a/internal/gitaly/service/repository/write_ref.go b/internal/gitaly/service/repository/write_ref.go index e0a83f561..1e86f3fcd 100644 --- a/internal/gitaly/service/repository/write_ref.go +++ b/internal/gitaly/service/repository/write_ref.go @@ -5,6 +5,7 @@ import ( "context" "fmt" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/git/updateref" @@ -87,6 +88,9 @@ func updateRef(ctx context.Context, repo *localrepo.Repo, req *gitalypb.WriteRef } func validateWriteRefRequest(req *gitalypb.WriteRefRequest) error { + if req.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if err := git.ValidateRevision(req.Ref); err != nil { return fmt.Errorf("invalid ref: %v", err) } diff --git a/internal/gitaly/service/smarthttp/inforefs.go b/internal/gitaly/service/smarthttp/inforefs.go index 9a48d26e6..fdb0ac16a 100644 --- a/internal/gitaly/service/smarthttp/inforefs.go +++ b/internal/gitaly/service/smarthttp/inforefs.go @@ -7,8 +7,10 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" log "github.com/sirupsen/logrus" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" "google.golang.org/grpc/codes" @@ -21,6 +23,9 @@ const ( ) func (s *server) InfoRefsUploadPack(in *gitalypb.InfoRefsRequest, stream gitalypb.SmartHTTPService_InfoRefsUploadPackServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repoPath, err := s.locator.GetRepoPath(in.GetRepository()) if err != nil { return err @@ -36,6 +41,9 @@ func (s *server) InfoRefsUploadPack(in *gitalypb.InfoRefsRequest, stream gitalyp } func (s *server) InfoRefsReceivePack(in *gitalypb.InfoRefsRequest, stream gitalypb.SmartHTTPService_InfoRefsReceivePackServer) error { + if in.GetRepository() == nil { + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + } repoPath, err := s.locator.GetRepoPath(in.GetRepository()) if err != nil { return err diff --git a/internal/gitaly/service/smarthttp/receive_pack.go b/internal/gitaly/service/smarthttp/receive_pack.go index 323b6742c..073ba77ff 100644 --- a/internal/gitaly/service/smarthttp/receive_pack.go +++ b/internal/gitaly/service/smarthttp/receive_pack.go @@ -5,6 +5,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" log "github.com/sirupsen/logrus" + 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/transaction" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -103,7 +104,7 @@ func validateReceivePackRequest(req *gitalypb.PostReceivePackRequest) error { return status.Errorf(codes.InvalidArgument, "PostReceivePack: non-empty Data") } if req.Repository == nil { - return helper.ErrInvalidArgumentf("PostReceivePack: empty Repository") + return helper.ErrInvalidArgumentf("PostReceivePack: %w", gitalyerrors.ErrEmptyRepository) } return nil diff --git a/internal/gitaly/service/smarthttp/upload_pack.go b/internal/gitaly/service/smarthttp/upload_pack.go index b0353da19..0a72c1d55 100644 --- a/internal/gitaly/service/smarthttp/upload_pack.go +++ b/internal/gitaly/service/smarthttp/upload_pack.go @@ -8,6 +8,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -38,7 +39,7 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS repoPath, gitConfig, err := s.validateUploadPackRequest(ctx, req) if err != nil { - return err + return helper.ErrInvalidArgument(err) } stdin := streamio.NewReader(func() ([]byte, error) { @@ -56,7 +57,7 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS func (s *server) PostUploadPackWithSidechannel(ctx context.Context, req *gitalypb.PostUploadPackWithSidechannelRequest) (*gitalypb.PostUploadPackWithSidechannelResponse, error) { repoPath, gitConfig, err := s.validateUploadPackRequest(ctx, req) if err != nil { - return nil, err + return nil, helper.ErrInvalidArgument(err) } conn, err := sidechannel.OpenSidechannel(ctx) @@ -110,16 +111,19 @@ func (s *server) runStatsCollector(ctx context.Context, r io.Reader) (io.Reader, } func (s *server) validateUploadPackRequest(ctx context.Context, req basicPostUploadPackRequest) (string, []git.ConfigPair, error) { + if req.GetRepository() == nil { + return "", nil, gitalyerrors.ErrEmptyRepository + } repoPath, err := s.locator.GetRepoPath(req.GetRepository()) if err != nil { - return "", nil, helper.ErrInvalidArgument(err) + return "", nil, err } git.WarnIfTooManyBitmaps(ctx, s.locator, req.GetRepository().GetStorageName(), repoPath) config, err := git.ConvertConfigOptions(req.GetGitConfigOptions()) if err != nil { - return "", nil, helper.ErrInvalidArgument(err) + return "", nil, err } return repoPath, config, nil diff --git a/internal/gitaly/service/ssh/receive_pack.go b/internal/gitaly/service/ssh/receive_pack.go index 1dbddee99..207c80f82 100644 --- a/internal/gitaly/service/ssh/receive_pack.go +++ b/internal/gitaly/service/ssh/receive_pack.go @@ -10,6 +10,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + 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/transaction" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -146,7 +147,7 @@ func validateFirstReceivePackRequest(req *gitalypb.SSHReceivePackRequest) error return errors.New("non-empty data in first request") } if req.Repository == nil { - return errors.New("repository is empty") + return gitalyerrors.ErrEmptyRepository } return nil diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go index 0f9317696..33c9be4c8 100644 --- a/internal/gitaly/service/ssh/receive_pack_test.go +++ b/internal/gitaly/service/ssh/receive_pack_test.go @@ -81,7 +81,7 @@ func TestReceivePack_validation(t *testing.T) { return helper.ErrInvalidArgumentf("repo scoped: empty Repository") } - return helper.ErrInvalidArgumentf("repository is empty") + return helper.ErrInvalidArgumentf("empty Repository") }(), }, { diff --git a/internal/gitaly/service/ssh/upload_archive.go b/internal/gitaly/service/ssh/upload_archive.go index 3dc362a91..c99d09f23 100644 --- a/internal/gitaly/service/ssh/upload_archive.go +++ b/internal/gitaly/service/ssh/upload_archive.go @@ -6,6 +6,7 @@ import ( "sync" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -98,6 +99,9 @@ func (s *server) sshUploadArchive(stream gitalypb.SSHService_SSHUploadArchiveSer } func validateFirstUploadArchiveRequest(req *gitalypb.SSHUploadArchiveRequest) error { + if req.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if req.Stdin != nil { return fmt.Errorf("non-empty stdin in first request") } diff --git a/internal/gitaly/service/ssh/upload_pack.go b/internal/gitaly/service/ssh/upload_pack.go index e468c834c..632ea17e8 100644 --- a/internal/gitaly/service/ssh/upload_pack.go +++ b/internal/gitaly/service/ssh/upload_pack.go @@ -10,6 +10,7 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" log "github.com/sirupsen/logrus" "gitlab.com/gitlab-org/gitaly/v15/internal/command" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/pktline" "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" @@ -181,6 +182,9 @@ func (s *server) sshUploadPack(rpcContext context.Context, req sshUploadPackRequ } func validateFirstUploadPackRequest(req *gitalypb.SSHUploadPackRequest) error { + if req.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } if req.Stdin != nil { return fmt.Errorf("non-empty stdin in first request") } diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go index 4d6e9e46b..44cd68a7f 100644 --- a/internal/gitaly/service/ssh/upload_pack_test.go +++ b/internal/gitaly/service/ssh/upload_pack_test.go @@ -457,7 +457,7 @@ func TestUploadPack_validation(t *testing.T) { if testhelper.IsPraefectEnabled() { return helper.ErrInvalidArgumentf("repo scoped: empty Repository") } - return helper.ErrInvalidArgumentf("GetStorageByName: no such storage: \"\"") + return helper.ErrInvalidArgumentf("empty Repository") }(), }, { 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/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") } diff --git a/internal/praefect/rename_repository.go b/internal/praefect/rename_repository.go index 1a0918940..ab304309e 100644 --- a/internal/praefect/rename_repository.go +++ b/internal/praefect/rename_repository.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/commonerr" @@ -41,7 +42,7 @@ func validateRenameRepositoryRequest(req *gitalypb.RenameRepositoryRequest, virt // These checks are not strictly necessary but they exist to keep retain compatibility with // Gitaly's tested behavior. if req.GetRepository() == nil { - return helper.ErrInvalidArgumentf("empty Repository") + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } else if req.GetRelativePath() == "" { return helper.ErrInvalidArgumentf("destination relative path is empty") } else if _, ok := virtualStorages[req.GetRepository().GetStorageName()]; !ok { |