8000 Merge pull request #413 from hainesr/file-options · rubyzip/rubyzip@c9b6523 · GitHub
[go: up one dir, main page]

Skip to content

Commit c9b6523

Browse files
authored
Merge pull request #413 from hainesr/file-options
Fix `restore_times` option when extracting, and test file options
2 parents e43e360 + 1a21f39 commit c9b6523

File tree

4 files changed

+120
-12
lines changed

4 files changed

+120
-12
lines changed

lib/zip/dos_time.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ def dos_equals(other)
2929
to_i / 2 == other.to_i / 2
3030
end
3131

32+
# Create a DOSTime instance from a vanilla Time instance.
33+
def self.from_time(time)
34+
local(time.year, time.month, time.day, time.hour, time.min, time.sec)
35+
end
36+
3237
def self.parse_binary_dos_format(binaryDosDate, binaryDosTime)
3338
second = 2 * (0b11111 & binaryDosTime)
3439
minute = (0b11111100000 & binaryDosTime) >> 5

lib/zip/entry.rb

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -406,24 +406,28 @@ def get_extra_attributes_from_path(path) # :nodoc:
406406
@unix_uid = stat.uid
407407
@unix_gid = stat.gid
408408
@unix_perms = stat.mode & 0o7777
409+
@time = ::Zip::DOSTime.from_time(stat.mtime)
409410
end
410411

411-
def set_unix_permissions_on_path(dest_path)
412-
# BUG: does not update timestamps into account
412+
def set_unix_attributes_on_path(dest_path)
413413
# ignore setuid/setgid bits by default. honor if @restore_ownership
414414
unix_perms_mask = 0o1777
415415
unix_perms_mask = 0o7777 if @restore_ownership
416416
::FileUtils.chmod(@unix_perms & unix_perms_mask, dest_path) if @restore_permissions && @unix_perms
417417
::FileUtils.chown(@unix_uid, @unix_gid, dest_path) if @restore_ownership && @unix_uid && @unix_gid && ::Process.egid == 0
418-
# File::utimes()
418+
419+
# Restore the timestamp on a file. This will either have come from the
420+
# original source file that was copied into the archive, or from the
421+
# creation date of the archive if there was no original source file.
422+
::FileUtils.touch(dest_path, mtime: time) if @restore_times
419423
end
420424

421425
def set_extra_attributes_on_path(dest_path) # :nodoc:
422426
return unless file? || directory?
423427

424428
case @fstype
425429
when ::Zip::FSTYPE_UNIX
426-
set_unix_permissions_on_path(dest_path)
430+
set_unix_attributes_on_path(dest_path)
427431
end
428432
end
429433

