Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync options and fix inconsistent behaviour of Zip::File#get_entry and Zip::File#find_entry. #423

Merged
merged 4 commits into from Dec 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/zip/entry.rb
Expand Up @@ -34,7 +34,7 @@ def set_default_vars_values
end
@follow_symlinks = false

@restore_times = true
@restore_times = false
@restore_permissions = false
@restore_ownership = false
# BUG: need an extra field to support uid/gid's
Expand Down
14 changes: 9 additions & 5 deletions lib/zip/file.rb
Expand Up @@ -376,7 +376,13 @@ def commit_required?
# Searches for entry with the specified name. Returns nil if
# no entry is found. See also get_entry
def find_entry(entry_name)
@entry_set.find_entry(entry_name)
selected_entry = @entry_set.find_entry(entry_name)
return if selected_entry.nil?

selected_entry.restore_ownership = @restore_ownership
selected_entry.restore_permissions = @restore_permissions
selected_entry.restore_times = @restore_times
selected_entry
end

# Searches for entries given a glob
Expand All @@ -388,10 +394,8 @@ def glob(*args, &block)
# if no entry is found.
def get_entry(entry)
selected_entry = find_entry(entry)
raise Errno::ENOENT, entry unless selected_entry
selected_entry.restore_ownership = @restore_ownership
selected_entry.restore_permissions = @restore_permissions
selected_entry.restore_times = @restore_times
raise Errno::ENOENT, entry if selected_entry.nil?

selected_entry
end

Expand Down
18 changes: 18 additions & 0 deletions test/file_options_test.rb
Expand Up @@ -75,6 +75,24 @@ def test_restore_times_false
assert_time_equal(::Time.now, ::File.mtime(EXTPATH_2))
end

def test_get_find_consistency
testzip = ::File.expand_path(::File.join('data', 'globTest.zip'), __dir__)
file_f = ::File.expand_path('f_test.txt', Dir.tmpdir)
file_g = ::File.expand_path('g_test.txt', Dir.tmpdir)

::Zip::File.open(testzip) do |zip|
e1 = zip.find_entry('globTest/food.txt')
e1.extract(file_f)
e2 = zip.get_entry('globTest/food.txt')
e2.extract(file_g)
end

assert_time_equal(::File.mtime(file_f), ::File.mtime(file_g))
ensure
::File.unlink(file_f)
::File.unlink(file_g)
end

private

# Method to compare file times. DOS times only have 2 second accuracy.
Expand Down
15 changes: 15 additions & 0 deletions test/file_test.rb
Expand Up @@ -653,6 +653,21 @@ def test_open_xls_does_not_raise_type_error
::Zip::File.open('test/data/test.xls')
end

def test_find_get_entry
::Zip::File.open(TEST_ZIP.zip_name) do |zf|
assert_nil zf.find_entry('not_in_here.txt')

refute_nil zf.find_entry('test/data/generated/empty.txt')

assert_raises(Errno::ENOENT) do
zf.get_entry('not_in_here.txt')
end

# Should not raise anything.
zf.get_entry('test/data/generated/empty.txt')
end
end

private

def assert_contains(zf, entryName, filename = entryName)
Expand Down