Merge branch 'sh-fix-delta-size-check' into 'master'

Fix delta size check to handle commit or nil objects

Closes #3446

See merge request gitlab-org/gitlab-ee!2976

changelogs/unreleased-ee/sh-fix-delta-size-check.yml

+---
+title: Fix delta size check to handle commit or nil objects
+merge_request:
+author:
+type: fixed

ee/lib/ee/gitlab/deltas.rb

@@ -10,9 +10,14 @@ def self.delta_size_check(change, repo)
           diff = tree_a.diff(tree_b)
           diff.each_delta do |d|
-            new_file_size = d.deleted? ? 0 : ::Gitlab::Git::Blob.raw(repo, d.new_file[:oid]).size
-            size_of_deltas += new_file_size
+            next if d.deleted?
+            blob = ::Gitlab::Git::Blob.raw(repo, d.new_file[:oid])
+
+            # It's possible the OID points to a commit or empty object
+            next unless blob
+
+            size_of_deltas += blob.size
           end
           size_of_deltas

lib/gitlab/git/blob.rb

@@ -32,6 +32,8 @@ def raw(repository, sha)
             else
               blob = repository.lookup(sha)
+              next unless blob.is_a?(Rugged::Blob)
+
               new(
                 id: blob.oid,
                 size: blob.size,

spec/ee/spec/lib/ee/gitlab/deltas_spec.rb

+require 'spec_helper'
+
+describe EE::Gitlab::Deltas do
+  let(:project) { create(:project, :repository) }
+
+  describe '.delta_size_check' do
+    it 'returns a non-zero file size' do
+      change = {
+        oldrev: TestEnv::BRANCH_SHA['feature'],
+        newrev: TestEnv::BRANCH_SHA['master']
+      }
+
+      expect(described_class.delta_size_check(change, project.repository)).to be > 0
+    end
+  end
+end

spec/lib/gitlab/git/blob_spec.rb

@@ -119,10 +119,13 @@
   shared_examples 'finding blobs by ID' do
     let(:raw_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::RubyBlob::ID) }
+    let(:bad_blob) { Gitlab::Git::Blob.raw(repository, SeedRepo::BigCommit::ID) }
+
     it { expect(raw_blob.id).to eq(SeedRepo::RubyBlob::ID) }
     it { expect(raw_blob.data[0..10]).to eq("require \'fi") }
     it { expect(raw_blob.size).to eq(669) }
     it { expect(raw_blob.truncated?).to be_falsey }
+    it { expect(bad_blob).to be_nil }
     context 'large file' do
       it 'limits the size of a large file' do