Merge branch 'master' into compression_level

Changelog.md

@@ -1,6 +1,7 @@
 # X.X.X (Next)
 
 - Set compression level on a per-zipfile basis. [#448](https://github.com/rubyzip/rubyzip/pull/448)
+- Fix input stream partial read error. [#462](https://github.com/rubyzip/rubyzip/pull/462)
 
 Tooling:
 

lib/zip/central_directory.rb

@@ -124,10 +124,37 @@ module Zip
       end
       @entry_set = EntrySet.new
       @size.times do
-        @entry_set << Entry.read_c_dir_entry(io)
+        entry = Entry.read_c_dir_entry(io)
+        next unless entry
+
+        offset = if entry.extra['Zip64']
+                   entry.extra['Zip64'].relative_header_offset
+                 else
+                   entry.local_header_offset
+                 end
+
+        unless offset.nil?
+          io_save = io.tell
+          io.seek(offset, IO::SEEK_SET)
+          entry.read_extra_field(read_local_extra_field(io))
+          io.seek(io_save, IO::SEEK_SET)
+        end
+
+        @entry_set << entry
       end
     end
 
+    def read_local_extra_field(io)
+      buf = io.read(::Zip::LOCAL_ENTRY_STATIC_HEADER_LENGTH) || ''
+      return '' unless buf.bytesize == ::Zip::LOCAL_ENTRY_STATIC_HEADER_LENGTH
+
+      head, _, _, _, _, _, _, _, _, _, n_len, e_len = buf.unpack('VCCvvvvVVVvv')
+      return '' unless head == ::Zip::LOCAL_ENTRY_SIGNATURE
+
+      io.seek(n_len, IO::SEEK_CUR) # Skip over the entry name.
+      io.read(e_len)
+    end
+
     def read_from_stream(io) #:nodoc:
       buf = start_buf(io)
       if zip64_file?(buf)

lib/zip/deflater.rb

@@ -13,16 +13,16 @@ module Zip
       val   = data.to_s
       @crc  = Zlib.crc32(val, @crc)
       @size += val.bytesize
-      buffer = @zlib_deflater.deflate(data)
-      if buffer.empty?
-        @output_stream
-      else
-        @output_stream << @encrypter.encrypt(buffer)
-      end
+      buffer = @zlib_deflater.deflate(data, Zlib::SYNC_FLUSH)
+      return @output_stream if buffer.empty?
+
+      @output_stream << @encrypter.encrypt(buffer)
     end
 
     def finish
-      @output_stream << @encrypter.encrypt(@zlib_deflater.finish) until @zlib_deflater.finished?
+      buffer = @zlib_deflater.finish
+      @output_stream << @encrypter.encrypt(buffer) unless buffer.empty?
+      @zlib_deflater.close
     end
 
     attr_reader :size, :crc

lib/zip/entry.rb

@@ -303,12 +303,7 @@ module Zip
         raise ::Zip::Error, 'Truncated local zip entry header'
       end
 
-      if @extra.kind_of?(::Zip::ExtraField)
-        @extra.merge(extra) if extra
-      else
-        @extra = ::Zip::ExtraField.new(extra)
-      end
-
+      read_extra_field(extra)
       parse_zip64_extra(true)
       @local_header_size = calculate_local_header_size
     end
@@ -411,11 +406,11 @@ module Zip
       raise ::Zip::Error, 'Truncated cdir zip entry header'
     end
 
-    def read_c_dir_extra_field(io)
+    def read_extra_field(buf)
       if @extra.kind_of?(::Zip::ExtraField)
-        @extra.merge(io.read(@extra_length))
+        @extra.merge(buf) if buf
       else
-        @extra = ::Zip::ExtraField.new(io.read(@extra_length))
+        @extra = ::Zip::ExtraField.new(buf)
       end
     end
 
@@ -429,7 +424,7 @@ module Zip
       if ::Zip.force_entry_names_encoding
         @name.force_encoding(::Zip.force_entry_names_encoding)
       end
-      read_c_dir_extra_field(io)
+      read_extra_field(io.read(@extra_length))
       @comment = io.read(@comment_length)
       check_c_dir_entry_comment_size
       set_ftype_from_c_dir_entry

lib/zip/extra_field/generic.rb

@@ -22,15 +22,6 @@ module Zip
       [binstr[2, 2].unpack1('v'), binstr[4..-1]]
     end
 
-    def ==(other)
-      return false if self.class != other.class
-
-      each do |k, v|
-        return false if v != other[k]
-      end
-      true
-    end
-
     def to_local_bin
       s = pack_for_local
       self.class.const_get(:HEADER_ID) + [s.bytesize].pack('v') << s

lib/zip/extra_field/unix.rb

@@ -20,8 +20,8 @@ module Zip
       return if !size || size == 0
 
       uid, gid = content.unpack('vv')
-      @uid ||= uid
-      @gid ||= gid # rubocop:disable Naming/MemoizedInstanceVariableName
+      @uid = uid
+      @gid = gid
     end
 
     def ==(other)

lib/zip/ioextras/abstract_input_stream.rb

@@ -19,7 +19,7 @@ module Zip
 
       def read(number_of_bytes = nil, buf = '')
         tbuf = if @output_buffer.bytesize > 0
-                 if number_of_bytes <= @output_buffer.bytesize
+                 if number_of_bytes && number_of_bytes <= @output_buffer.bytesize
                    @output_buffer.slice!(0, number_of_bytes)
                  else
                    number_of_bytes -= @output_buffer.bytesize if number_of_bytes

rubyzip.gemspec

@@ -12,7 +12,7 @@ Gem::Specification.new do |s|
   s.summary               = 'rubyzip is a ruby module for reading and writing zip files'
   s.files                 = Dir.glob('{samples,lib}/**/*.rb') + %w[README.md TODO Rakefile]
   s.require_paths         = ['lib']
-  s.license               = 'BSD 2-Clause'
+  s.license               = 'BSD-2-Clause'
   s.metadata              = {
     'bug_tracker_uri'   => 'https://github.com/rubyzip/rubyzip/issues',
     'changelog_uri'     => "https://github.com/rubyzip/rubyzip/blob/v#{s.version}/Changelog.md",

test/encryption_test.rb

@@ -13,20 +13,28 @@ class EncryptionTest < MiniTest::Test
   end
 
   def test_encrypt
-    test_file = ::File.open(ENCRYPT_ZIP_TEST_FILE, 'rb').read
+    content = File.open(INPUT_FILE1, 'r').read
+    test_filename = 'top_secret_file.txt'
 
-    @rand = [250, 143, 107, 13, 143, 22, 155, 75, 228, 150, 12]
-    @output = ::Zip::DOSTime.stub(:now, ::Zip::DOSTime.new(2014, 12, 17, 15, 56, 24)) do
-      Random.stub(:rand, ->(_range) { @rand.shift }) do
-        Zip::OutputStream.write_buffer(::StringIO.new(''), Zip::TraditionalEncrypter.new('password')) do |zos|
-          zos.put_next_entry('file1.txt')
-          zos.write ::File.open(INPUT_FILE1).read
-        end.string
-      end
+    password = 'swordfish'
+
+    encrypted_zip = Zip::OutputStream.write_buffer(::StringIO.new(''), Zip::TraditionalEncrypter.new(password)) do |out|
+      out.put_next_entry(test_filename)
+      out.write content
     end
 
-    @output.unpack('C*').each_with_index do |c, i|
-      assert_equal test_file[i].ord, c
+    Zip::InputStream.open(encrypted_zip, 0, Zip::TraditionalDecrypter.new(password)) do |zis|
+      entry = zis.get_next_entry
+      assert_equal test_filename, entry.name
+      assert_equal 1327, entry.size
+      assert_equal content, zis.read
+    end
+
+    assert_raises(Zip::DecompressionError) do
+      Zip::InputStream.open(encrypted_zip, 0, Zip::TraditionalDecrypter.new(password + 'wrong')) do |zis|
+        zis.get_next_entry
+        assert_equal content, zis.read
+      end
     end
   end
 

test/extra_field_test.rb

@@ -17,6 +17,16 @@ class ZipExtraFieldTest < MiniTest::Test
     assert_equal(extra.to_s, 'fooabarbaz')
   end
 
+  def test_bad_header_id
+    str = "ut\x5\0\x3\250$\r@"
+    ut = nil
+    assert_output('', /WARNING/) do
+      ut = ::Zip::ExtraField::UniversalTime.new(str)
+    end
+    assert_instance_of(::Zip::ExtraField::UniversalTime, ut)
+    assert_nil(ut.mtime)
+  end
+
   def test_ntfs
     str = "\x0A\x00 \x00\x00\x00\x00\x00\x01\x00\x18\x00\xC0\x81\x17\xE8B\xCE\xCF\x01\xC0\x81\x17\xE8B\xCE\xCF\x01\xC0\x81\x17\xE8B\xCE\xCF\x01"
     extra = ::Zip::ExtraField.new(str)
@@ -25,6 +35,8 @@ class ZipExtraFieldTest < MiniTest::Test
     assert_equal(t, extra['NTFS'].mtime)
     assert_equal(t, extra['NTFS'].atime)
     assert_equal(t, extra['NTFS'].ctime)
+
+    assert_equal(str.force_encoding('BINARY'), extra.to_local_bin)
   end
 
   def test_merge
@@ -73,4 +85,16 @@ class ZipExtraFieldTest < MiniTest::Test
     extra1.create('IUnix')
     assert_equal(extra1, extra3)
   end
+
+  def test_read_local_extra_field
+    ::Zip::File.open('test/data/local_extra_field.zip') do |zf|
+      ['file1.txt', 'file2.txt'].each do |file|
+        entry = zf.get_entry(file)
+
+        assert_instance_of(::Zip::ExtraField, entry.extra)
+        assert_equal(1_000, entry.extra['IUnix'].uid)
+        assert_equal(1_000, entry.extra['IUnix'].gid)
+      end
+    end
+  end
 end

test/filesystem/file_nonmutating_test.rb

@@ -295,8 +295,10 @@ class ZipFsFileNonmutatingTest < MiniTest::Test
   end
 
   def test_atime
-    assert_nil(@zip_file.file.atime('file1'))
-    assert_nil(@zip_file.file.stat('file1').atime)
+    assert_equal(::Zip::DOSTime.at(1_027_694_306),
+                 @zip_file.file.atime('file1'))
+    assert_equal(::Zip::DOSTime.at(1_027_694_306),
+                 @zip_file.file.stat('file1').atime)
   end
 
   def test_ntfs_time

test/filesystem/file_stat_test.rb

@@ -19,11 +19,11 @@ class ZipFsFileStatTest < MiniTest::Test
   end
 
   def test_uid
-    assert_equal(0, @zip_file.file.stat('file1').uid)
+    assert_equal(500, @zip_file.file.stat('file1').uid)
   end
 
   def test_gid
-    assert_equal(0, @zip_file.file.stat('file1').gid)
+    assert_equal(500, @zip_file.file.stat('file1').gid)
   end
 
   def test_ftype

test/input_stream_test.rb

@@ -179,4 +179,14 @@ class ZipInputStreamTest < MiniTest::Test
       assert_equal('$VERBOSE =', zis.read(10))
     end
   end
+
+  def test_readline_then_read
+    ::Zip::InputStream.open(TestZipFile::TEST_ZIP2.zip_name) do |zis|
+      zis.get_next_entry
+      assert_equal("#!/usr/bin/env ruby\n", zis.readline)
+      refute(zis.eof?)
+      refute_empty(zis.read) # Also should not raise an error.
+      assert(zis.eof?)
+    end
+  end
 end