diff options
author | Jacob Vosmaer <jacob@gitlab.com> | 2018-06-26 15:12:08 +0300 |
---|---|---|
committer | Jacob Vosmaer <jacob@gitlab.com> | 2018-06-26 15:12:08 +0300 |
commit | 33c950f293f13aa4c4e1b2c098695cba3cd36029 (patch) | |
tree | fd1c9d74223fa323263774938d318076e02c517b | |
parent | 955f0ea57f3ab3cc06124ecea795c1375b3eb92f (diff) |
Client-side fix for Gitaly TreeEntry bug
-rw-r--r-- | lib/gitlab/git/commit.rb | 41 | ||||
-rw-r--r-- | lib/gitlab/gitaly_client/commit_service.rb | 7 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 31 |
3 files changed, 30 insertions, 49 deletions
diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 341768752dc..74240cedc9d 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -493,13 +493,18 @@ module Gitlab def tree_entry(path) return unless path.present? - @repository.gitaly_migrate(:commit_tree_entry) do |is_migrated| - if is_migrated - gitaly_tree_entry(path) - else - rugged_tree_entry(path) - end - end + # We're only interested in metadata, so limit actual data to 1 byte + # since Gitaly doesn't support "send no data" option. + entry = @repository.gitaly_commit_client.tree_entry(id, path, 1) + return unless entry + + # To be compatible with the rugged format + entry = entry.to_h + entry.delete(:data) + entry[:name] = File.basename(path) + entry[:type] = entry[:type].downcase + + entry end def to_gitaly_commit @@ -562,28 +567,6 @@ module Gitlab SERIALIZE_KEYS end - def gitaly_tree_entry(path) - # We're only interested in metadata, so limit actual data to 1 byte - # since Gitaly doesn't support "send no data" option. - entry = @repository.gitaly_commit_client.tree_entry(id, path, 1) - return unless entry - - # To be compatible with the rugged format - entry = entry.to_h - entry.delete(:data) - entry[:name] = File.basename(path) - entry[:type] = entry[:type].downcase - - entry - end - - # Is this the same as Blob.find_entry_by_path ? - def rugged_tree_entry(path) - rugged_commit.tree.path(path) - rescue Rugged::TreeError - nil - end - def gitaly_commit_author_from_rugged(author_or_committer) Gitaly::CommitAuthor.new( name: author_or_committer[:name].b, diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 7f2e6441f16..077297b9d6d 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -76,6 +76,13 @@ module Gitlab end def tree_entry(ref, path, limit = nil) + if Pathname.new(path).cleanpath.to_s.start_with?('../') + # The TreeEntry RPC should return an empty reponse in this case but in + # Gitaly 0.107.0 and earlier we get an exception instead. This early return + # saves us a Gitaly roundtrip while also avoiding the exception. + return + end + request = Gitaly::TreeEntryRequest.new( repository: @gitaly_repo, revision: encode_binary(ref), diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 090f91168ad..5157d8fc645 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -514,30 +514,21 @@ eos end describe '#uri_type' do - shared_examples 'URI type' do - it 'returns the URI type at the given path' do - expect(commit.uri_type('files/html')).to be(:tree) - expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) - expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw) - expect(commit.uri_type('files/js/application.js')).to be(:blob) - end - - it "returns nil if the path doesn't exists" do - expect(commit.uri_type('this/path/doesnt/exist')).to be_nil - end - - it 'is nil if the path is nil or empty' do - expect(commit.uri_type(nil)).to be_nil - expect(commit.uri_type("")).to be_nil - end + it 'returns the URI type at the given path' do + expect(commit.uri_type('files/html')).to be(:tree) + expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) + expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw) + expect(commit.uri_type('files/js/application.js')).to be(:blob) end - context 'when Gitaly commit_tree_entry feature is enabled' do - it_behaves_like 'URI type' + it "returns nil if the path doesn't exists" do + expect(commit.uri_type('this/path/doesnt/exist')).to be_nil + expect(commit.uri_type('../path/doesnt/exist')).to be_nil end - context 'when Gitaly commit_tree_entry feature is disabled', :disable_gitaly do - it_behaves_like 'URI type' + it 'is nil if the path is nil or empty' do + expect(commit.uri_type(nil)).to be_nil + expect(commit.uri_type("")).to be_nil end end |