[rubygems/rubygems] Raise Gem::Package::FormatError on EOF, indicating corrupt gem

Gem::Package::TarReader::Entry now raises EOFError or returns nil
appropriately based on Ruby core IO.read and IO.readpartial behavior.

Zlib will respond accordingly by raising Zlib::GzipFile::Error on EOF.

When verifying a gem or extracting contents, raise FormatError similar
to other cases of corrupt gems.

Addresses a bug where Gem::Package would attempt to call size on nil
instead of raising a more descriptive and useful error, leading users
to assume the problem is internal to rubygems.

Remove unused error class TarReader::UnexpectedEOF that was never raised
since the NoMethodError on nil would happen first. Use EOFError instead.

https://github.com/rubygems/rubygems/commit/dc6129644b

2 files changed, 130 insertions, 1 deletions

diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb
index 1adb780158..4ca806efc3 100644
--- a/test/rubygems/test_gem_package.rb
+++ b/test/rubygems/test_gem_package.rb
@@ -946,6 +946,87 @@ class TestGemPackage < Gem::Package::TarTestCase
     tf.close!
   end
 
+  def test_verify_corrupt_tar_metadata_entry
+    gem = tar_file_header("metadata.gz", "", 0, 999, Time.now)
+
+    File.open "corrupt.gem", "wb" do |io|
+      io.write gem
+    end
+
+    package = Gem::Package.new "corrupt.gem"
+
+    e = assert_raise Gem::Package::FormatError do
+      package.verify
+    end
+
+    assert_match(/(EOFError|end of file reached) in corrupt.gem/i, e.message)
+  end
+
+  def test_verify_corrupt_tar_checksums_entry
+    gem = tar_file_header("checksums.yaml.gz", "", 0, 100, Time.now)
+
+    File.open "corrupt.gem", "wb" do |io|
+      io.write gem
+    end
+
+    package = Gem::Package.new "corrupt.gem"
+
+    e = assert_raise Gem::Package::FormatError do
+      package.verify
+    end
+
+    assert_equal "not in gzip format in corrupt.gem", e.message
+  end
+
+  def test_verify_corrupt_tar_data_entry
+    gem = tar_file_header("data.tar.gz", "", 0, 100, Time.now)
+
+    File.open "corrupt.gem", "wb" do |io|
+      io.write gem
+    end
+
+    package = Gem::Package.new "corrupt.gem"
+
+    e = assert_raise Gem::Package::FormatError do
+      package.verify
+    end
+
+    assert_match(/(EOFError|end of file reached) in corrupt.gem/i, e.message)
+  end
+
+  def test_corrupt_data_tar_gz
+    data_tgz = util_gzip tar_file_header("lib/code.rb", "", 0, 100, Time.now)
+    metadata_gz = util_gzip @spec.to_yaml
+
+    gem = util_tar do |tar|
+      tar.add_file "data.tar.gz", 0o444 do |io|
+        io.write data_tgz
+      end
+
+      tar.add_file "metadata.gz", 0o644 do |io|
+        io.write metadata_gz
+      end
+    end
+
+    File.open "corrupt.gem", "wb" do |io|
+      io.write gem.string
+    end
+
+    package = Gem::Package.new "corrupt.gem"
+
+    e = assert_raise Gem::Package::FormatError do
+      package.contents
+    end
+
+    assert_match(/(EOFError|end of file reached) in corrupt.gem/i, e.message)
+
+    e = assert_raise Gem::Package::FormatError do
+      package.extract_files @destination
+    end
+
+    assert_match(/(EOFError|end of file reached) in corrupt.gem/i, e.message)
+  end
+
   def test_verify_empty
     FileUtils.touch "empty.gem"
 
diff --git a/test/rubygems/test_gem_package_tar_reader_entry.rb b/test/rubygems/test_gem_package_tar_reader_entry.rb
index d8c9ba5665..7510e1d199 100644
--- a/test/rubygems/test_gem_package_tar_reader_entry.rb
+++ b/test/rubygems/test_gem_package_tar_reader_entry.rb
@@ -181,10 +181,15 @@ class TestGemPackageTarReaderEntry < Gem::Package::TarTestCase
     assert_equal @contents[100..-1], @entry.read
   end
 
-  def test_read_partial
+  def test_readpartial
     assert_equal @contents[0...100], @entry.readpartial(100)
   end
 
+  def test_readpartial_to_eof
+    assert_equal @contents, @entry.readpartial(4096)
+    assert @entry.eof?
+  end
+
   def test_read_partial_buffer
     buffer = "".b
     @entry.readpartial(100, buffer)
@@ -193,11 +198,38 @@ class TestGemPackageTarReaderEntry < Gem::Package::TarTestCase
 
   def test_readpartial_past_eof
     @entry.readpartial(@contents.size)
+    assert @entry.eof?
     assert_raise(EOFError) do
       @entry.readpartial(1)
     end
   end
 
+  def test_read_corrupted_tar
+    corrupt_tar = String.new
+    corrupt_tar << tar_file_header("lib/foo", "", 0, 100, Time.now)
+    corrupt_tar << tar_file_contents("")
+    corrupt_entry = util_entry corrupt_tar
+
+    assert_equal "", corrupt_entry.read(0)
+    assert_equal "", corrupt_entry.read, "IO.read without len should return empty string (even though it's at an unpexpected EOF)"
+
+    corrupt_entry.rewind
+
+    assert_nil corrupt_entry.read(100), "IO.read with len should return nil as per IO.read docs"
+  end
+
+  def test_readpartial_corrupted_tar
+    corrupt_tar = String.new
+    corrupt_tar << tar_file_header("lib/foo", "", 0, 100, Time.now)
+    corrupt_tar << tar_file_contents("")
+
+    corrupt_entry = util_entry corrupt_tar
+
+    assert_raise EOFError do
+      corrupt_entry.readpartial(100)
+    end
+  end
+
   def test_rewind
     char = @entry.getc
 
@@ -303,4 +335,20 @@ class TestGemPackageTarReaderEntry < Gem::Package::TarTestCase
       assert_equal contents2.size, entry.pos
     end
   end
+
+  def test_seek_in_gzip_io_corrupted
+    @tar << tar_file_header("lib/bar", "", 0, 100, Time.now)
+    @tar << tar_file_contents("")
+
+    tgz = util_gzip(@tar)
+
+    Zlib::GzipReader.wrap StringIO.new(tgz) do |gzio|
+      util_entry(gzio).close # skip the first entry so io.pos is not 0
+      entry = util_entry(gzio)
+
+      assert_raise EOFError do
+        entry.seek(50)
+      end
+    end
+  end
 end