diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-02-19 11:37:45 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-02-19 11:37:45 +0300 |
commit | 5d4e8be941747b59aab99e94473dcdc705204490 (patch) | |
tree | da7833cafccaba8104e6f45a6ba4a2cda09a5890 | |
parent | 96e1bcbfe4445bddfaddb5c2ac8eedbddf3f68af (diff) | |
parent | 6a5ef63a10f4bf541dbb5a0bcad8c3774b42d74c (diff) |
Merge branch 'smh-fix-user-commit-files-index-error-handling' into 'master'
Fix UserCommitFiles index error handling
Closes #3461
See merge request gitlab-org/gitaly!3165
-rw-r--r-- | changelogs/unreleased/smh-fix-user-commit-files-index-error-handling.yml | 5 | ||||
-rw-r--r-- | cmd/gitaly-git2go/commit/commit.go | 4 | ||||
-rw-r--r-- | internal/git2go/commit.go | 6 | ||||
-rw-r--r-- | internal/git2go/gob.go | 1 | ||||
-rw-r--r-- | internal/gitaly/service/operations/commit_files.go | 12 | ||||
-rw-r--r-- | internal/gitaly/service/operations/commit_files_test.go | 12 |
6 files changed, 32 insertions, 8 deletions
diff --git a/changelogs/unreleased/smh-fix-user-commit-files-index-error-handling.yml b/changelogs/unreleased/smh-fix-user-commit-files-index-error-handling.yml new file mode 100644 index 000000000..bc0063740 --- /dev/null +++ b/changelogs/unreleased/smh-fix-user-commit-files-index-error-handling.yml @@ -0,0 +1,5 @@ +--- +title: Fix UserCommitFiles index error handling +merge_request: 3165 +author: +type: fixed diff --git a/cmd/gitaly-git2go/commit/commit.go b/cmd/gitaly-git2go/commit/commit.go index c8051c7a2..3c80452d2 100644 --- a/cmd/gitaly-git2go/commit/commit.go +++ b/cmd/gitaly-git2go/commit/commit.go @@ -64,6 +64,10 @@ func commit(ctx context.Context, params git2go.CommitParams) (string, error) { for _, action := range params.Actions { if err := apply(action, repo, index); err != nil { + if git.IsErrorClass(err, git.ErrClassIndex) { + err = git2go.IndexError(err.Error()) + } + return "", fmt.Errorf("apply action %T: %w", action, err) } } diff --git a/internal/git2go/commit.go b/internal/git2go/commit.go index 2a9478609..558c82bd9 100644 --- a/internal/git2go/commit.go +++ b/internal/git2go/commit.go @@ -7,6 +7,12 @@ import ( "fmt" ) +// IndexError is an error that was produced by performing an invalid operation on the index. +type IndexError string + +// Error returns the error message of the index error. +func (err IndexError) Error() string { return string(err) } + // InvalidArgumentError is returned when an invalid argument is provided. type InvalidArgumentError string diff --git a/internal/git2go/gob.go b/internal/git2go/gob.go index aa98b7964..4e760b437 100644 --- a/internal/git2go/gob.go +++ b/internal/git2go/gob.go @@ -30,6 +30,7 @@ var registeredTypes = map[interface{}]struct{}{ FileNotFoundError(""): {}, InvalidArgumentError(""): {}, HasConflictsError{}: {}, + IndexError(""): {}, } // Result is the serialized result. diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go index 2e8d70f29..8aa3791bf 100644 --- a/internal/gitaly/service/operations/commit_files.go +++ b/internal/gitaly/service/operations/commit_files.go @@ -27,10 +27,6 @@ import ( "google.golang.org/grpc/status" ) -type indexError string - -func (err indexError) Error() string { return string(err) } - func errorWithStderr(err error, stderr *bytes.Buffer) error { return fmt.Errorf("%w, stderr: %q", err, stderr) } @@ -74,7 +70,7 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile var ( response gitalypb.UserCommitFilesResponse - indexError indexError + indexError git2go.IndexError preReceiveError preReceiveError ) @@ -143,7 +139,7 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile func validatePath(rootPath, relPath string) (string, error) { if relPath == "" { - return "", indexError("You must provide a file path") + return "", git2go.IndexError("You must provide a file path") } else if strings.Contains(relPath, "//") { // This is a workaround to address a quirk in porting the RPC from Ruby to Go. // GitLab's QA pipeline runs tests with filepath 'invalid://file/name/here'. @@ -154,13 +150,13 @@ func validatePath(rootPath, relPath string) (string, error) { // // The Rails code expects to receive an error prefixed with 'invalid path', which is done // here to retain compatibility. - return "", indexError(fmt.Sprintf("invalid path: '%s'", relPath)) + return "", git2go.IndexError(fmt.Sprintf("invalid path: '%s'", relPath)) } path, err := storage.ValidateRelativePath(rootPath, relPath) if err != nil { if errors.Is(err, storage.ErrRelativePathEscapesRoot) { - return "", indexError("Path cannot include directory traversal") + return "", git2go.IndexError("Path cannot include directory traversal") } return "", err diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go index e72151e9f..43c25aab9 100644 --- a/internal/gitaly/service/operations/commit_files_test.go +++ b/internal/gitaly/service/operations/commit_files_test.go @@ -103,6 +103,18 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) { steps []step }{ { + desc: "create file with .git/hooks/pre-commit", + steps: []step{ + { + actions: []*gitalypb.UserCommitFilesRequest{ + createFileHeaderRequest(".git/hooks/pre-commit"), + actionContentRequest("content-1"), + }, + indexError: "invalid path: '.git/hooks/pre-commit'", + }, + }, + }, + { desc: "create directory", steps: []step{ { |