Hadoop Common
  1. Hadoop Common
  2. HADOOP-7327

FileSystem.listStatus() throws NullPointerException instead of IOException upon access permission failure

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.22.0, 0.23.0
    • Fix Version/s: 0.23.0
    • Component/s: fs
    • Labels:
      None

      Description

      Many processes that call listStatus() expect to handle IOException, but instead are getting runtime error NullPointerException, if the directory being scanned is visible but no-access to the running user id. For example, if directory foo is drwxr-xr-x, and subdirectory foo/bar is drwx------, then trying to do listStatus(Path(foo/bar)) will cause a NullPointerException.

      1. hadoop-7327-2.patch
        3 kB
        Matt Foley
      2. hadoop-7327-1.patch
        0.6 kB
        Matt Foley

        Issue Links

          Activity

          Hide
          Matt Foley added a comment -

          The problem is that the underlying implementation relies on File.list(), which returns null upon access error, rather than throwing IOException. (The underlying implementation can be hard to track down because of layering of FilterFileSystems, but for an example see RawLocalFileSystem.listStatus(Path).)

          We can't change that behavior, because it is defined by the Java io library. But at our FileSystem class level, it is appropriate to check for this condition at the point where FileSystem.listStatus() is about to generate a NullPointerException, and throw an IOException instead.

          Show
          Matt Foley added a comment - The problem is that the underlying implementation relies on File.list(), which returns null upon access error, rather than throwing IOException. (The underlying implementation can be hard to track down because of layering of FilterFileSystems, but for an example see RawLocalFileSystem.listStatus(Path).) We can't change that behavior, because it is defined by the Java io library. But at our FileSystem class level, it is appropriate to check for this condition at the point where FileSystem.listStatus() is about to generate a NullPointerException, and throw an IOException instead.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12480218/hadoop-7327-1.patch
          against trunk revision 1126719.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these core unit tests:

          -1 system test framework. The patch failed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/512//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/512//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/512//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12480218/hadoop-7327-1.patch against trunk revision 1126719. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these core unit tests: -1 system test framework. The patch failed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/512//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/512//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/512//console This message is automatically generated.
          Hide
          Todd Lipcon added a comment -

          This looks good, but unfortunately the Hudson build slave seems to be in bad shape (disk full). I'm working on getting that fixed. Would it be possible to run the tests locally in the meantime?

          Also, can you add a unit test? Should be easy to mkdir/chown a directory to reproduce the issue.

          Show
          Todd Lipcon added a comment - This looks good, but unfortunately the Hudson build slave seems to be in bad shape (disk full). I'm working on getting that fixed. Would it be possible to run the tests locally in the meantime? Also, can you add a unit test? Should be easy to mkdir/chown a directory to reproduce the issue.
          Hide
          Daryn Sharp added a comment -

          A generic IOE with a message of "Error accessing", while definitively preferable to a NPE that tears down FsShell, affords the user no idea of the underlying error. At a minimum, it would be nice to distinguish a permission denied from a "something went wrong". I think that would still conform to NIO behavior since the security manager will throw an exception.

          Show
          Daryn Sharp added a comment - A generic IOE with a message of "Error accessing", while definitively preferable to a NPE that tears down FsShell, affords the user no idea of the underlying error. At a minimum, it would be nice to distinguish a permission denied from a "something went wrong". I think that would still conform to NIO behavior since the security manager will throw an exception.
          Hide
          Daryn Sharp added a comment -

          Also, I'm pretty sure acl exceptions were being thrown a few months. Did something maybe change in the NN?

          Show
          Daryn Sharp added a comment - Also, I'm pretty sure acl exceptions were being thrown a few months. Did something maybe change in the NN?
          Hide
          Matt Foley added a comment -

          Daryn, at the point of this patch insertion, all we know is that the underlying call (via the abstract API) returned null. How do you propose to distinguish among the possible causes?

          Show
          Matt Foley added a comment - Daryn, at the point of this patch insertion, all we know is that the underlying call (via the abstract API) returned null. How do you propose to distinguish among the possible causes?
          Hide
          Matt Foley added a comment -

          For the larger question of a fix for all calls to FileSystem.listStatus(Path) which may cause NPE, please see the new ticket HADOOP-7352.

          Since that's an architectural issue that is likely to take a while to hash out, this ticket HADOOP-7327 is hereby restricted to a quick, correct fix for this single instance of NPE in FileSystem::listStatus(ArrayList<FileStatus>, Path, PathFilter), in order to fix a persistent fault in automated testing with HDFS-1967.

          Show
          Matt Foley added a comment - For the larger question of a fix for all calls to FileSystem.listStatus(Path) which may cause NPE, please see the new ticket HADOOP-7352 . Since that's an architectural issue that is likely to take a while to hash out, this ticket HADOOP-7327 is hereby restricted to a quick, correct fix for this single instance of NPE in FileSystem::listStatus(ArrayList<FileStatus>, Path, PathFilter), in order to fix a persistent fault in automated testing with HDFS-1967 .
          Hide
          Daryn Sharp added a comment -

          I checked an old path from months ago that I never submitted for FsShell. I see that I was catching AccessControlExceptions to standardized the output format. File access is throwing ACEs but I'm not sure whether dirs were too. It would stand to reason that both file and dir access should throw ACEs, which I think is consistent with NIO behavior?

          That said, I'm only commenting/lamenting so my intention isn't to block this patch since a NPE will prematurely grind many FsShell commands to a halt. For instance, "du" is worthless if just one dir in the tree is unreadable. In many cases this patch is unquestionably better than nothing.

          Show
          Daryn Sharp added a comment - I checked an old path from months ago that I never submitted for FsShell. I see that I was catching AccessControlExceptions to standardized the output format. File access is throwing ACEs but I'm not sure whether dirs were too. It would stand to reason that both file and dir access should throw ACEs, which I think is consistent with NIO behavior? That said, I'm only commenting/lamenting so my intention isn't to block this patch since a NPE will prematurely grind many FsShell commands to a halt. For instance, "du" is worthless if just one dir in the tree is unreadable. In many cases this patch is unquestionably better than nothing.
          Hide
          Matt Foley added a comment -
          Show
          Matt Foley added a comment - @Daryn, for discussion of what exception to throw in the more complete solution, please see https://issues.apache.org/jira/browse/HADOOP-7352?focusedCommentId=13042884&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13042884
          Hide
          Matt Foley added a comment -

          We have two instances of this bug actually causing NPEs in the real world (or at least the Test world).

          The first is in TestTrash.testTrashEmptier(), which runs on the LocalFileSystem, and launches a Trash$Emptier thread in background. Every time that thread cycles, it attempts to empty the local .Trash directory in EVERY "home directory" known. On my Mac, there are four user accounts, each with its own "home" directory, one of which, /Users/test, has a .Trash directory with access permissions set so it is not readable by me. When it tries to list the .Trash subdirectories with FileSystem.listStatus(), it logs the following:

          11/05/23 14:34:54 WARN fs.Trash: RuntimeException during Trash.Emptier.run() java.lang.NullPointerException
          at org.apache.hadoop.fs.FileSystem.listStatus(FileSystem.java:1114)
          at org.apache.hadoop.fs.FileSystem.listStatus(FileSystem.java:1137)
          at org.apache.hadoop.fs.ChecksumFileSystem.listStatus(ChecksumFileSystem.java:494)
          at org.apache.hadoop.fs.Trash.expunge(Trash.java:215)
          at org.apache.hadoop.fs.Trash$Emptier.run(Trash.java:313)
          at java.lang.Thread.run(Thread.java:680)

          The second example is from Daryn, and was partly described in HADOOP-7345. When he tries to use "bin/hadoop fs -ls aaa" on any directory "aaa" for which he does not have access privs, it logs:

          java.lang.NullPointerException
          at org.apache.hadoop.fs.FileSystem.listStatus(FileSystem.java:1115)
          at org.apache.hadoop.fs.FileSystem.listStatus(FileSystem.java:1150)
          at org.apache.hadoop.fs.ChecksumFileSystem.listStatus(ChecksumFileSystem.java:494)
          at org.apache.hadoop.fs.shell.PathData.getDirectoryContents(PathData.java:163)

          It is seen that both of these cases go through the FileSystem.listStatus(ArrayList<FileStatus>, Path, PathFilter) form of the call, and it is here that the attached patch is targeted.

          Show
          Matt Foley added a comment - We have two instances of this bug actually causing NPEs in the real world (or at least the Test world). The first is in TestTrash.testTrashEmptier(), which runs on the LocalFileSystem, and launches a Trash$Emptier thread in background. Every time that thread cycles, it attempts to empty the local .Trash directory in EVERY "home directory" known. On my Mac, there are four user accounts, each with its own "home" directory, one of which, /Users/test, has a .Trash directory with access permissions set so it is not readable by me. When it tries to list the .Trash subdirectories with FileSystem.listStatus(), it logs the following: 11/05/23 14:34:54 WARN fs.Trash: RuntimeException during Trash.Emptier.run() java.lang.NullPointerException at org.apache.hadoop.fs.FileSystem.listStatus(FileSystem.java:1114) at org.apache.hadoop.fs.FileSystem.listStatus(FileSystem.java:1137) at org.apache.hadoop.fs.ChecksumFileSystem.listStatus(ChecksumFileSystem.java:494) at org.apache.hadoop.fs.Trash.expunge(Trash.java:215) at org.apache.hadoop.fs.Trash$Emptier.run(Trash.java:313) at java.lang.Thread.run(Thread.java:680) The second example is from Daryn, and was partly described in HADOOP-7345 . When he tries to use "bin/hadoop fs -ls aaa" on any directory "aaa" for which he does not have access privs, it logs: java.lang.NullPointerException at org.apache.hadoop.fs.FileSystem.listStatus(FileSystem.java:1115) at org.apache.hadoop.fs.FileSystem.listStatus(FileSystem.java:1150) at org.apache.hadoop.fs.ChecksumFileSystem.listStatus(ChecksumFileSystem.java:494) at org.apache.hadoop.fs.shell.PathData.getDirectoryContents(PathData.java:163) It is seen that both of these cases go through the FileSystem.listStatus(ArrayList<FileStatus>, Path, PathFilter) form of the call, and it is here that the attached patch is targeted.
          Hide
          Matt Foley added a comment -

          Although this issue was discovered while investigating HDFS-1852, it does not actually block it, so removing the "blocker" link. It doesn't cause TestTrash to fail, it just junks up the log with NPEs while running TestTrash on a local system.

          Show
          Matt Foley added a comment - Although this issue was discovered while investigating HDFS-1852 , it does not actually block it, so removing the "blocker" link. It doesn't cause TestTrash to fail, it just junks up the log with NPEs while running TestTrash on a local system.
          Hide
          Daryn Sharp added a comment -

          +1 It looks like the issue will be more comprehensively addressed by HADOOP-7352, so I'm fine with this as an incremental improvement.

          Show
          Daryn Sharp added a comment - +1 It looks like the issue will be more comprehensively addressed by HADOOP-7352 , so I'm fine with this as an incremental improvement.
          Hide
          Matt Foley added a comment -

          Correct.

          I've added a test case to FSMainOperationsBaseTest. Also touched up a couple apparent errors in that unit test file, such as no @Test annotations on two testcases that work just fine and fast.

          Show
          Matt Foley added a comment - Correct. I've added a test case to FSMainOperationsBaseTest. Also touched up a couple apparent errors in that unit test file, such as no @Test annotations on two testcases that work just fine and fast.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12482998/hadoop-7327-2.patch
          against trunk revision 1136249.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/650//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/650//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/650//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12482998/hadoop-7327-2.patch against trunk revision 1136249. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/650//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/650//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/650//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          +1 still looks good!

          Show
          Daryn Sharp added a comment - +1 still looks good!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #675 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/675/)
          HADOOP-7327. FileSystem.listStatus() throws NullPointerException instead of IOException upon access permission failure. Contributed by Matt Foley.

          mattf : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1143491
          Files :

          • /hadoop/common/trunk/common/CHANGES.txt
          • /hadoop/common/trunk/common/src/java/org/apache/hadoop/fs/FileSystem.java
          • /hadoop/common/trunk/common/src/test/core/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #675 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/675/ ) HADOOP-7327 . FileSystem.listStatus() throws NullPointerException instead of IOException upon access permission failure. Contributed by Matt Foley. mattf : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1143491 Files : /hadoop/common/trunk/common/CHANGES.txt /hadoop/common/trunk/common/src/java/org/apache/hadoop/fs/FileSystem.java /hadoop/common/trunk/common/src/test/core/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #740 (See https://builds.apache.org/job/Hadoop-Common-trunk/740/)
          HADOOP-7327. FileSystem.listStatus() throws NullPointerException instead of IOException upon access permission failure. Contributed by Matt Foley.

          mattf : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1143491
          Files :

          • /hadoop/common/trunk/common/CHANGES.txt
          • /hadoop/common/trunk/common/src/java/org/apache/hadoop/fs/FileSystem.java
          • /hadoop/common/trunk/common/src/test/core/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #740 (See https://builds.apache.org/job/Hadoop-Common-trunk/740/ ) HADOOP-7327 . FileSystem.listStatus() throws NullPointerException instead of IOException upon access permission failure. Contributed by Matt Foley. mattf : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1143491 Files : /hadoop/common/trunk/common/CHANGES.txt /hadoop/common/trunk/common/src/java/org/apache/hadoop/fs/FileSystem.java /hadoop/common/trunk/common/src/test/core/org/apache/hadoop/fs/FSMainOperationsBaseTest.java
          Hide
          Matt Foley added a comment -

          Committed to trunk.

          Show
          Matt Foley added a comment - Committed to trunk.

            People

            • Assignee:
              Matt Foley
              Reporter:
              Matt Foley
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development