diff --git a/Changelog.md b/Changelog.md index ec6ae1f..5a1bbe1 100644 --- a/Changelog.md +++ b/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: diff --git a/lib/zip/central_directory.rb b/lib/zip/central_directory.rb index 9975884..5b64c7f 100644 --- a/lib/zip/central_directory.rb +++ b/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) diff --git a/lib/zip/deflater.rb b/lib/zip/deflater.rb index 8509cf4..9f5371c 100644 --- a/lib/zip/deflater.rb +++ b/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 diff --git a/lib/zip/entry.rb b/lib/zip/entry.rb index c56dd05..2988b26 100644 --- a/lib/zip/entry.rb +++ b/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 diff --git a/lib/zip/extra_field/generic.rb b/lib/zip/extra_field/generic.rb index 5eb702d..9237a1d 100644 --- a/lib/zip/extra_field/generic.rb +++ b/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 diff --git a/lib/zip/extra_field/unix.rb b/lib/zip/extra_field/unix.rb index 9a66c81..d83087e 100644 --- a/lib/zip/extra_field/unix.rb +++ b/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) diff --git a/lib/zip/ioextras/abstract_input_stream.rb b/lib/zip/ioextras/abstract_input_stream.rb index 8392d24..848dcae 100644 --- a/lib/zip/ioextras/abstract_input_stream.rb +++ b/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 diff --git a/rubyzip.gemspec b/rubyzip.gemspec index 24e9ecf..67de78d 100644 --- a/rubyzip.gemspec +++ b/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", diff --git a/test/data/local_extra_field.zip b/test/data/local_extra_field.zip new file mode 100644 index 0000000..5a936e4 Binary files /dev/null and b/test/data/local_extra_field.zip differ diff --git a/test/encryption_test.rb b/test/encryption_test.rb index 7cb39ca..cad0525 100644 --- a/test/encryption_test.rb +++ b/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 diff --git a/test/extra_field_test.rb b/test/extra_field_test.rb index fa6e212..52140a2 100644 --- a/test/extra_field_test.rb +++ b/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 diff --git a/test/filesystem/file_nonmutating_test.rb b/test/filesystem/file_nonmutating_test.rb index 346d5a7..485298f 100644 --- a/test/filesystem/file_nonmutating_test.rb +++ b/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 diff --git a/test/filesystem/file_stat_test.rb b/test/filesystem/file_stat_test.rb index 05d7fff..b8efe75 100644 --- a/test/filesystem/file_stat_test.rb +++ b/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 diff --git a/test/input_stream_test.rb b/test/input_stream_test.rb index 773ee6b..34042ba 100644 --- a/test/input_stream_test.rb +++ b/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