From 9acc50f0046daddf650dad2e9ad15399c8cee8c7 Mon Sep 17 00:00:00 2001
From: Robert Haines <rhaines@manchester.ac.uk>
Date: Tue, 30 Dec 2014 10:39:57 +0000
Subject: [PATCH] Do something more expected with new file permissions.

Instead of inheriting the permissions from the tmp directory, new files should
have permissions that reflect the defaults on the system taking umask into
account.

It seems that (unix) permissions of 666 - umask are as close to a standard as
anything [1] and that 'touch' uses this. On Windows it seems sensible to just
use 644 directly [2].

[1] http://unix.stackexchange.com/a/102080
[2] http://ruby-doc.org/core-1.9.3/File.html
---
 lib/zip/file.rb               |  9 ++++-
 test/file_permissions_test.rb | 69 +++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 test/file_permissions_test.rb

diff --git a/lib/zip/file.rb b/lib/zip/file.rb
index 3cc4c35..4a5267d 100644
--- a/lib/zip/file.rb
+++ b/lib/zip/file.rb
@@ -71,11 +71,12 @@ module Zip
       case
       when !buffer && ::File.size?(file_name)
         @create = nil
-        @exist_file_perms = ::File.stat(file_name).mode
+        @file_permissions = ::File.stat(file_name).mode
         ::File.open(name, 'rb') do |f|
           read_from_stream(f)
         end
       when create
+        @file_permissions = create_file_permissions
         @entry_set = EntrySet.new
       else
         raise Error, "File #{file_name} not found"
@@ -405,7 +406,7 @@ module Zip
       tmpfile.close
       if yield tmp_filename
         ::File.rename(tmp_filename, name)
-        ::File.chmod(@exist_file_perms, name) if defined?(@exist_file_perms)
+        ::File.chmod(@file_permissions, name) if defined?(@file_permissions)
       end
     ensure
       tmpfile.unlink if tmpfile
@@ -416,6 +417,10 @@ module Zip
       temp_file.binmode
       temp_file
     end
+
+    def create_file_permissions
+      ::Zip::RUNNING_ON_WINDOWS ? 0644 : 0666 - ::File.umask
+    end
   end
 end
 
diff --git a/test/file_permissions_test.rb b/test/file_permissions_test.rb
new file mode 100644
index 0000000..67ae30b
--- /dev/null
+++ b/test/file_permissions_test.rb
@@ -0,0 +1,69 @@
+require 'test_helper'
+
+class FilePermissionsTest < MiniTest::Test
+
+  FILENAME = File.join(File.dirname(__FILE__), "umask.zip")
+
+  def teardown
+    ::File.unlink(FILENAME)
+  end
+
+  if ::Zip::RUNNING_ON_WINDOWS
+    # Windows tests
+
+    DEFAULT_PERMS = 0644
+
+    def test_windows_perms
+      create_file
+
+      assert_equal DEFAULT_PERMS, ::File.stat(FILENAME).mode
+    end
+
+  else
+    # Unix tests
+
+    DEFAULT_PERMS = 0100666
+
+    def test_current_umask
+      umask = DEFAULT_PERMS - ::File.umask
+      create_file
+
+      assert_equal umask, ::File.stat(FILENAME).mode
+    end
+
+    def test_umask_000
+      set_umask(0000) do
+        create_file
+      end
+
+      assert_equal DEFAULT_PERMS, ::File.stat(FILENAME).mode
+    end
+
+    def test_umask_066
+      umask = 0066
+      set_umask(umask) do
+        create_file
+      end
+
+      assert_equal((DEFAULT_PERMS - umask), ::File.stat(FILENAME).mode)
+    end
+
+  end
+
+  def create_file
+    ::Zip::File.open(FILENAME, ::Zip::File::CREATE) do |zip|
+      zip.comment = "test"
+    end
+  end
+
+  # If anything goes wrong, make sure the umask is restored.
+  def set_umask(umask, &block)
+    begin
+      saved_umask = ::File.umask(umask)
+      yield
+    ensure
+      ::File.umask(saved_umask)
+    end
+  end
+
+end