You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by al...@apache.org on 2016/10/21 19:58:59 UTC
[14/27] nifi git commit: NIFI-2919 Improved GetFile processor to fail
(and provide bulletins) if target directory is inaccessible.
NIFI-2919 Improved GetFile processor to fail (and provide bulletins) if target directory is inaccessible.
This closes #1147.
Signed-off-by: Andy LoPresto <al...@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
Project: http://git-wip-us.apache.org/repos/asf/nifi/repo
Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/611cadd2
Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/611cadd2
Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/611cadd2
Branch: refs/heads/appveyor-improvement
Commit: 611cadd231309126a0a7737391f6e07730bb3864
Parents: a8e1c77
Author: Oleg Zhurakousky <ol...@suitcase.io>
Authored: Wed Oct 19 09:30:12 2016 -0400
Committer: Andy LoPresto <al...@apache.org>
Committed: Wed Oct 19 16:33:33 2016 -0400
----------------------------------------------------------------------
.../nifi/processors/standard/GetFile.java | 9 +-
.../nifi/processors/standard/TestGetFile.java | 101 +++++++++++++++----
2 files changed, 87 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/nifi/blob/611cadd2/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetFile.java
----------------------------------------------------------------------
diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetFile.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetFile.java
index 45c95eb..cdfb857 100644
--- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetFile.java
+++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/GetFile.java
@@ -48,7 +48,6 @@ import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.regex.Pattern;
-
import org.apache.nifi.annotation.behavior.InputRequirement;
import org.apache.nifi.annotation.behavior.InputRequirement.Requirement;
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) {
+ 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<>();
if (!directory.exists()) {
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();
if (children == null) {
http://git-wip-us.apache.org/repos/asf/nifi/blob/611cadd2/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestGetFile.java
----------------------------------------------------------------------
diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestGetFile.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestGetFile.java
index eb8a764..daf807a 100644
--- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestGetFile.java
+++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestGetFile.java
@@ -16,8 +16,6 @@
*/
package org.apache.nifi.processors.standard;
-import org.apache.nifi.processors.standard.GetFile;
-
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -32,10 +30,10 @@ import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
+import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;
-
import org.apache.nifi.flowfile.attributes.CoreAttributes;
import org.apache.nifi.util.MockFlowFile;
import org.apache.nifi.util.TestRunner;
@@ -45,6 +43,72 @@ import org.junit.Test;
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
public void testFilePickedUp() throws IOException {
final File directory = new File("target/test/data/in");
deleteDirectory(directory);
@@ -73,7 +137,7 @@ public class TestGetFile {
}
private void deleteDirectory(final File directory) throws IOException {
- if (directory.exists()) {
+ if (directory != null && directory.exists()) {
for (final File file : directory.listFiles()) {
if (file.isDirectory()) {
deleteDirectory(file);
@@ -155,29 +219,30 @@ public class TestGetFile {
try {
destFile.setLastModified(1000000000);
verifyLastModified = true;
- } catch (Exception donothing) {
+ } catch (Exception doNothing) {
}
boolean verifyPermissions = false;
try {
- // 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.
- // 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
- // attribute views, but the setPosixFilePermissions() has no effect.
- //
- // 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.
- // The purpose of the extra check is to ensure, that setPosixFilePermissions() changes the file's
- // permissions, and set verifyPermissions, after we are convinced.
+ /* 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.
+ * 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
+ * attribute views, but the setPosixFilePermissions() has no effect.
+ *
+ * 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.
+ * The purpose of the extra check is to ensure, that setPosixFilePermissions() changes the file's
+ * permissions, and set verifyPermissions, after we are convinced.
+ */
Set<PosixFilePermission> perms = PosixFilePermissions.fromString("r--r-----");
Files.setPosixFilePermissions(targetPath, perms);
- Set<PosixFilePermission> permsAfterSet = Files.getPosixFilePermissions(targetPath);
+ Set<PosixFilePermission> permsAfterSet = Files.getPosixFilePermissions(targetPath);
if (perms.equals(permsAfterSet)) {
- verifyPermissions = true;
+ verifyPermissions = true;
}
- } catch (Exception donothing) {
+ } catch (Exception doNothing) {
}
final TestRunner runner = TestRunners.newTestRunner(new GetFile());