From 8c694d38ee683b1c2fda3430a928e2308942e434 Mon Sep 17 00:00:00 2001
From: Robert Haines <rhaines@manchester.ac.uk>
Date: Sun, 6 Oct 2019 19:37:03 +0100
Subject: [PATCH] Add functionality to restore file timestamps.

There has been an option in `Zip::File` (`:restore_times`) for a long
time, but it seems it has never worked. Firstly the actual timestamp of
an added file wasn't being saved, and secondly an extracted file wasn't
having its timestamp set correctly.

This commit fixes both of those issues, and adds tests to make sure.
---
 lib/zip/entry.rb          | 18 ++++++++----
 test/file_options_test.rb | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 6 deletions(-)
 create mode 100644 test/file_options_test.rb

diff --git a/lib/zip/entry.rb b/lib/zip/entry.rb
index 142308a..88b5049 100644
--- a/lib/zip/entry.rb
+++ b/lib/zip/entry.rb
@@ -406,16 +406,22 @@ module Zip
       @unix_uid   = stat.uid
       @unix_gid   = stat.gid
       @unix_perms = stat.mode & 0o7777
+
+      mtime = stat.mtime
+      @time = ::Zip::DOSTime.local(mtime.year, mtime.month, mtime.day, mtime.hour, mtime.min, mtime.sec)
     end
 
-    def set_unix_permissions_on_path(dest_path)
-      # BUG: does not update timestamps into account
+    def set_unix_attributes_on_path(dest_path)
       # ignore setuid/setgid bits by default.  honor if @restore_ownership
       unix_perms_mask = 0o1777
       unix_perms_mask = 0o7777 if @restore_ownership
       ::FileUtils.chmod(@unix_perms & unix_perms_mask, dest_path) if @restore_permissions && @unix_perms
       ::FileUtils.chown(@unix_uid, @unix_gid, dest_path) if @restore_ownership && @unix_uid && @unix_gid && ::Process.egid == 0
-      # File::utimes()
+
+      # Restore the timestamp on a file. This will either have come from the
+      # original source file that was copied into the archive, or from the
+      # creation date of the archive if there was no original source file.
+      ::FileUtils.touch(dest_path, mtime: time) if @restore_times
     end
 
     def set_extra_attributes_on_path(dest_path) # :nodoc:
@@ -423,7 +429,7 @@ module Zip
 
       case @fstype
       when ::Zip::FSTYPE_UNIX
-        set_unix_permissions_on_path(dest_path)
+        set_unix_attributes_on_path(dest_path)
       end
     end
 
@@ -601,8 +607,6 @@ module Zip
       end
       ::File.open(dest_path, 'wb') do |os|
         get_input_stream do |is|
-          set_extra_attributes_on_path(dest_path)
-
           bytes_written = 0
           warned = false
           buf = ''.dup
@@ -621,6 +625,8 @@ module Zip
           end
         end
       end
+
+      set_extra_attributes_on_path(dest_path)
     end
 
     def create_directory(dest_path)
diff --git a/test/file_options_test.rb b/test/file_options_test.rb
new file mode 100644
index 0000000..4f8e571
--- /dev/null
+++ b/test/file_options_test.rb
@@ -0,0 +1,58 @@
+require 'test_helper'
+
+class FileOptionsTest < MiniTest::Test
+  ZIPPATH = ::File.join(Dir.tmpdir, 'options.zip').freeze
+  TXTPATH = ::File.expand_path(::File.join('data', 'file1.txt'), __dir__).freeze
+  EXTPATH_1 = ::File.join(Dir.tmpdir, 'extracted_1.txt').freeze
+  EXTPATH_2 = ::File.join(Dir.tmpdir, 'extracted_2.txt').freeze
+  ENTRY_1 = 'entry_1.txt'.freeze
+  ENTRY_2 = 'entry_2.txt'.freeze
+
+  def teardown
+    ::File.unlink(ZIPPATH) if ::File.exist?(ZIPPATH)
+    ::File.unlink(EXTPATH_1) if ::File.exist?(EXTPATH_1)
+    ::File.unlink(EXTPATH_2) if ::File.exist?(EXTPATH_2)
+  end
+
+  def test_restore_times_true
+    ::Zip::File.open(ZIPPATH, true) do |zip|
+      zip.add(ENTRY_1, TXTPATH)
+      zip.add_stored(ENTRY_2, TXTPATH)
+    end
+
+    ::Zip::File.open(ZIPPATH, false, restore_times: true) do |zip|
+      zip.extract(ENTRY_1, EXTPATH_1)
+      zip.extract(ENTRY_2, EXTPATH_2)
+    end
+
+    assert_time_equal(::File.mtime(TXTPATH), ::File.mtime(EXTPATH_1))
+    assert_time_equal(::File.mtime(TXTPATH), ::File.mtime(EXTPATH_2))
+  end
+
+  def test_restore_times_false
+    ::Zip::File.open(ZIPPATH, true) do |zip|
+      zip.add(ENTRY_1, TXTPATH)
+      zip.add_stored(ENTRY_2, TXTPATH)
+    end
+
+    ::Zip::File.open(ZIPPATH, false, restore_times: false) do |zip|
+      zip.extract(ENTRY_1, EXTPATH_1)
+      zip.extract(ENTRY_2, EXTPATH_2)
+    end
+
+    assert_time_equal(::Time.now, ::File.mtime(EXTPATH_1))
+    assert_time_equal(::Time.now, ::File.mtime(EXTPATH_2))
+  end
+
+  private
+
+  # Method to compare file times. DOS times only have 2 second accuracy.
+  def assert_time_equal(expected, actual)
+    assert_equal(expected.year, actual.year)
+    assert_equal(expected.month, actual.month)
+    assert_equal(expected.day, actual.day)
+    assert_equal(expected.hour, actual.hour)
+    assert_equal(expected.min, actual.min)
+    assert_in_delta(expected.sec, actual.sec, 1)
+  end
+end