diff options
| author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-12-14 11:16:37 +0300 |
|---|---|---|
| committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-12-14 17:30:50 +0300 |
| commit | d87059eac752a1c64116bea0e0c073b0d40e1de7 (patch) | |
| tree | d97e5a0bac853be4d84d5e0b89958b81511dad1f /internal/gitaly/service/operations/merge.go | |
| parent | d5a9a458e393f3bccbec1a12e71d49179912457b (diff) | |
operations: Fix Go UserMergeBranch failing with ambiguous references
The UserMergeBranch RPC gets as input the branch name that is to be
merged into. This branch name is not fully qualified, so instead of
passing in "refs/heads/branch" the user only passes in "branch". We thus
need to make sure to convert it into a fully qualified reference, lest
any ambiguous reference may cause us to misbehave.
This causes us to mibehave with both Ruby and Go implementations of
UserMergeBranch. In Ruby, we silently compute the merge with the tag
instead of with the branch as the first parent, which is kind of the
worst thing we can do as it silently corrupts data. In Go, we at least
raise an error, which is arguably better behaviour compared to silently
computing the wrong merge.
This commit introduces a test which would trigger faulty behaviour in
both implementations by creating such an ambiguous reference and fixes
the bug in the Go implementation. The Ruby fix is likely more involved,
which may not be worthwhile to fix considering it's currently being
phased out anyway and has been a long standing bug. As such, the bug is
simply left as-is.
Diffstat (limited to 'internal/gitaly/service/operations/merge.go')
| -rw-r--r-- | internal/gitaly/service/operations/merge.go | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go index f4c729bb8..6dd16f82a 100644 --- a/internal/gitaly/service/operations/merge.go +++ b/internal/gitaly/service/operations/merge.go @@ -114,7 +114,8 @@ func (s *Server) userMergeBranch(stream gitalypb.OperationService_UserMergeBranc return err } - revision, err := git.NewRepository(repo).ResolveRefish(ctx, string(firstRequest.Branch)) + branch := "refs/heads/" + text.ChompBytes(firstRequest.Branch) + revision, err := git.NewRepository(repo).ResolveRefish(ctx, branch) if err != nil { return err } @@ -148,7 +149,6 @@ func (s *Server) userMergeBranch(stream gitalypb.OperationService_UserMergeBranc return helper.ErrPreconditionFailedf("merge aborted by client") } - branch := "refs/heads/" + text.ChompBytes(firstRequest.Branch) if err := s.updateReferenceWithHooks(ctx, firstRequest.Repository, firstRequest.User, branch, merge.CommitID, revision); err != nil { var preReceiveError preReceiveError var updateRefError updateRefError |