Preserve modification timestamp when zipping files.

third_party/zlib/google/zip_unittest.cc

Index: third_party/zlib/google/zip_unittest.cc
diff --git a/third_party/zlib/google/zip_unittest.cc b/third_party/zlib/google/zip_unittest.cc
index 1023705f1b8b33e466b5625759be42eabbf36935..2f41914f480ff1cad9e18c18cfc9712b04be2d74 100644
--- a/third_party/zlib/google/zip_unittest.cc
+++ b/third_party/zlib/google/zip_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include <set>
+#include <string>
 #include <vector>
 
 #include "base/file_util.h"
@@ -98,6 +99,51 @@ class ZipTest : public PlatformTest {
     EXPECT_EQ(expected_count, count);
   }
 
+  // This function does the following:
+  // 1) Creates a test.txt file with the given last modification timestamp
+  //    set to the valud or |date_time|.

[satorux1, 2013/12/13 04:52:50]

valud? this line seems to be unnecessary.

[satorux1, 2013/12/17 02:32:25]

Please mark comments you addressed with 'Done'. That makes it easier to track
what's done and what's not.

[João Eiras, 2013/12/17 10:15:38]

Done.

[João Eiras, 2013/12/17 10:15:38]

Done.

+  // 2) Zips test.txt and extracts it back into a different location.
+  // 3) Confirms that test.txt in the output directory has the specified
+  //    last modification timestamp if it is valid (|valid_year| is true).
+  //    If the timestamp is not supported by the zip format, the last
+  //    modification defaults to the current time.

[satorux1, 2013/12/13 04:52:50]

I think we should have documentation like this in zip_reader.h, describing the
policy on timestamp preservation. Could you add it?

[João Eiras, 2013/12/17 10:15:38]

Done.

+  void TestTimeStamp(const char* date_time, bool valid_year) {
+    SCOPED_TRACE(std::string("TestTimeStamp(") + date_time + ")");
+    base::ScopedTempDir temp_dir;
+    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+    base::FilePath zip_file = temp_dir.path().AppendASCII("out.zip");
+    base::FilePath src_dir = temp_dir.path().AppendASCII("input");
+    base::FilePath out_dir = temp_dir.path().AppendASCII("output");
+
+    base::FilePath src_file = src_dir.AppendASCII("test.txt");
+    base::FilePath out_file = out_dir.AppendASCII("test.txt");
+
+    EXPECT_TRUE(base::CreateDirectory(src_dir));
+    EXPECT_TRUE(base::CreateDirectory(out_dir));
+
+    base::Time test_mtime;
+    ASSERT_TRUE(base::Time::FromString(date_time, &test_mtime));
+    base::Time now_time = base::Time::Now() - base::TimeDelta::FromSeconds(1);

[satorux1, 2013/12/13 04:52:50]

|now_time| actually points to a time in the past (because of -1 second), which
is somewhat awkward.  I guess -1 was added because clock can slightly go
backward. However, here, we are doing a lot of file IO, so I guess we don't have
to worry about it. maybe:

base::Time start_time = base::Time::Now();

[João Eiras, 2013/12/13 17:01:57]

The seconds in the file's timestamp  are floored when zipping, so -1 is a quick
way to work around that, because Now(), although it should actually be -2
because "3.900" seconss will be stored as "2" in the zip file. I've made it more
explicit.

[satorux1, 2013/12/17 02:32:25]

Didn't know that the timestamp resolution in zip files is two seconds. Please
document it explicitly here.

[João Eiras, 2013/12/17 10:15:38]

Done.

[João Eiras, 2013/12/17 10:15:38]

Done.

[João Eiras, 2013/12/17 10:15:38]

Done.

+
+    EXPECT_EQ(1, file_util::WriteFile(src_file, "1", 1));
+    EXPECT_TRUE(base::TouchFile(src_file, base::Time::Now(), test_mtime));
+
+    EXPECT_TRUE(zip::Zip(src_dir, zip_file, true));
+    ASSERT_TRUE(zip::Unzip(zip_file, out_dir));
+
+    base::PlatformFileInfo file_info;
+    EXPECT_TRUE(base::GetFileInfo(out_file, &file_info));
+    EXPECT_EQ(file_info.size, 1);
+
+    if (valid_year) {
+      EXPECT_EQ(file_info.last_modified, test_mtime);
+    } else {
+      // Invalid date means the modification time will default to 'now'.
+      EXPECT_GE(file_info.last_modified, now_time);
+    }
+  }
+
   // The path to temporary directory used to contain the test operations.
   base::FilePath test_dir_;
 
@@ -192,6 +238,28 @@ TEST_F(ZipTest, ZipNonASCIIDir) {
   TestUnzipFile(zip_file, true);
 }
 
+TEST_F(ZipTest, ZipTimeStamp) {
+  // The dates tested are arbitrary, with some constraints. The zip format can
+  // only store years from 1980 to 2107 and an even number of seconds, due to it
+  // using the ms dos date format.
+
+  // Valid arbitrary date.
+  TestTimeStamp("23 Oct 1997 23:22:20", true);

[satorux1, 2013/12/13 04:52:50]

true/false looks cryptic. It'd be nice to define an enum like:

enum {
  VALID_YEAR,
  INVALID_YEAR
}

Then it becomes clearer:

   TestTimeStamp("23 Oct 1997 23:22:20", VALID_YEAR);

[João Eiras, 2013/12/17 10:15:38]

Done.

+
+  // Date before 1980, zip format limitation, must default to unix epoch.
+  TestTimeStamp("29 Dec 1979 21:00:10", false);
+
+  // Despite the minizip headers telling the maximum year should be 2044, it
+  // can actually go up to 2107. Beyond that, the dos date format cannot store
+  // the year (2107-1980=127). To test that limit, the input file needs to be
+  // touched, but the code that modifies the file access and modification times
+  // relies on time_t which is defined as long, therefore being in many
+  // platforms just a 4-byte integer, like 32-bit Mac OSX or linux. As such, it
+  // suffers from the year-2038 bug. Therefore 2038 is the highest we can test
+  // in all platforms reliably.
+  TestTimeStamp("02 Jan 2038 23:59:58", true);
+}
+
 #if defined(OS_POSIX)
 TEST_F(ZipTest, ZipFiles) {
   base::FilePath src_dir;