Fix gitaly ref encoding bugs
What does this MR do?
Adds missing encoding coercion calls on Gitaly ref responses.
Are there points in the code the reviewer needs to double check?
Why was this MR needed?
We were injecting ASCII-8BIT strings into GitLab, that doesn't work. It has to be UTF-8.
Screenshots (if relevant)
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated -
API support added - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Branch has no merge conflicts with master
(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Merge request reports
Activity
- Resolved by Jacob Vosmaer
changed milestone to %9.4
changed milestone to %9.3
assigned to @rspeicher
- Resolved by Jacob Vosmaer
Some polish in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12526; that's separate because I want to get the bug fix in as a small commit.
mentioned in merge request !12526 (merged)
- Resolved by Robert Speicher
64 65 end 65 66 66 67 def consume_branches_response(response) 67 response.flat_map { |r| r.branches } 68 response.flat_map do |message| 69 message.branches.map do |gitaly_branch| 70 Gitlab::Git::Branch.new( 71 @repository, 72 encode!(gitaly_branch.name.dup), 73 gitaly_branch.commit_id mentioned in commit 9f44687a
mentioned in commit 20bb1c99
@jacobvosmaer-gitlab Attempting to pick this into https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12508 for
9.3.3
resulted in conflicts. I attempted to fix them with https://gitlab.com/gitlab-org/gitlab-ce/commit/20bb1c99ca4d8fb3b57e034325f6bb1fce179753, but that resulted in the following failures:1) Gitlab::Git::Repository#ref_name_for_sha returns an empty name if the ref doesn't exist Failure/Error: expect(repository.ref_name_for_sha(ref_path, "000000")).to eq("") expected: "" got: "commit" (compared using ==) # ./spec/lib/gitlab/git/repository_spec.rb:1089:in `block (3 levels) in <top (required)>' # -e:1:in `<main>' 2) Gitlab::Git::Repository#local_branches with gitaly enabled returns a Branch with UTF-8 fields Failure/Error: consume_branches_response(stub.find_local_branches(request)) #<Double :stub> received unexpected message :find_local_branches with (<Gitaly::FindLocalBranchesRequest: repository: <Gitaly::Repository: path: "/Users/jedwardsjones/gitla...le-repo.git/.git", storage_name: "default", relative_path: "mutable-repo.git/.git">, sort_by: :NAME>) # ./lib/gitlab/gitaly_client/ref.rb:53:in `local_branches' # ./lib/gitlab/git/repository.rb:116:in `block in local_branches' # ./lib/gitlab/gitaly_client.rb:77:in `block in migrate' # ./lib/gitlab/metrics/influx_db.rb:92:in `measure' # ./lib/gitlab/gitaly_client.rb:76:in `migrate' # ./lib/gitlab/git/repository.rb:1245:in `gitaly_migrate' # ./lib/gitlab/git/repository.rb:114:in `local_branches' # ./spec/lib/gitlab/git/repository_spec.rb:1301:in `block (4 levels) in <top (required)>' # -e:1:in `<main>'
mentioned in merge request !12508 (merged)
I can't reproduce that test failure at https://gitlab.com/gitlab-org/gitlab-ce/commit/20bb1c99ca4d8fb3b57e034325f6bb1fce179753 locally (yet).
Edit: also, worth pointing out: that commit is green in CI.
Edited by Jacob Vosmaermentioned in commit 00b03d9f
Picked into https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12508, will merge into
9.3-stable
ready for9.3.3
added devopscreate groupgitaly labels