Skip to content

Commit

Permalink
HADOOP-18145. Fileutil's unzip method causes unzipped files to lose t…
Browse files Browse the repository at this point in the history 
…heir original permissions (#4036)

Contributed by jingxiong zhong

Change-Id: Ie252e609798719dc658364f9bae48b34dc72a79c
  • Loading branch information
@smallzhongfeng @steveloughran
smallzhongfeng authored and steveloughran committed Mar 30, 2022
1 parent 4f85c9a commit 1ee93f7
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 29 deletions.
71 changes: 62 additions & 9 deletions hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,17 @@
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.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;
Expand All @@ -52,13 +55,14 @@
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;
import org.apache.hadoop.conf.Configuration;
Expand Down Expand Up @@ -622,12 +626,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)) {
Expand All @@ -646,6 +650,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) {
Expand All @@ -655,6 +662,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
Expand All @@ -663,14 +713,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 {
Expand All @@ -695,6 +745,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();
}
Expand Down
85 changes: 65 additions & 20 deletions hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -744,26 +744,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);
ZipOutputStream tos = new ZipOutputStream(os);
try {
ZipEntry ze = new ZipEntry("foo");
byte[] data = "some-content".getBytes("UTF-8");
ze.setSize(data.length);
tos.putNextEntry(ze);
tos.write(data);
tos.closeEntry();
try (OutputStream os = new FileOutputStream(simpleZip);
ZipArchiveOutputStream tos = new ZipArchiveOutputStream(os)) {
List<ZipArchiveEntry> ZipArchiveList = new ArrayList<>(7);
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");
archiveEntry.setSize(data.length);
tos.putArchiveEntry(archiveEntry);
tos.write(data);
}
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"));
assertEquals(12, new File(tmp, "foo").length());
assertTrue(foo0.exists());
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"));
Expand All @@ -775,14 +820,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();
}
Expand Down

0 comments on commit 1ee93f7

Please sign in to comment.