Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 6.3.4

* Fix a bug whereby `rollback!` would cause an exception without any entries having been written yet (rollback on first entry).

## 6.3.3

* Make sure `Writable#<<` converts the strings it is given into binary if they are not already in binary. This fixes an issue where `Heuristic` would suddenly start forwarding strings as-is to downstream callees. There is a lot of spots where the string-to-write gets forwarded and converting in every single one will be quite wasteful, but it can be handy to do in a few key places.
Expand Down
9 changes: 7 additions & 2 deletions lib/zip_kit/streamer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,13 @@ def rollback!
end

# Create filler for the truncated or unusable local file entry that did get written into the output
filler_size_bytes = @out.tell - @offset_before_last_local_file_header
@files << Filler.new(filler_size_bytes)
# Only create a filler if a local file header was actually written (indicated by
# @offset_before_last_local_file_header being set). If it's nil, no header was written,
# so there's nothing to create a filler for.
if @offset_before_last_local_file_header
filler_size_bytes = @out.tell - @offset_before_last_local_file_header
@files << Filler.new(filler_size_bytes)
end
Comment thread
cursor[bot] marked this conversation as resolved.

@out.tell
end
Expand Down
2 changes: 1 addition & 1 deletion lib/zip_kit/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true

module ZipKit
VERSION = "6.3.3"
VERSION = "6.3.4"
end
30 changes: 30 additions & 0 deletions spec/zip_kit/streamer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -675,4 +675,34 @@ def stream_with_just_write.write(bytes)
zip.close
}.not_to raise_error # Offset validation should pass
end

it "handles rollback when first entry fails before header is written" do
# This test reproduces the issue described in https://github.com/julik/zip_kit/issues/26
# When the first entry fails to add (e.g., due to an invalid URL or other error
# before the local file header is written), rollback! should not fail with a TypeError.
# This can happen when using write_file with Heuristic, where an exception occurs
# in the block before the Heuristic decides which compression method to use and
# calls write_deflated_file/write_stored_file (which would set @offset_before_last_local_file_header).
zip = described_class.new(StringIO.new)

# Simulate a scenario where an exception occurs in the block passed to write_file
# before the Heuristic calls write_deflated_file or write_stored_file (which would
# set @offset_before_last_local_file_header). This happens when the exception
# occurs before close() is called on the Heuristic writable.
expect {
zip.write_file("test.txt") do |sink|
# Raise an error before any data is written, so the Heuristic hasn't decided
# which compression method to use yet, and no header has been written
raise "Invalid URL or other error"
end
}.to raise_error("Invalid URL or other error")

expect {
zip.rollback!
}.not_to raise_error

expect {
zip.close
}.not_to raise_error
end
end