[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

3 files changed, 24 insertions, 27 deletions

diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb
index 1a37ba4112..ba05fadbaf 100644
--- a/lib/rubygems/package.rb
+++ b/lib/rubygems/package.rb
@@ -347,6 +347,8 @@ EOM
         return @contents
       end
     end
+  rescue Zlib::GzipFile::Error, EOFError, Gem::Package::TarInvalidError => e
+    raise Gem::Package::FormatError.new e.message, @gem
   end
 
   ##
@@ -363,7 +365,7 @@ EOM
     algorithms.each do |algorithm|
       digester = Gem::Security.create_digest(algorithm)
 
-      digester << entry.read(16_384) until entry.eof?
+      digester << entry.readpartial(16_384) until entry.eof?
 
       entry.rewind
 
@@ -395,6 +397,8 @@ EOM
         break # ignore further entries
       end
     end
+  rescue Zlib::GzipFile::Error, EOFError, Gem::Package::TarInvalidError => e
+    raise Gem::Package::FormatError.new e.message, @gem
   end
 
   ##
@@ -626,7 +630,7 @@ EOM
     raise
   rescue Errno::ENOENT => e
     raise Gem::Package::FormatError.new e.message
-  rescue Gem::Package::TarInvalidError => e
+  rescue Zlib::GzipFile::Error, EOFError, Gem::Package::TarInvalidError => e
     raise Gem::Package::FormatError.new e.message, @gem
   end
 
diff --git a/lib/rubygems/package/tar_reader.rb b/lib/rubygems/package/tar_reader.rb
index b12e83a703..410cf2e0b3 100644
--- a/lib/rubygems/package/tar_reader.rb
+++ b/lib/rubygems/package/tar_reader.rb
@@ -14,11 +14,6 @@ class Gem::Package::TarReader
   include Enumerable
 
   ##
-  # Raised if the tar IO is not seekable
-
-  class UnexpectedEOF < StandardError; end
-
-  ##
   # Creates a new TarReader on +io+ and yields it to the block, if given.
 
   def self.new(io)
diff --git a/lib/rubygems/package/tar_reader/entry.rb b/lib/rubygems/package/tar_reader/entry.rb
index e22efa95b3..5e9d9af5c6 100644
--- a/lib/rubygems/package/tar_reader/entry.rb
+++ b/lib/rubygems/package/tar_reader/entry.rb
@@ -102,9 +102,7 @@ class Gem::Package::TarReader::Entry
   # Read one byte from the tar entry
 
   def getc
-    check_closed
-
-    return nil if @read >= @header.size
+    return nil if eof?
 
     ret = @io.getc
     @read += 1 if ret
@@ -156,30 +154,28 @@ class Gem::Package::TarReader::Entry
   alias_method :length, :size
 
   ##
-  # Reads +len+ bytes from the tar file entry, or the rest of the entry if
-  # nil
-
-  def read(len = nil)
-    check_closed
-
-    len ||= @header.size - @read
+  # Reads +maxlen+ bytes from the tar file entry, or the rest of the entry if nil
 
-    return nil if len > 0 && @read >= @header.size
+  def read(maxlen = nil)
+    if eof?
+      return maxlen.to_i.zero? ? "" : nil
+    end
 
-    max_read = [len, @header.size - @read].min
+    max_read = [maxlen, @header.size - @read].compact.min
 
     ret = @io.read max_read
+    if ret.nil?
+      return maxlen ? nil : "" # IO.read returns nil on EOF with len argument
+    end
     @read += ret.size
 
     ret
   end
 
-  def readpartial(maxlen = nil, outbuf = "".b)
-    check_closed
-
-    maxlen ||= @header.size - @read
-
-    raise EOFError if maxlen > 0 && @read >= @header.size
+  def readpartial(maxlen, outbuf = "".b)
+    if eof? && maxlen > 0
+      raise EOFError, "end of file reached"
+    end
 
     max_read = [maxlen, @header.size - @read].min
 
@@ -213,6 +209,8 @@ class Gem::Package::TarReader::Entry
 
     pending = new_pos - @io.pos
 
+    return 0 if pending == 0
+
     if @io.respond_to?(:seek)
       begin
         # avoid reading if the @io supports seeking
@@ -230,8 +228,8 @@ class Gem::Package::TarReader::Entry
     end
 
     while pending > 0 do
-      size_read = @io.read([pending, 4096].min).size
-      raise UnexpectedEOF if @io.eof?
+      size_read = @io.read([pending, 4096].min)&.size
+      raise(EOFError, "end of file reached") if size_read.nil?
       pending -= size_read
     end