Skip to content

Commit

Permalink
[FLINK-23166][python] Fix ZipUtils to handle properly for softlinks
Browse files Browse the repository at this point in the history 
This closes apache#16309.
  • Loading branch information
@dianfu
dianfu authored Jun 28, 2021
1 parent 3725497 commit c44dd62
Show file tree
Hide file tree
Showing 2 changed files with 199 additions and 14 deletions.
52 changes: 38 additions & 14 deletions flink-python/src/main/java/org/apache/flink/python/util/ZipUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipFile;

import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.PosixFilePermission;
Expand All @@ -39,46 +41,60 @@

/** Utils used to extract zip files and try to restore the origin permissions of files. */
@Internal
public class ZipUtils {
public final class ZipUtils {

public static void extractZipFileWithPermissions(String zipFilePath, String targetPath)
throws IOException {
try (ZipFile zipFile = new ZipFile(zipFilePath)) {
Enumeration<ZipArchiveEntry> entries = zipFile.getEntries();
boolean isUnix = isUnix();
ByteArrayOutputStream baos = new ByteArrayOutputStream();
String canonicalTargetPath = new File(targetPath).getCanonicalPath() + File.separator;

while (entries.hasMoreElements()) {
ZipArchiveEntry entry = entries.nextElement();
File file;
File outputFile = new File(canonicalTargetPath, entry.getName());
if (!outputFile.getCanonicalPath().startsWith(canonicalTargetPath)) {
throw new IOException(
"Expand "
+ entry.getName()
+ " would create a file outside of "
+ targetPath);
}

if (entry.isDirectory()) {
file = new File(targetPath, entry.getName());
if (!file.exists()) {
if (!file.mkdirs()) {
if (!outputFile.exists()) {
if (!outputFile.mkdirs()) {
throw new IOException(
"Create dir: " + file.getAbsolutePath() + " failed!");
"Create dir: " + outputFile.getAbsolutePath() + " failed!");
}
}
} else {
file = new File(targetPath, entry.getName());
File parentDir = file.getParentFile();
File parentDir = outputFile.getParentFile();
if (!parentDir.exists()) {
if (!parentDir.mkdirs()) {
throw new IOException(
"Create dir: " + file.getAbsolutePath() + " failed!");
"Create dir: " + outputFile.getAbsolutePath() + " failed!");
}
}
if (file.createNewFile()) {
OutputStream output = new FileOutputStream(file);
if (entry.isUnixSymlink()) {
// the content of the file is the target path of the symlink
baos.reset();
IOUtils.copyBytes(zipFile.getInputStream(entry), baos);
Files.createSymbolicLink(
outputFile.toPath(), new File(parentDir, baos.toString()).toPath());
} else if (outputFile.createNewFile()) {
OutputStream output = new FileOutputStream(outputFile);
IOUtils.copyBytes(zipFile.getInputStream(entry), output);
} else {
throw new IOException(
"Create file: " + file.getAbsolutePath() + " failed!");
"Create file: " + outputFile.getAbsolutePath() + " failed!");
}
}
if (isUnix) {
int mode = entry.getUnixMode();
if (mode != 0) {
Path path = Paths.get(file.toURI());
Path outputPath = Paths.get(outputFile.toURI());
Set<PosixFilePermission> permissions = new HashSet<>();
addIfBitSet(mode, 8, permissions, PosixFilePermission.OWNER_READ);
addIfBitSet(mode, 7, permissions, PosixFilePermission.OWNER_WRITE);
Expand All @@ -89,7 +105,15 @@ public static void extractZipFileWithPermissions(String zipFilePath, String targ
addIfBitSet(mode, 2, permissions, PosixFilePermission.OTHERS_READ);
addIfBitSet(mode, 1, permissions, PosixFilePermission.OTHERS_WRITE);
addIfBitSet(mode, 0, permissions, PosixFilePermission.OTHERS_EXECUTE);
Files.setPosixFilePermissions(path, permissions);
// the permission of the target file will be set to be the same as the
// symlink
// TODO: support setting the permission without following links
try {
Files.setPosixFilePermissions(outputPath, permissions);
} catch (NoSuchFileException e) {
// this may happens when the target file of the symlink is still not
// extracted
}
}
}
}
Expand Down
161 changes: 161 additions & 0 deletions flink-python/src/test/java/org/apache/flink/python/util/ZipUtilsTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.flink.python.util;

import org.apache.flink.configuration.ConfigConstants;

import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFilePermission;
import java.util.Set;

import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

/** Tests for {@link ZipUtils}. */
public class ZipUtilsTest {

private static final int S_IFLNK = 40960;

@Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder();

@Test
public void testSymlink() throws IOException {
File zipFile = temporaryFolder.newFile();

try (ZipArchiveOutputStream zipOut =
new ZipArchiveOutputStream(new FileOutputStream(zipFile))) {
String file1 = "zipFile1";
ZipArchiveEntry entry = new ZipArchiveEntry(file1);
entry.setUnixMode(0644);
zipOut.putArchiveEntry(entry);
zipOut.write(new byte[] {1, 1, 1, 1, 1});
zipOut.closeArchiveEntry();

String file2 = "zipFile2";
entry = new ZipArchiveEntry(file2);
entry.setUnixMode(0644);
zipOut.putArchiveEntry(entry);
zipOut.write(new byte[] {2, 2, 2, 2, 2});
zipOut.closeArchiveEntry();

entry = new ZipArchiveEntry("softlink");
entry.setUnixMode(S_IFLNK | 0644);
zipOut.putArchiveEntry(entry);
zipOut.write(file1.getBytes(ConfigConstants.DEFAULT_CHARSET));
zipOut.closeArchiveEntry();
}

String targetPath = temporaryFolder.newFolder().getCanonicalPath();
ZipUtils.extractZipFileWithPermissions(zipFile.getCanonicalPath(), targetPath);
Path softLink = new File(targetPath, "softlink").toPath();
assertTrue(Files.isSymbolicLink(softLink));
assertTrue(Files.readSymbolicLink(softLink).toString().endsWith("zipFile1"));

Path file1Path = new File(targetPath, "zipFile1").toPath();
assertTrue(Files.isRegularFile(file1Path));
assertArrayEquals(Files.readAllBytes(file1Path), new byte[] {1, 1, 1, 1, 1});

Path file2Path = new File(targetPath, "zipFile2").toPath();
assertTrue(Files.isRegularFile(file2Path));
assertArrayEquals(Files.readAllBytes(file2Path), new byte[] {2, 2, 2, 2, 2});
}

@Test
public void testSymlinkWithoutTargetFile() throws IOException {
File zipFile = temporaryFolder.newFile();

try (ZipArchiveOutputStream zipOut =
new ZipArchiveOutputStream(new FileOutputStream(zipFile))) {
ZipArchiveEntry entry = new ZipArchiveEntry("softlink");
entry.setUnixMode(S_IFLNK | 0644);
zipOut.putArchiveEntry(entry);
zipOut.write("targetFile".getBytes(ConfigConstants.DEFAULT_CHARSET));
zipOut.closeArchiveEntry();
}

String targetPath = temporaryFolder.newFolder().getCanonicalPath();
ZipUtils.extractZipFileWithPermissions(zipFile.getCanonicalPath(), targetPath);
Path softLink = new File(targetPath, "softlink").toPath();
assertTrue(Files.isSymbolicLink(softLink));
assertTrue(Files.readSymbolicLink(softLink).toString().endsWith("targetFile"));
}

@Test
public void testExpandOutOfTargetDir() throws IOException {
File zipFile = temporaryFolder.newFile();

try (ZipArchiveOutputStream zipOut =
new ZipArchiveOutputStream(new FileOutputStream(zipFile))) {
String file1 = "../zipFile";
ZipArchiveEntry entry = new ZipArchiveEntry(file1);
zipOut.putArchiveEntry(entry);
zipOut.write(new byte[] {1, 1, 1, 1, 1});
zipOut.closeArchiveEntry();
}

String targetPath = temporaryFolder.newFolder().getCanonicalPath();
try {
ZipUtils.extractZipFileWithPermissions(zipFile.getCanonicalPath(), targetPath);
Assert.fail("exception expected");
} catch (IOException e) {
assertTrue(e.getMessage().contains("Expand ../zipFile would create a file outside of"));
}
}

@Test
public void testPermissionRestored() throws IOException {
File zipFile = temporaryFolder.newFile();

try (ZipArchiveOutputStream zipOut =
new ZipArchiveOutputStream(new FileOutputStream(zipFile))) {
String file1 = "zipFile";
ZipArchiveEntry entry = new ZipArchiveEntry(file1);
entry.setUnixMode(0355);
zipOut.putArchiveEntry(entry);
zipOut.write(new byte[] {1, 1, 1, 1, 1});
zipOut.closeArchiveEntry();
}

String targetPath = temporaryFolder.newFolder().getCanonicalPath();
ZipUtils.extractZipFileWithPermissions(zipFile.getCanonicalPath(), targetPath);

Path path = new File(targetPath, "zipFile").toPath();
assertEquals(0355, toUnixMode(Files.getPosixFilePermissions(path)));
}

private int toUnixMode(Set<PosixFilePermission> permission) {
int mode = 0;
for (PosixFilePermission action : PosixFilePermission.values()) {
mode = mode << 1;
mode += permission.contains(action) ? 1 : 0;
}
return mode;
}
}

0 comments on commit c44dd62

Please sign in to comment.