8000 Merge pull request #300 from hainesr/fix-create-perms · rubyzip/rubyzip@a0cf673 · GitHub
[go: up one dir, main page]
Skip to content

Commit a0cf673

Browse files
simonoffsimonoff
authored
Merge pull request #300 from hainesr/fix-create-perms
Fix permissions on new zip files (#294)
2 parents 82fa57c + c00d767 commit a0cf673

File tree

3 files changed

+63
-59
lines changed

3 files changed

+63
-59
lines changed

‎lib/zip/file.rb

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ module Zip
4343
# interface for accessing the filesystem, ie. the File and Dir classes.
4444

4545
class File < CentralDirectory
46-
CREATE = 1
46+
CREATE = true
4747
SPLIT_SIGNATURE = 0x08074b50
4848
ZIP64_EOCD_SIGNATURE = 0x06064b50
4949
MAX_SEGMENT_SIZE = 3_221_225_472
@@ -64,20 +64,19 @@ class File < CentralDirectory
6464

6565
# Opens a zip archive. Pass true as the second parameter to create
6666
# a new archive if it doesn't exist already.
67-
def initialize(file_name, create = nil, buffer = false, options = {})
67+
def initialize(file_name, create = false, buffer = false, options = {})
6868
super()
6969
@name = file_name
7070
@comment = ''
71-
@create = create
71+
@create = create ? true : false # allow any truthy value to mean true
7272
case
7373
when !buffer && ::File.size?(file_name)
74-
@create = nil
74+
@create = false
7575
@file_permissions = ::File.stat(file_name).mode
7676
::File.open(name, 'rb') do |f|
7777
read_from_stream(f)
7878
end
79-
when create
80-
@file_permissions = create_file_permissions
79+
when @create
8180
@entry_set = EntrySet.new
8281
when ::File.zero?(file_name)
8382
raise Error, "File #{file_name} has zero size. Did you mean to pass the create flag?"
@@ -95,7 +94,7 @@ class << self
9594
# Same as #new. If a block is passed the ZipFile object is passed
9695
# to the block and is automatically closed afterwards just as with
9796
# ruby's builtin File.open method.
98-
def open(file_name, create = nil)
97+
def open(file_name, create = false)
9998
zf = ::Zip::File.new(file_name, create)
10099
return zf unless block_given?
101100
begin
@@ -340,7 +339,7 @@ def commit_required?
340339
@entry_set.each do |e|
341340
return true if e.dirty
342341
end
343-
@comment != @stored_comment || @entry_set != @stored_entries || @create == ::Zip::File::CREATE
342+
@comment != @stored_comment || @entry_set != @stored_entries || @create
344343
end
345344

346345
# Searches for entry with the specified name. Returns nil if
@@ -403,27 +402,18 @@ def check_file(path)
403402
end
404403

405404
def on_success_replace
406-
tmp_filename = create_tmpname
407-
if yield tmp_filename
408-
::File.rename(tmp_filename, name)
409-
::File.chmod(@file_permissions, name) if defined?(@file_permissions)
410-
end
411-
ensure
412-
::File.unlink(tmp_filename) if ::File.exist?(tmp_filename)
413-
end
414-
415-
def create_tmpname
416405
dirname, basename = ::File.split(name)
417-
::Dir::Tmpname.create(basename, dirname) do |tmpname|
418-
opts = {perm: 0600, mode: ::File::CREAT | ::File::WRONLY | ::File::EXCL}
419-
f = File.open(tmpname, opts)
420-
f.close
406+
::Dir::Tmpname.create(basename, dirname) do |tmp_filename|
407+
begin
408+
if yield tmp_filename
409+
::File.rename(tmp_filename, name)
410+
::File.chmod(@file_permissions, name) unless @create
411+
end
412+
ensure
413+
::File.unlink(tmp_filename) if ::File.exist?(tmp_filename)
414+
end
421415
end
422416
end
423-
424-
def create_file_permissions
425-
::Zip::RUNNING_ON_WINDOWS ? 0644 : 0666 - ::File.umask
426-
end
427417
end
428418
end
429419

‎test/file_permissions_test.rb

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,58 +2,58 @@
22

33
class FilePermissionsTest < MiniTest::Test
44

5-
FILENAME = File.join(File.dirname(__FILE__), "umask.zip")
5+
ZIPNAME = File.join(File.dirname(__FILE__), "umask.zip")
6+
FILENAME = File.join(File.dirname(__FILE__), "umask.txt")
67

78
def teardown
9+
::File.unlink(ZIPNAME)
810
::File.unlink(FILENAME)
911
end
1012

11-
if ::Zip::RUNNING_ON_WINDOWS
12-
# Windows tests
13-
14-
DEFAULT_PERMS = 0644
15-
16-
def test_windows_perms
17-
create_file
13+
def test_current_umask
14+
create_files
15+
assert_matching_permissions FILENAME, ZIPNAME
16+
end
1817

19-
assert_equal DEFAULT_PERMS, ::File.stat(FILENAME).mode
18+
def test_umask_000
19+
set_umask(0000) do
20+
create_files
2021
end
2122

22-
else
23-
# Unix tests
24-
25-
DEFAULT_PERMS = 0100666
26-
27-
def test_current_umask
28-
umask = DEFAULT_PERMS - ::File.umask
29-
create_file
23+
assert_matching_permissions FILENAME, ZIPNAME
24+
end
3025

31-
assert_equal umask, ::File.stat(FILENAME).mode
26+
def test_umask_066
27+
set_umask(0066) do
28+
create_files
3229
end
3330

34-
def test_umask_000
35-
set_umask(0000) do
36-
create_file
37-
end
31+
assert_matching_permissions FILENAME, ZIPNAME
32+
end
3833

39-
assert_equal DEFAULT_PERMS, ::File.stat(FILENAME).mode
34+
def test_umask_027
35+
set_umask(0027) do
36+
create_files
4037
end
4138

42-
def test_umask_066
43-
umask = 0066
44-
set_umask(umask) do
45-
create_file
46-
end
47-
48-
assert_equal((DEFAULT_PERMS - umask), ::File.stat(FILENAME).mode)
49-
end
39+
assert_matching_permissions FILENAME, ZIPNAME
40+
end
5041

42+
def assert_matching_permissions(expected_file, actual_file)
43+
assert_equal(
44+
::File.stat(expected_file).mode.to_s(8).rjust(4, '0'),
45+
::File.stat(actual_file).mode.to_s(8).rjust(4, '0')
46+
)
5147
end
5248

53-
def create_file
54-
::Zip::File.open(FILENAME, ::Zip::File::CREATE) do |zip|
49+
def create_files
50+
::Zip::File.open(ZIPNAME, ::Zip::File::CREATE) do |zip|
5551
zip.comment = "test"
5652
end
53+
54+
::File.open(FILENAME, 'w') do |file|
55+
file << 'test'
56+
end
5757
end
5858

5959
# If anything goes wrong, make sure the umask is restored.

‎test/file_test.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,20 @@ def test_create_from_scratch
4141
assert_equal(2, zfRead.entries.length)
4242
end
4343

44+
def test_create_from_scratch_with_old_create_parameter
45+
comment = 'a short comment'
46+
47+
zf = ::Zip::File.new(EMPTY_FILENAME, 1)
48+
zf.get_output_stream('myFile') { |os| os.write 'myFile contains just this' }
49+
zf.mkdir('dir1')
50+
zf.comment = comment
51+
zf.close
52+
53+
zfRead = ::Zip::File.new(EMPTY_FILENAME)
54+
assert_equal(comment, zfRead.comment)
55+
assert_equal(2, zfRead.entries.length)
56+
end
57+
4458
def test_get_output_stream
4559
entryCount = nil
4660
::Zip::File.open(TEST_ZIP.zip_name) do |zf|

0 commit comments

Comments
 (0)
0