Skip to content

Commit

Permalink
Fix OutputStream#put_next_entry to preserve StreamableStreams.
Browse files Browse the repository at this point in the history
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
  • Loading branch information
hainesr committed Jan 18, 2022
1 parent 4cf801c commit 6fc42a2
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 19 deletions.
23 changes: 15 additions & 8 deletions lib/zip/entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,21 @@ def initialize(
@ftype = name_is_directory? ? :directory : :file

@zipfile = zipfile
@comment = comment
@compression_method = compression_method
@compression_level = compression_level

@compressed_size = compressed_size
@crc = crc
@size = size
@time = time
@comment = comment || ''
@compression_method = compression_method || DEFLATED
@compression_level = compression_level || ::Zip.default_compression

@compressed_size = compressed_size || 0
@crc = crc || 0
@size = size || 0
@time = case time
when ::Zip::DOSTime
time
when Time
::Zip::DOSTime.from_time(time)
else
::Zip::DOSTime.now
end
@extra =
extra.kind_of?(ExtraField) ? extra : ExtraField.new(extra.to_s)

Expand Down
18 changes: 9 additions & 9 deletions lib/zip/output_stream.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,15 @@ def put_next_entry(
)
raise Error, 'zip stream is closed' if @closed

new_entry = if entry_name.kind_of?(Entry)
entry_name
else
Entry.new(
@file_name, entry_name.to_s, comment: comment,
extra: extra, compression_method: compression_method,
compression_level: level
)
end
new_entry =
if entry_name.kind_of?(Entry) || entry_name.kind_of?(StreamableStream)
entry_name
else
Entry.new(
@file_name, entry_name.to_s, comment: comment, extra: extra,
compression_method: compression_method, compression_level: level
)
end

init_next_entry(new_entry)
@current_entry = new_entry
Expand Down
5 changes: 4 additions & 1 deletion test/entry_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ def test_constructor_and_getters
assert_equal(TEST_COMPRESSIONMETHOD, entry.compression_method)
assert_equal(TEST_NAME, entry.name)
assert_equal(TEST_SIZE, entry.size)
assert_equal(TEST_TIME, entry.time)

# Reverse times when testing because we need to use DOSTime#== for the
# comparison, not Time#==.
assert_equal(entry.time, TEST_TIME)
end

def test_is_directory_and_is_file
Expand Down
23 changes: 22 additions & 1 deletion test/file_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ def test_get_output_stream
custom_entry_args[:compression_level], entry.compression_level
)
assert_equal(custom_entry_args[:size], entry.size)
assert_equal(custom_entry_args[:time], entry.time)

# Reverse times when testing because we need to use DOSTime#== for the
# comparison, not Time#==.
assert_equal(entry.time, custom_entry_args[:time])

zf.get_output_stream('entry.bin') do |os|
os.write(::File.open('test/data/generated/5entry.zip', 'rb').read)
Expand All @@ -110,6 +113,24 @@ def test_get_output_stream
end
end

def test_get_output_stream_with_entry
Dir.mktmpdir do |tmp|
test_zip = File.join(tmp, 'test.zip')
time = Time.new(1999, 12, 31)

::Zip::File.open(test_zip, create: true) do |zip|
entry = ::Zip::Entry.new(zip.name, 'entry.txt', time: time)
zip.get_output_stream(entry) { |out| out.puts 'CONTENT!' }
end

::Zip::File.open(test_zip) do |zip|
# Reverse times when testing because we need to use DOSTime#== for the
# comparison, not Time#==.
assert_equal(zip.get_entry('entry.txt').time, time)
end
end
end

def test_open_buffer_with_string
data = File.read('test/data/rubycode.zip', mode: 'rb')
string = data.dup
Expand Down

0 comments on commit 6fc42a2

Please sign in to comment.