Repository: nifi Updated Branches: refs/heads/master a8e1c775f -> 611cadd23
NIFI-2919 Improved GetFile processor to fail (and provide bulletins) if target directory is inaccessible. This closes #1147. Signed-off-by: Andy LoPresto <[email protected]> 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/master Commit: 611cadd231309126a0a7737391f6e07730bb3864 Parents: a8e1c77 Author: Oleg Zhurakousky <[email protected]> Authored: Wed Oct 19 09:30:12 2016 -0400 Committer: Andy LoPresto <[email protected]> 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());
