mirror of https://github.com/apache/nifi.git
NIFI-2919 Improved GetFile processor to fail (and provide bulletins) if target directory is inaccessible.
This closes #1147.
Signed-off-by: Andy LoPresto <alopresto@apache.org>
Fixed typos in error messages, renamed variables in test, and cleaned up unnecessary imports. (+1 squashed commit)
Squashed commits:
[e755cbd
] NIFI-2919 improved GetFile to fail if target directory is inaccessible
This commit is contained in:
parent
a8e1c775fd
commit
611cadd231
|
@ -48,7 +48,6 @@ import java.util.concurrent.atomic.AtomicReference;
|
||||||
import java.util.concurrent.locks.Lock;
|
import java.util.concurrent.locks.Lock;
|
||||||
import java.util.concurrent.locks.ReentrantLock;
|
import java.util.concurrent.locks.ReentrantLock;
|
||||||
import java.util.regex.Pattern;
|
import java.util.regex.Pattern;
|
||||||
|
|
||||||
import org.apache.nifi.annotation.behavior.InputRequirement;
|
import org.apache.nifi.annotation.behavior.InputRequirement;
|
||||||
import org.apache.nifi.annotation.behavior.InputRequirement.Requirement;
|
import org.apache.nifi.annotation.behavior.InputRequirement.Requirement;
|
||||||
import org.apache.nifi.annotation.behavior.TriggerWhenEmpty;
|
import org.apache.nifi.annotation.behavior.TriggerWhenEmpty;
|
||||||
|
@ -295,14 +294,14 @@ public class GetFile extends AbstractProcessor {
|
||||||
}
|
}
|
||||||
|
|
||||||
private Set<File> performListing(final File directory, final FileFilter filter, final boolean recurseSubdirectories) {
|
private Set<File> performListing(final File directory, final FileFilter filter, final boolean recurseSubdirectories) {
|
||||||
|
Path p = directory.toPath();
|
||||||
|
if (!Files.isWritable(p) || !Files.isReadable(p)) {
|
||||||
|
throw new IllegalStateException("Directory '" + directory + "' does not have sufficient permissions (i.e., not writable and readable)");
|
||||||
|
}
|
||||||
final Set<File> queue = new HashSet<>();
|
final Set<File> queue = new HashSet<>();
|
||||||
if (!directory.exists()) {
|
if (!directory.exists()) {
|
||||||
return queue;
|
return queue;
|
||||||
}
|
}
|
||||||
// this check doesn't work on Windows
|
|
||||||
if (!directory.canRead()) {
|
|
||||||
getLogger().warn("No read permission on directory {}", new Object[]{directory.toString()});
|
|
||||||
}
|
|
||||||
|
|
||||||
final File[] children = directory.listFiles();
|
final File[] children = directory.listFiles();
|
||||||
if (children == null) {
|
if (children == null) {
|
||||||
|
|
|
@ -16,8 +16,6 @@
|
||||||
*/
|
*/
|
||||||
package org.apache.nifi.processors.standard;
|
package org.apache.nifi.processors.standard;
|
||||||
|
|
||||||
import org.apache.nifi.processors.standard.GetFile;
|
|
||||||
|
|
||||||
import static org.junit.Assert.assertEquals;
|
import static org.junit.Assert.assertEquals;
|
||||||
import static org.junit.Assert.assertTrue;
|
import static org.junit.Assert.assertTrue;
|
||||||
import static org.junit.Assert.fail;
|
import static org.junit.Assert.fail;
|
||||||
|
@ -32,10 +30,10 @@ import java.text.DateFormat;
|
||||||
import java.text.ParseException;
|
import java.text.ParseException;
|
||||||
import java.text.SimpleDateFormat;
|
import java.text.SimpleDateFormat;
|
||||||
import java.util.Date;
|
import java.util.Date;
|
||||||
|
import java.util.HashSet;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Locale;
|
import java.util.Locale;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
|
|
||||||
import org.apache.nifi.flowfile.attributes.CoreAttributes;
|
import org.apache.nifi.flowfile.attributes.CoreAttributes;
|
||||||
import org.apache.nifi.util.MockFlowFile;
|
import org.apache.nifi.util.MockFlowFile;
|
||||||
import org.apache.nifi.util.TestRunner;
|
import org.apache.nifi.util.TestRunner;
|
||||||
|
@ -44,6 +42,72 @@ import org.junit.Test;
|
||||||
|
|
||||||
public class TestGetFile {
|
public class TestGetFile {
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testWithInaccessibleDir() throws IOException {
|
||||||
|
File inaccessibleDir = new File("target/inaccessible");
|
||||||
|
inaccessibleDir.deleteOnExit();
|
||||||
|
inaccessibleDir.mkdir();
|
||||||
|
Set<PosixFilePermission> posixFilePermissions = new HashSet<>();
|
||||||
|
Files.setPosixFilePermissions(inaccessibleDir.toPath(), posixFilePermissions);
|
||||||
|
final TestRunner runner = TestRunners.newTestRunner(new GetFile());
|
||||||
|
runner.setProperty(GetFile.DIRECTORY, inaccessibleDir.getAbsolutePath());
|
||||||
|
try {
|
||||||
|
runner.run();
|
||||||
|
fail();
|
||||||
|
} catch (AssertionError e) {
|
||||||
|
assertTrue(e.getCause().getMessage()
|
||||||
|
.endsWith("does not have sufficient permissions (i.e., not writable and readable)"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testWithUnreadableDir() throws IOException {
|
||||||
|
File unreadableDir = new File("target/unreadable");
|
||||||
|
unreadableDir.deleteOnExit();
|
||||||
|
unreadableDir.mkdir();
|
||||||
|
Set<PosixFilePermission> posixFilePermissions = new HashSet<>();
|
||||||
|
posixFilePermissions.add(PosixFilePermission.GROUP_EXECUTE);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.GROUP_WRITE);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.OTHERS_EXECUTE);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.OTHERS_WRITE);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.OWNER_EXECUTE);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.OWNER_WRITE);
|
||||||
|
Files.setPosixFilePermissions(unreadableDir.toPath(), posixFilePermissions);
|
||||||
|
final TestRunner runner = TestRunners.newTestRunner(new GetFile());
|
||||||
|
runner.setProperty(GetFile.DIRECTORY, unreadableDir.getAbsolutePath());
|
||||||
|
try {
|
||||||
|
runner.run();
|
||||||
|
fail();
|
||||||
|
} catch (AssertionError e) {
|
||||||
|
assertTrue(e.getCause().getMessage()
|
||||||
|
.endsWith("does not have sufficient permissions (i.e., not writable and readable)"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testWithUnwritableDir() throws IOException {
|
||||||
|
File unwritableDir = new File("target/unwritable");
|
||||||
|
unwritableDir.deleteOnExit();
|
||||||
|
unwritableDir.mkdir();
|
||||||
|
Set<PosixFilePermission> posixFilePermissions = new HashSet<>();
|
||||||
|
posixFilePermissions.add(PosixFilePermission.GROUP_EXECUTE);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.GROUP_READ);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.OTHERS_EXECUTE);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.OTHERS_READ);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.OWNER_EXECUTE);
|
||||||
|
posixFilePermissions.add(PosixFilePermission.OWNER_READ);
|
||||||
|
Files.setPosixFilePermissions(unwritableDir.toPath(), posixFilePermissions);
|
||||||
|
final TestRunner runner = TestRunners.newTestRunner(new GetFile());
|
||||||
|
runner.setProperty(GetFile.DIRECTORY, unwritableDir.getAbsolutePath());
|
||||||
|
try {
|
||||||
|
runner.run();
|
||||||
|
fail();
|
||||||
|
} catch (AssertionError e) {
|
||||||
|
assertTrue(e.getCause().getMessage()
|
||||||
|
.endsWith("does not have sufficient permissions (i.e., not writable and readable)"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testFilePickedUp() throws IOException {
|
public void testFilePickedUp() throws IOException {
|
||||||
final File directory = new File("target/test/data/in");
|
final File directory = new File("target/test/data/in");
|
||||||
|
@ -73,7 +137,7 @@ public class TestGetFile {
|
||||||
}
|
}
|
||||||
|
|
||||||
private void deleteDirectory(final File directory) throws IOException {
|
private void deleteDirectory(final File directory) throws IOException {
|
||||||
if (directory.exists()) {
|
if (directory != null && directory.exists()) {
|
||||||
for (final File file : directory.listFiles()) {
|
for (final File file : directory.listFiles()) {
|
||||||
if (file.isDirectory()) {
|
if (file.isDirectory()) {
|
||||||
deleteDirectory(file);
|
deleteDirectory(file);
|
||||||
|
@ -155,29 +219,30 @@ public class TestGetFile {
|
||||||
try {
|
try {
|
||||||
destFile.setLastModified(1000000000);
|
destFile.setLastModified(1000000000);
|
||||||
verifyLastModified = true;
|
verifyLastModified = true;
|
||||||
} catch (Exception donothing) {
|
} catch (Exception doNothing) {
|
||||||
}
|
}
|
||||||
|
|
||||||
boolean verifyPermissions = false;
|
boolean verifyPermissions = false;
|
||||||
try {
|
try {
|
||||||
// If you mount an NTFS partition in Linux, you are unable to change the permissions of the files,
|
/* If you mount an NTFS partition in Linux, you are unable to change the permissions of the files,
|
||||||
// because every file has the same permissions, controlled by the 'fmask' and 'dmask' mount options.
|
* because every file has the same permissions, controlled by the 'fmask' and 'dmask' mount options.
|
||||||
// Executing a chmod command will not fail, but it does not change the file's permissions.
|
* Executing a chmod command will not fail, but it does not change the file's permissions.
|
||||||
// From Java perspective the NTFS mount point, as a FileStore supports the 'unix' and 'posix' file
|
* From Java perspective the NTFS mount point, as a FileStore supports the 'unix' and 'posix' file
|
||||||
// attribute views, but the setPosixFilePermissions() has no effect.
|
* attribute views, but the setPosixFilePermissions() has no effect.
|
||||||
//
|
*
|
||||||
// If you set verifyPermissions to true without the following extra check, the test case will fail
|
* If you set verifyPermissions to true without the following extra check, the test case will fail
|
||||||
// on a file system, where Nifi source is located on a NTFS mount point in Linux.
|
* on a file system, where Nifi source is located on a NTFS mount point in Linux.
|
||||||
// The purpose of the extra check is to ensure, that setPosixFilePermissions() changes the file's
|
* The purpose of the extra check is to ensure, that setPosixFilePermissions() changes the file's
|
||||||
// permissions, and set verifyPermissions, after we are convinced.
|
* permissions, and set verifyPermissions, after we are convinced.
|
||||||
|
*/
|
||||||
Set<PosixFilePermission> perms = PosixFilePermissions.fromString("r--r-----");
|
Set<PosixFilePermission> perms = PosixFilePermissions.fromString("r--r-----");
|
||||||
Files.setPosixFilePermissions(targetPath, perms);
|
Files.setPosixFilePermissions(targetPath, perms);
|
||||||
Set<PosixFilePermission> permsAfterSet = Files.getPosixFilePermissions(targetPath);
|
Set<PosixFilePermission> permsAfterSet = Files.getPosixFilePermissions(targetPath);
|
||||||
if (perms.equals(permsAfterSet)) {
|
if (perms.equals(permsAfterSet)) {
|
||||||
verifyPermissions = true;
|
verifyPermissions = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
} catch (Exception donothing) {
|
} catch (Exception doNothing) {
|
||||||
}
|
}
|
||||||
|
|
||||||
final TestRunner runner = TestRunners.newTestRunner(new GetFile());
|
final TestRunner runner = TestRunners.newTestRunner(new GetFile());
|
||||||
|
|
Loading…
Reference in New Issue