@@ -601,8 +605,6 @@ def create_file(dest_path, _continue_on_exists_proc = proc { Zip.continue_on_exi
601605
end
602606
::File.open(dest_path, 'wb') do |os|
603607
get_input_stream do |is|
604-
set_extra_attributes_on_path(dest_path)
605-
606608
bytes_written = 0
607609
warned = false
608610
buf = ''.dup
@@ -621,6 +623,8 @@ def create_file(dest_path, _continue_on_exists_proc = proc { Zip.continue_on_exi
621623
end
622624
end
623625
end
626+
627+
set_extra_attributes_on_path(dest_path)
624628
end
625629

626630
def create_directory(dest_path)

lib/zip/file.rb

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,31 @@ class File < CentralDirectory
5151
DATA_BUFFER_SIZE = 8192
5252
IO_METHODS = [:tell, :seek, :read, :close]
5353

54+
DEFAULT_OPTIONS = {
55+
restore_ownership: false,
56+
restore_permissions: false,
57+
restore_times: false
58+
}.freeze
59+
5460
attr_reader :name
5561

56-
# default -> false
62+
# default -> false.
5763
attr_accessor :restore_ownership
58-
# default -> false
64+
65+
# default -> false, but will be set to true in a future version.
5966
attr_accessor :restore_permissions
60-
# default -> true
67+
68+
# default -> false, but will be set to true in a future version.
6169
attr_accessor :restore_times
70+
6271
# Returns the zip files comment, if it has one
6372
attr_accessor :comment
6473

6574
# Opens a zip archive. Pass true as the second parameter to create
6675
# a new archive if it doesn't exist already.
6776
def initialize(path_or_io, create = false, buffer = false, options = {})
6877
super()
78+
options = DEFAULT_OPTIONS.merge(options)
6979
@name = path_or_io.respond_to?(:path) ? path_or_io.path : path_or_io
7080
@comment = ''
7181
@create = create ? true : false # allow any truthy value to mean true
@@ -98,9 +108,9 @@ def initialize(path_or_io, create = false, buffer = false, options = {})
98108

99109
@stored_entries = @entry_set.dup
100110
@stored_comment = @comment
101-
@restore_ownership = options[:restore_ownership] || false
102-
@restore_permissions = options[:restore_permissions] || true
103-
@restore_times = options[:restore_times] || true
111+
@restore_ownership = options[:restore_ownership]
112+
@restore_permissions = options[:restore_permissions]
113+
@restore_times = options[:restore_times]
104114
end
105115

106116
class << self

test/file_options_test.rb

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
require 'test_helper'
2+
3+
class FileOptionsTest < MiniTest::Test
4+
ZIPPATH = ::File.join(Dir.tmpdir, 'options.zip').freeze
5+
TXTPATH = ::File.expand_path(::File.join('data', 'file1.txt'), __dir__).freeze
6+
TXTPATH_600 = ::File.join(Dir.tmpdir, 'file1.600.txt').freeze
7+
TXTPATH_755 = ::File.join(Dir.tmpdir, 'file1.755.txt').freeze
8+
EXTPATH_1 = ::File.join(Dir.tmpdir, 'extracted_1.txt').freeze
9+
EXTPATH_2 = ::File.join(Dir.tmpdir, 'extracted_2.txt').freeze
10+
EXTPATH_3 = ::File.join(Dir.tmpdir, 'extracted_3.txt').freeze
11+
ENTRY_1 = 'entry_1.txt'.freeze
12+
ENTRY_2 = 'entry_2.txt'.freeze
13+
ENTRY_3 = 'entry_3.txt'.freeze
14+
15+
def teardown
16+
::File.unlink(ZIPPATH) if ::File.exist?(ZIPPATH)
17+
::File.unlink(EXTPATH_1) if ::File.exist?(EXTPATH_1)
18+
::File.unlink(EXTPATH_2) if ::File.exist?(EXTPATH_2)
19+
::File.unlink(EXTPATH_3) if ::File.exist?(EXTPATH_3)
20+
::File.unlink(TXTPATH_600) if ::File.exist?(TXTPATH_600)
21+
::File.unlink(TXTPATH_755) if ::File.exist?(TXTPATH_755)
22+
end
23+
24+
def test_restore_permissions
25+
# Copy and set up files with different permissions.
26+
::FileUtils.cp(TXTPATH, TXTPATH_600)
27+
::File.chmod(0600, TXTPATH_600)
28+
::FileUtils.cp(TXTPATH, TXTPATH_755)
29+
::File.chmod(0755, TXTPATH_755)
30+
31+
::Zip::File.open(ZIPPATH, true) do |zip|
32+
zip.add(ENTRY_1, TXTPATH)
33+
zip.add(ENTRY_2, TXTPATH_600)
34+
zip.add(ENTRY_3, TXTPATH_755)
35+
end
36+
37+
::Zip::File.open(ZIPPATH, false, restore_permissions: true) do |zip|
38+
zip.extract(ENTRY_1, EXTPATH_1)
39+
zip.extract(ENTRY_2, EXTPATH_2)
40+
zip.extract(ENTRY_3, EXTPATH_3)
41+
end
42+
43+
assert_equal(::File.stat(TXTPATH).mode, ::File.stat(EXTPATH_1).mode)
44+
assert_equal(::File.stat(TXTPATH_600).mode, ::File.stat(EXTPATH_2).mode)
45+
assert_equal(::File.stat(TXTPATH_755).mode, ::File.stat(EXTPATH_3).mode)
46+
end
47+
48+
def test_restore_times_true
49+
::Zip::File.open(ZIPPATH, true) do |zip|
50+
zip.add(ENTRY_1, TXTPATH)
51+
zip.add_stored(ENTRY_2, TXTPATH)
52+
end
53+
54+
::Zip::File.open(ZIPPATH, false, restore_times: true) do |zip|
55+
zip.extract(ENTRY_1, EXTPATH_1)
56+
zip.extract(ENTRY_2, EXTPATH_2)
57+
end
58+
59+
assert_time_equal(::File.mtime(TXTPATH), ::File.mtime(EXTPATH_1))
60+
assert_time_equal(::File.mtime(TXTPATH), ::File.mtime(EXTPATH_2))
61+
end
62+
63+
def test_restore_times_false
64+
::Zip::File.open(ZIPPATH, true) do |zip|
65+
zip.add(ENTRY_1, TXTPATH)
66+
zip.add_stored(ENTRY_2, TXTPATH)
67+
end
68+
69+
::Zip::File.open(ZIPPATH, false, restore_times: false) do |zip|
70+
zip.extract(ENTRY_1, EXTPATH_1)
71+
zip.extract(ENTRY_2, EXTPATH_2)
72+
end
73+
74+
assert_time_equal(::Time.now, ::File.mtime(EXTPATH_1))
75+
assert_time_equal(::Time.now, ::File.mtime(EXTPATH_2))
76+
end
77+
78+
private
79+
80+
# Method to compare file times. DOS times only have 2 second accuracy.
81+
def assert_time_equal(expected, actual)
82+
assert_equal(expected.year, actual.year)
83+
assert_equal(expected.month, actual.month)
84+
assert_equal(expected.day, actual.day)
85+
assert_equal(expected.hour, actual.hour)
86+
assert_equal(expected.min, actual.min)
87+
assert_in_delta(expected.sec, actual.sec, 1)
88+
end
89+
end

0 commit comments

Comments
 (0)
0