Skip to content

Commit

Permalink
Merge pull request #423 from hainesr/sync-options
Browse files Browse the repository at this point in the history
Sync options and fix inconsistent behaviour of `Zip::File#get_entry` and `Zip::File#find_entry`.
  • Loading branch information
jdleesmiller committed Dec 15, 2019
2 parents 6389d65 + f3a2f4a commit c925bdb
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 6 deletions.
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

0 comments on commit c925bdb

Please sign in to comment.