HADOOP-18145. Fileutil's unzip method causes unzipped files to lose their original permissions (#4036)

Contributed by jingxiong zhong

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java

@@ -36,15 +36,18 @@
 import java.nio.charset.CharsetEncoder;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.AccessDeniedException;
+import java.nio.file.attribute.PosixFilePermission;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Enumeration;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -53,13 +56,13 @@
 import java.util.jar.JarOutputStream;
 import java.util.jar.Manifest;
 import java.util.zip.GZIPInputStream;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipFile;
-import java.util.zip.ZipInputStream;
 
 import org.apache.commons.collections.map.CaseInsensitiveMap;
 import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
 import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
+import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream;
+import org.apache.commons.compress.archivers.zip.ZipFile;
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
@@ -644,12 +647,12 @@ public static long getDU(File dir) {
    */
   public static void unZip(InputStream inputStream, File toDir)
       throws IOException {
-    try (ZipInputStream zip = new ZipInputStream(inputStream)) {
+    try (ZipArchiveInputStream zip = new ZipArchiveInputStream(inputStream)) {
       int numOfFailedLastModifiedSet = 0;
       String targetDirPath = toDir.getCanonicalPath() + File.separator;
-      for(ZipEntry entry = zip.getNextEntry();
+      for(ZipArchiveEntry entry = zip.getNextZipEntry();
           entry != null;
-          entry = zip.getNextEntry()) {
+          entry = zip.getNextZipEntry()) {
         if (!entry.isDirectory()) {
           File file = new File(toDir, entry.getName());
           if (!file.getCanonicalPath().startsWith(targetDirPath)) {
@@ -668,6 +671,9 @@ public static void unZip(InputStream inputStream, File toDir)
           if (!file.setLastModified(entry.getTime())) {
             numOfFailedLastModifiedSet++;
           }
+          if (entry.getPlatform() == ZipArchiveEntry.PLATFORM_UNIX) {
+            Files.setPosixFilePermissions(file.toPath(), permissionsFromMode(entry.getUnixMode()));
+          }
         }
       }
       if (numOfFailedLastModifiedSet > 0) {
@@ -677,6 +683,49 @@ public static void unZip(InputStream inputStream, File toDir)
     }
   }
 
+  /**
+   * The permission operation of this method only involves users, user groups, and others.
+   * If SUID is set, only executable permissions are reserved.
+   * @param mode Permissions are represented by numerical values
+   * @return The original permissions for files are stored in collections
+   */
+  private static Set<PosixFilePermission> permissionsFromMode(int mode) {
+    EnumSet<PosixFilePermission> permissions =
+        EnumSet.noneOf(PosixFilePermission.class);
+    addPermissions(permissions, mode, PosixFilePermission.OTHERS_READ,
+        PosixFilePermission.OTHERS_WRITE, PosixFilePermission.OTHERS_EXECUTE);
+    addPermissions(permissions, mode >> 3, PosixFilePermission.GROUP_READ,
+        PosixFilePermission.GROUP_WRITE, PosixFilePermission.GROUP_EXECUTE);
+    addPermissions(permissions, mode >> 6, PosixFilePermission.OWNER_READ,
+        PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE);
+    return permissions;
+  }
+
+  /**
+   * Assign the original permissions to the file
+   * @param permissions The original permissions for files are stored in collections
+   * @param mode Use a value of type int to indicate permissions
+   * @param r Read permission
+   * @param w Write permission
+   * @param x Execute permission
+   */
+  private static void addPermissions(
+      Set<PosixFilePermission> permissions,
+      int mode,
+      PosixFilePermission r,
+      PosixFilePermission w,
+      PosixFilePermission x) {
+    if ((mode & 1L) == 1L) {
+      permissions.add(x);
+    }
+    if ((mode & 2L) == 2L) {
+      permissions.add(w);
+    }
+    if ((mode & 4L) == 4L) {
+      permissions.add(r);
+    }
+  }
+
   /**
    * Given a File input it will unzip it in the unzip directory.
    * passed as the second parameter
@@ -685,14 +734,14 @@ public static void unZip(InputStream inputStream, File toDir)
    * @throws IOException An I/O exception has occurred
    */
   public static void unZip(File inFile, File unzipDir) throws IOException {
-    Enumeration<? extends ZipEntry> entries;
+    Enumeration<? extends ZipArchiveEntry> entries;
     ZipFile zipFile = new ZipFile(inFile);
 
     try {
-      entries = zipFile.entries();
+      entries = zipFile.getEntries();
       String targetDirPath = unzipDir.getCanonicalPath() + File.separator;
       while (entries.hasMoreElements()) {
-        ZipEntry entry = entries.nextElement();
+        ZipArchiveEntry entry = entries.nextElement();
         if (!entry.isDirectory()) {
           InputStream in = zipFile.getInputStream(entry);
           try {
@@ -717,6 +766,9 @@ public static void unZip(File inFile, File unzipDir) throws IOException {
             } finally {
               out.close();
             }
+            if (entry.getPlatform() == ZipArchiveEntry.PLATFORM_UNIX) {
+              Files.setPosixFilePermissions(file.toPath(), permissionsFromMode(entry.getUnixMode()));
+            }
           } finally {
             in.close();
           }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java

@@ -53,11 +53,11 @@
 import java.util.jar.Attributes;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipOutputStream;
 
 import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
 import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream;
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
+import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.test.GenericTestUtils;
@@ -773,26 +773,71 @@ public void testCreateLocalTempFile() throws IOException {
   public void testUnZip() throws Exception {
     // make sa simple zip
     final File simpleZip = new File(del, FILE);
-    OutputStream os = new FileOutputStream(simpleZip); 
+    try (OutputStream os = new FileOutputStream(simpleZip);
-    ZipOutputStream tos = new ZipOutputStream(os);
+         ZipArchiveOutputStream tos = new ZipArchiveOutputStream(os)) {
-    try {
+      List<ZipArchiveEntry> ZipArchiveList = new ArrayList<>(7);
-      ZipEntry ze = new ZipEntry("foo");
+      int count = 0;
+      // create 7 files to verify permissions
+      for (int i = 0; i < 7; i++) {
+        ZipArchiveList.add(new ZipArchiveEntry("foo_" + i));
+        ZipArchiveEntry archiveEntry = ZipArchiveList.get(i);
+        archiveEntry.setUnixMode(count += 0100);
         byte[] data = "some-content".getBytes("UTF-8");
-      ze.setSize(data.length);
+        archiveEntry.setSize(data.length);
-      tos.putNextEntry(ze);
+        tos.putArchiveEntry(archiveEntry);
         tos.write(data);
-      tos.closeEntry();
+      }
+      tos.closeArchiveEntry();
       tos.flush();
       tos.finish();
-    } finally {
-      tos.close();
     }
 
     // successfully unzip it into an existing dir:
     FileUtil.unZip(simpleZip, tmp);
+    File foo0 = new File(tmp, "foo_0");
+    File foo1 = new File(tmp, "foo_1");
+    File foo2 = new File(tmp, "foo_2");
+    File foo3 = new File(tmp, "foo_3");
+    File foo4 = new File(tmp, "foo_4");
+    File foo5 = new File(tmp, "foo_5");
+    File foo6 = new File(tmp, "foo_6");
     // check result:
-    Verify.exists(new File(tmp, "foo"));
+    assertTrue(foo0.exists());
-    assertEquals(12, new File(tmp, "foo").length());
+    assertTrue(foo1.exists());
+    assertTrue(foo2.exists());
+    assertTrue(foo3.exists());
+    assertTrue(foo4.exists());
+    assertTrue(foo5.exists());
+    assertTrue(foo6.exists());
+    assertEquals(12, foo0.length());
+    // tests whether file foo_0 has executable permissions
+    assertTrue("file lacks execute permissions", foo0.canExecute());
+    assertFalse("file has write permissions", foo0.canWrite());
+    assertFalse("file has read permissions", foo0.canRead());
+    // tests whether file foo_1 has writable permissions
+    assertFalse("file has execute permissions", foo1.canExecute());
+    assertTrue("file lacks write permissions", foo1.canWrite());
+    assertFalse("file has read permissions", foo1.canRead());
+    // tests whether file foo_2 has executable and writable permissions
+    assertTrue("file lacks execute permissions", foo2.canExecute());
+    assertTrue("file lacks write permissions", foo2.canWrite());
+    assertFalse("file has read permissions", foo2.canRead());
+    // tests whether file foo_3 has readable permissions
+    assertFalse("file has execute permissions", foo3.canExecute());
+    assertFalse("file has write permissions", foo3.canWrite());
+    assertTrue("file lacks read permissions", foo3.canRead());
+    // tests whether file foo_4 has readable and executable permissions
+    assertTrue("file lacks execute permissions", foo4.canExecute());
+    assertFalse("file has write permissions", foo4.canWrite());
+    assertTrue("file lacks read permissions", foo4.canRead());
+    // tests whether file foo_5 has readable and writable permissions
+    assertFalse("file has execute permissions", foo5.canExecute());
+    assertTrue("file lacks write permissions", foo5.canWrite());
+    assertTrue("file lacks read permissions", foo5.canRead());
+    // tests whether file foo_6 has readable, writable and executable permissions
+    assertTrue("file lacks execute permissions", foo6.canExecute());
+    assertTrue("file lacks write permissions", foo6.canWrite());
+    assertTrue("file lacks read permissions", foo6.canRead());
 
     final File regularFile =
         Verify.createNewFile(new File(tmp, "QuickBrownFoxJumpsOverTheLazyDog"));
@@ -804,14 +849,14 @@ public void testUnZip2() throws IOException {
     // make a simple zip
     final File simpleZip = new File(del, FILE);
     OutputStream os = new FileOutputStream(simpleZip);
-    try (ZipOutputStream tos = new ZipOutputStream(os)) {
+    try (ZipArchiveOutputStream tos = new ZipArchiveOutputStream(os)) {
       // Add an entry that contains invalid filename
-      ZipEntry ze = new ZipEntry("../foo");
+      ZipArchiveEntry ze = new ZipArchiveEntry("../foo");
       byte[] data = "some-content".getBytes(StandardCharsets.UTF_8);
       ze.setSize(data.length);
-      tos.putNextEntry(ze);
+      tos.putArchiveEntry(ze);
       tos.write(data);
-      tos.closeEntry();
+      tos.closeArchiveEntry();
       tos.flush();
       tos.finish();
     }