Merge branch 'sh-pages-eof-error' into 'master'

Fix EOF detection with CI artifacts metadata

Closes #36954

See merge request gitlab-org/gitlab-ce!22479

4 files changed, 88 insertions, 2 deletions

diff --git a/changelogs/unreleased/sh-pages-eof-error.yml b/changelogs/unreleased/sh-pages-eof-error.yml
new file mode 100644
index 00000000000..497a74c1458
--- /dev/null
+++ b/changelogs/unreleased/sh-pages-eof-error.yml
@@ -0,0 +1,5 @@
+---
+title: Fix EOF detection with CI artifacts metadata
+merge_request: 22479
+author:
+type: fixed
diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb
index 375d8bc1ff5..551d4f4473e 100644
--- a/lib/gitlab/ci/build/artifacts/metadata.rb
+++ b/lib/gitlab/ci/build/artifacts/metadata.rb
@@ -59,9 +59,12 @@ module Gitlab
 
             until gz.eof?
               begin
-                path = read_string(gz).force_encoding('UTF-8')
-                meta = read_string(gz).force_encoding('UTF-8')
+                path = read_string(gz)&.force_encoding('UTF-8')
+                meta = read_string(gz)&.force_encoding('UTF-8')
 
+                # We might hit an EOF while reading either value, so we should
+                # abort if we don't get any data.
+                next unless path && meta
                 next unless path.valid_encoding? && meta.valid_encoding?
                 next unless path =~ match_pattern
                 next if path =~ INVALID_PATH_PATTERN
diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb
index e327399d82d..a9a4af1f455 100644
--- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb
+++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb
@@ -112,4 +112,34 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do
       end
     end
   end
+
+  context 'generated metadata' do
+    let(:tmpfile) { Tempfile.new('test-metadata') }
+    let(:generator) { CiArtifactMetadataGenerator.new(tmpfile) }
+    let(:entry_count) { 5 }
+
+    before do
+      tmpfile.binmode
+
+      (1..entry_count).each do |index|
+        generator.add_entry("public/test-#{index}.txt")
+      end
+
+      generator.write
+    end
+
+    after do
+      File.unlink(tmpfile.path)
+    end
+
+    describe '#find_entries!' do
+      it 'reads expected number of entries' do
+        stream = File.open(tmpfile.path)
+
+        metadata = described_class.new(stream, 'public', { recursive: true })
+
+        expect(metadata.find_entries!.count).to eq entry_count
+      end
+    end
+  end
 end
diff --git a/spec/support/helpers/ci_artifact_metadata_generator.rb b/spec/support/helpers/ci_artifact_metadata_generator.rb
new file mode 100644
index 00000000000..ef638d59d2d
--- /dev/null
+++ b/spec/support/helpers/ci_artifact_metadata_generator.rb
@@ -0,0 +1,48 @@
+# frozen_sting_literal: true
+
+# This generates fake CI metadata .gz for testing
+# Based off https://gitlab.com/gitlab-org/gitlab-workhorse/blob/master/internal/zipartifacts/metadata.go
+class CiArtifactMetadataGenerator
+  attr_accessor :entries, :output
+
+  ARTIFACT_METADATA = "GitLab Build Artifacts Metadata 0.0.2\n".freeze
+
+  def initialize(stream)
+    @entries = {}
+    @output = Zlib::GzipWriter.new(stream)
+  end
+
+  def add_entry(filename)
+    @entries[filename] = { CRC: rand(0xfffffff), Comment: FFaker::Lorem.sentence(10) }
+  end
+
+  def write
+    write_version
+    write_errors
+    write_entries
+    output.close
+  end
+
+  private
+
+  def write_version
+    write_string(ARTIFACT_METADATA)
+  end
+
+  def write_errors
+    write_string('{}')
+  end
+
+  def write_entries
+    entries.each do |filename, metadata|
+      write_string(filename)
+      write_string(metadata.to_json + "\n")
+    end
+  end
+
+  def write_string(data)
+    bytes = [data.length].pack('L>')
+    output.write(bytes)
+    output.write(data)
+  end
+end