Fix #3787, #936: javalib ZipEntry now gets & sets MS-DOS times correc…

…tly (#3794)

* Fix #3787: javalib ZipEntry now gets & sets ms-dos times correctly
* Debug failing Unix JVM compliance test failures
* Correct handling of Daylight Saving Time
* Still trying to understand JVM compliance failures
* Fix a number of correctness, thread-safety, & performance issues
* Give null proper respect

javalib/src/main/scala/java/util/zip/ZipEntry.scala

@@ -1,13 +1,19 @@
 package java.util.zip
 
-// Ported from Apache Harmony
+// Ported from Apache Harmony. Extensive changes for Scala Native.
 
 import java.io.{
   EOFException,
   InputStream,
   RandomAccessFile,
   UnsupportedEncodingException
 }
+
+import scala.scalanative.posix.time._
+import scala.scalanative.posix.timeOps.tmOps
+
+import scala.scalanative.unsafe._
+
 class ZipEntry private (
     private[zip] var name: String,
     private[zip] var comment: String,
@@ -24,7 +30,7 @@ class ZipEntry private (
     with Cloneable {
 
   def this(name: String) =
-    this(name, null, -1, -1, -1, -1, -1, -1, null, -1, -1)
+    this(name, null, -1L, -1L, -1L, -1, -1, -1, null, -1, -1L)
 
   def this(e: ZipEntry) =
     this(
@@ -44,6 +50,7 @@ class ZipEntry private (
   if (name == null) {
     throw new NullPointerException()
   }
+
   if (name.length() > 0xffff) {
     throw new IllegalArgumentException()
   }
@@ -69,22 +76,28 @@ class ZipEntry private (
   def getSize(): Long =
     size
 
-  def getTime(): Long =
-    -1
-  // TODO: Uncomment once we have Calendar
-  // if (time != -1) {
-  //   val cal = new GregorianCalendar()
-  //   cal.set(Calendar.MILLISECOND, 0)
-  //   cal.set(1980 + ((modDate >> 9) & 0x7f),
-  //           ((modDate >> 5) & 0xf) - 1,
-  //           modDate & 0x1f,
-  //           (time >> 11) & 0x1f,
-  //           (time >> 5) & 0x3f,
-  //           (time & 0x1f) << 1)
-  //   cal.getTime().getTime()
-  // } else {
-  //   -1
-  // }
+  def getTime(): Long = {
+    if ((time == -1) || (modDate == -1)) -1L
+    else
+      synchronized {
+        val tm = stackalloc[tm]()
+
+        tm.tm_year = ((modDate >> 9) & 0x7f) + 80
+        tm.tm_mon = ((modDate >> 5) & 0xf) - 1
+        tm.tm_mday = modDate & 0x1f
+
+        tm.tm_hour = (time >> 11) & 0x1f
+        tm.tm_min = (time >> 5) & 0x3f
+        tm.tm_sec = (time & 0x1f) << 1
+
+        tm.tm_isdst = -1
+
+        val unixEpochSeconds = mktime(tm)
+
+        if (unixEpochSeconds < 0) -1L // Per JVM doc, -1 means "Unspecified"
+        else unixEpochSeconds * 1000L
+      }
+  }
 
   def isDirectory(): Boolean =
     name.charAt(name.length - 1) == '/'
@@ -130,21 +143,50 @@ class ZipEntry private (
     }
 
   def setTime(value: Long): Unit = {
-    // TODO: Uncomment once we have Date
-    // val cal = new GregorianCalendar()
-    // cal.setTime(new Date(value))
-    // val year = cal.get(Calendar.YEAR)
-    // if (year < 1980) {
-    //   modDate = 0x21
-    //   time = 0
-    // } else {
-    //   modDate = cal.get(Calendar.DATE)
-    //   modDate = (cal.get(Calendar.MONTH) + 1 << 5) | modDate
-    //   modDate = ((cal.get(Calendar.YEAR) - 1980) << 9) | modDate
-    //   time = cal.get(Calendar.SECOND) >> 1
-    //   time = (cal.get(Calendar.MINUTE) << 5) | time
-    //   time = (cal.get(Calendar.HOUR_OF_DAY) << 11) | time
-    // }
+    /* Convert Java time in milliseconds since the Unix epoch to
+     * MS-DOS standard time.
+     *
+     * This URL gives a good description of standard MS-DOS time & the
+     * required bit manipulations:
+     *     https://learn.microsoft.com/en-us/windows/win32/api/oleauto/
+     *         nf-oleauto-dosdatetimetovarianttime
+     *
+     * Someone familiar with Windows could probably provide an operating
+     * system specific version of this method.
+     */
+
+    /* Concurrency issue:
+     *   localtime() is not required to be thread-safe, but is likely to exist
+     *   on Windows. Change to known thread-safe localtime_r() when this
+     *   section is unix-only.
+     */
+
+    val timer = stackalloc[time_t]()
+
+    // truncation OK, MS-DOS uses 2 second intervals, no rounding.
+    !timer = (value / 1000L).toSize
+
+    val tm = localtime(timer) // Not necessarily thread safe.
+
+    if (tm == null) {
+      modDate = 0x21
+      time = 0
+    } else {
+      val msDosYears = tm.tm_year - 80
+
+      if (msDosYears <= 0) {
+        modDate = 0x21 // 01-01-1980 00:00 MS-DOS epoch
+        time = 0
+      } else {
+        modDate = tm.tm_mday
+        modDate = ((tm.tm_mon + 1) << 5) | modDate
+        modDate = (msDosYears << 9) | modDate
+
+        time = tm.tm_sec >> 1
+        time = (tm.tm_min << 5) | time
+        time = (tm.tm_hour << 11) | time
+      }
+    }
   }
 
   override def toString(): String =

javalib/src/main/scala/java/util/zip/ZipFile.scala

@@ -121,10 +121,14 @@ class ZipFile(file: File, mode: Int, charset: Charset) extends Closeable {
     if (entryName == null)
       throw new NullPointerException()
 
-    mEntries.getOrDefault(
-      entryName,
-      mEntries.getOrDefault(entryName + "/", null)
-    )
+    val me = mEntries
+      .getOrDefault(
+        entryName,
+        mEntries.getOrDefault(entryName + "/", null)
+      )
+
+    if (me == null) null
+    else me.clone().asInstanceOf[ZipEntry] // keep original entry immutable
   }
 
   def getInputStream(_entry: ZipEntry): InputStream = {

unit-tests/shared/src/test/scala/org/scalanative/testsuite/javalib/util/zip/ZipBytes.scala

@@ -9,7 +9,8 @@ import java.nio.file.Files
 object ZipBytes {
 
   def getFile(bs: Array[Byte]): File = {
-    val path = Files.createTempFile("zipFile", ".zip")
+    val path =
+      Files.createTempFile("scala-native-testsuite_javalib_zipFile", ".zip")
     Files.write(path, bs)
     path.toFile
   }

unit-tests/shared/src/test/scala/org/scalanative/testsuite/javalib/util/zip/ZipEntryIssuesTest.scala

@@ -13,15 +13,18 @@ import java.util.Arrays
 
 import java.util.zip.{ZipEntry, ZipFile, ZipOutputStream}
 
-/* Do not disturb the peace of the Harmony code ported to Scala Native.
- * Consolidate Test(s) written well after that port in this separate file.
+/* Do not disturb the peace of Tests written when the  Harmony code
+ * was ported to Scala Native.
+ *
+ * Consolidate Test(s) written well after that time in this separate file.
  */
 
 object ZipEntryIssuesTest {
 
   private var workDirString: String = _
 
   private val zipTestDataFileName = "zipEntryReadCommentTestData.zip"
+  private val zipTestSetDosTimeFileName = "zipEntrySetDosTimeTestData.zip"
 
   private def makeTestDirs(): String = {
     val orgDir = Files.createTempDirectory("scala-native-testsuite")
@@ -33,13 +36,54 @@ object ZipEntryIssuesTest {
       .resolve("ZipEntriesIssuesTest")
 
     val testDirSrcPath = testDirRootPath.resolve("src")
+    val testDirDstPath = testDirRootPath.resolve("dst")
 
     Files.createDirectories(testDirRootPath)
     Files.createDirectory(testDirSrcPath)
+    Files.createDirectory(testDirDstPath)
 
     testDirRootPath.toString()
   }
 
+  private def createZipFile(
+      location: String,
+      entryNames: Array[String]
+  ): Unit = {
+    val zipOut = new ZipOutputStream(
+      new BufferedOutputStream(new FileOutputStream(location))
+    )
+    try {
+      zipOut.setComment("Some interesting moons of Saturn.")
+
+      Arrays
+        .stream(entryNames)
+        .forEach(e => zipOut.putNextEntry(new ZipEntry(e)))
+
+    } finally {
+      zipOut.close()
+    }
+  }
+
+  private def provisionZipEntrySetDosTimeTestData(zosTestDir: String): Unit = {
+    // In JVM, cwd is set to unit-tests/jvm/[scala-version]
+    val inputRootDir =
+      if (Platform.executingInJVM) "../.."
+      else "unit-tests"
+
+    val outputFileQualifiedName =
+      s"${zosTestDir}/src/${zipTestSetDosTimeFileName}"
+
+    val entryNames = Array(
+      "Rhea_1",
+      "Prometheus_2",
+      "Phoebe_3",
+      "Tethys_4",
+      "Iapetus_5"
+    )
+
+    createZipFile(outputFileQualifiedName, entryNames)
+  }
+
   private def provisionZipEntryIssuesTestData(zeTestDir: String): Unit = {
     // In JVM, cwd is set to unit-tests/jvm/[scala-version]
     val inputRootDir =
@@ -62,6 +106,7 @@ object ZipEntryIssuesTest {
   def beforeClass(): Unit = {
     workDirString = makeTestDirs()
     provisionZipEntryIssuesTestData(workDirString)
+    provisionZipEntrySetDosTimeTestData(workDirString)
   }
 }
 
@@ -91,4 +136,82 @@ class ZipEntryIssuesTest {
       zf.close()
     }
   }
+
+  // Issue 3787
+  @Test def setEntryDosTime(): Unit = {
+    val srcName =
+      s"${workDirString}/src/${zipTestSetDosTimeFileName}"
+
+    val dstName =
+      s"${workDirString}/dst/CopyOf_${zipTestSetDosTimeFileName}"
+
+    /*  expectedMillis generated using JVM:
+     *  val y2k = Instant.parse("2000-01-01T00:00:00.00Z").toEpochMilli
+     *  val y2k: Long = 946684800000
+     */
+
+    val changeEntry = "Tethys_4"
+
+    val expectedMillis = 946684800000L
+
+    val zf = new ZipFile(srcName)
+    try {
+      val zipOut = new ZipOutputStream(
+        new BufferedOutputStream(new FileOutputStream(dstName))
+      )
+
+      try {
+        zf.stream()
+          .limit(99)
+          .forEach(e => {
+            zipOut.putNextEntry(e)
+
+            if (!e.isDirectory()) {
+              val fis = zf.getInputStream(e)
+              val buf = new Array[Byte](2 * 1024)
+
+              try {
+                var nRead = 0
+                // Poor but useful idioms creep in: porting from Java style
+                while ({ nRead = fis.read(buf); nRead } > 0) {
+                  zipOut.write(buf, 0, nRead)
+                  assertEquals("fis nRead", e.getSize(), nRead)
+                }
+              } finally {
+                fis.close()
+              }
+            }
+            // make a change to modification time which should be noticable.
+            if (e.getName() == changeEntry) {
+              e.setTime(expectedMillis)
+              e.setComment(
+                "ms-dos modtime should be Year 2000 UTC, " +
+                  s"local to where file was written."
+              )
+            }
+            zipOut.closeEntry()
+          })
+
+      } finally {
+        zipOut.close()
+      }
+
+    } finally {
+      zf.close()
+    }
+
+    /* Re-read to see if getTime() returns the expected value.
+     * If not, manual visual inspection of the output file will distinguish
+     * if the change was durable or if getTime() mangled reading it.
+     */
+
+    val zfDst = new ZipFile(dstName)
+    try {
+      val ze = zfDst.getEntry(changeEntry)
+      assertNotNull("zipEntry '${changeEntry}' not found", ze)
+      assertEquals("getTime()", expectedMillis, ze.getTime())
+    } finally {
+      zfDst.close()
+    }
+  }
 }