Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-6232

OfflineEditsViewer throws a NPE on edits containing ACL modifications

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0, 2.4.0
    • Fix Version/s: 2.4.1
    • Component/s: tools
    • Labels:
      None

      Description

      The OfflineEditsViewer using the XML parser will through a NPE when using an edit with a SET_ACL op.

      [root@hdfs-nfs current]# hdfs oev -i edits_0000000000000000001-0000000000000000007 -o fsedits.out
      14/04/10 14:14:18 ERROR offlineEditsViewer.OfflineEditsBinaryLoader: Got RuntimeException at position 505
      Encountered exception. Exiting: null
      java.lang.NullPointerException
      	at org.apache.hadoop.hdfs.util.XMLUtils.mangleXmlString(XMLUtils.java:122)
      	at org.apache.hadoop.hdfs.util.XMLUtils.addSaxString(XMLUtils.java:193)
      	at org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.appendAclEntriesToXml(FSEditLogOp.java:4085)
      	at org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.access$3300(FSEditLogOp.java:132)
      	at org.apache.hadoop.hdfs.server.namenode.FSEditLogOp$SetAclOp.toXml(FSEditLogOp.java:3528)
      	at org.apache.hadoop.hdfs.server.namenode.FSEditLogOp.outputToXml(FSEditLogOp.java:3928)
      	at org.apache.hadoop.hdfs.tools.offlineEditsViewer.XmlEditsVisitor.visitOp(XmlEditsVisitor.java:116)
      	at org.apache.hadoop.hdfs.tools.offlineEditsViewer.OfflineEditsBinaryLoader.loadEdits(OfflineEditsBinaryLoader.java:80)
      	at org.apache.hadoop.hdfs.tools.offlineEditsViewer.OfflineEditsViewer.go(OfflineEditsViewer.java:142)
      	at org.apache.hadoop.hdfs.tools.offlineEditsViewer.OfflineEditsViewer.run(OfflineEditsViewer.java:228)
      	at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:70)
      	at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:84)
      	at org.apache.hadoop.hdfs.tools.offlineEditsViewer.OfflineEditsViewer.main(OfflineEditsViewer.java:237)
      [root@hdfs-nfs current]# 
      

      This is reproducible by setting an acl on a file and then running the OEV on the editsinprogress file.

      The stats and binary parsers run OK.

      1. HDFS-6232.patch
        3 kB
        Akira AJISAKA
      2. HDFS-6232.2.patch
        4 kB
        Akira AJISAKA

        Issue Links

          Activity

          Hide
          Chris Nauroth added a comment -

          Stephen Chu, thank you for reporting the bug. Akira AJISAKA, thank you for fixing it.

          Show
          Chris Nauroth added a comment - Stephen Chu , thank you for reporting the bug. Akira AJISAKA , thank you for fixing it.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1755 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1755/)
          HDFS-6232. OfflineEditsViewer throws a NPE on edits containing ACL modifications (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586790)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #1755 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1755/ ) HDFS-6232 . OfflineEditsViewer throws a NPE on edits containing ACL modifications (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586790 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk #1730 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1730/)
          HDFS-6232. OfflineEditsViewer throws a NPE on edits containing ACL modifications (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586790)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #1730 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1730/ ) HDFS-6232 . OfflineEditsViewer throws a NPE on edits containing ACL modifications (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586790 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #538 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/538/)
          HDFS-6232. OfflineEditsViewer throws a NPE on edits containing ACL modifications (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586790)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #538 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/538/ ) HDFS-6232 . OfflineEditsViewer throws a NPE on edits containing ACL modifications (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586790 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #5507 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5507/)
          HDFS-6232. OfflineEditsViewer throws a NPE on edits containing ACL modifications (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586790)

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #5507 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5507/ ) HDFS-6232 . OfflineEditsViewer throws a NPE on edits containing ACL modifications (ajisakaa via cmccabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1586790 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/DFSTestUtil.java
          Hide
          Colin Patrick McCabe added a comment -

          committed. Thanks, Akira.

          Show
          Colin Patrick McCabe added a comment - committed. Thanks, Akira.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12639853/HDFS-6232.2.patch
          against trunk revision .

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

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

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

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +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 unit tests in hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6653//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6653//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/12639853/HDFS-6232.2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +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 unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6653//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6653//console This message is automatically generated.
          Hide
          Colin Patrick McCabe added a comment -

          +1 pending jenkins. thanks, Akira.

          Show
          Colin Patrick McCabe added a comment - +1 pending jenkins. thanks, Akira.
          Hide
          Akira AJISAKA added a comment -

          Thanks Colin for the comment. I updated the patch to distinguish null and the empty string.

          Yeah. The deprecation warnings aren't relevant to this JIRA. Check out HDFS-4629 if you're interested in a solution to that.

          Thanks again for the linking.

          Show
          Akira AJISAKA added a comment - Thanks Colin for the comment. I updated the patch to distinguish null and the empty string. Yeah. The deprecation warnings aren't relevant to this JIRA. Check out HDFS-4629 if you're interested in a solution to that. Thanks again for the linking.
          Hide
          Colin Patrick McCabe added a comment -

          It looks like you are trying to make XMLUtils#addSaxString treat a null value as equal to the empty string. This seems confusing to me, since when we read the XML file and create edits again, we can't distinguish between null and the empty string.

          While it's possible that null and empty really are interchangeable here, I would rather leave it to the caller to make this determination. Why not just do the simple thing and fix the ACL code so it doesn't pass null to this function?

          I tried to use org.apache.xml.serialize.XMLSerializer in Apache Xerces, but it was deprecated. I'm thinking we should use another library or write our own code.

          Yeah. The deprecation warnings aren't relevant to this JIRA. Check out HDFS-4629 if you're interested in a solution to that.

          Show
          Colin Patrick McCabe added a comment - It looks like you are trying to make XMLUtils#addSaxString treat a null value as equal to the empty string. This seems confusing to me, since when we read the XML file and create edits again, we can't distinguish between null and the empty string. While it's possible that null and empty really are interchangeable here, I would rather leave it to the caller to make this determination. Why not just do the simple thing and fix the ACL code so it doesn't pass null to this function? I tried to use org.apache.xml.serialize.XMLSerializer in Apache Xerces, but it was deprecated. I'm thinking we should use another library or write our own code. Yeah. The deprecation warnings aren't relevant to this JIRA. Check out HDFS-4629 if you're interested in a solution to that.
          Hide
          Akira AJISAKA added a comment -

          I suggest to use Xerces instead.

          I tried to use org.apache.xml.serialize.XMLSerializer in Apache Xerces, but it was deprecated. I'm thinking we should use another library or write our own code.

          Show
          Akira AJISAKA added a comment - I suggest to use Xerces instead. I tried to use org.apache.xml.serialize.XMLSerializer in Apache Xerces, but it was deprecated. I'm thinking we should use another library or write our own code.
          Hide
          Akira AJISAKA added a comment -

          The javac warnings in the test code are due to the use of com.sun.org.apache.xml.internal.serialize.XMLSerializer and com.sun.org.apache.xml.internal.serialize.OutputFormat. They are already used by OfflineEditsViewer.
          I suggest to use Xerces instead.

          Show
          Akira AJISAKA added a comment - The javac warnings in the test code are due to the use of com.sun.org.apache.xml.internal.serialize.XMLSerializer and com.sun.org.apache.xml.internal.serialize.OutputFormat . They are already used by OfflineEditsViewer. I suggest to use Xerces 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/12639741/HDFS-6232.patch
          against trunk revision .

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

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

          -1 javac. The applied patch generated 1293 javac compiler warnings (more than the trunk's current 1287 warnings).

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +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 unit tests in hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6651//testReport/
          Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6651//artifact/trunk/patchprocess/diffJavacWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6651//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/12639741/HDFS-6232.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. -1 javac . The applied patch generated 1293 javac compiler warnings (more than the trunk's current 1287 warnings). +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +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 unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6651//testReport/ Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/6651//artifact/trunk/patchprocess/diffJavacWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6651//console This message is automatically generated.
          Hide
          Akira AJISAKA added a comment -

          I confirmed the patch fixed the issue locally.

          [root@trunk hadoop-3.0.0-SNAPSHOT]# bin/hdfs oev -i edits_inprogress_0000000000000005251 -o fsedits.out
          [root@trunk hadoop-3.0.0-SNAPSHOT]# cat fsedits.out
          <?xml version="1.0" encoding="UTF-8"?>
          <EDITS>
            <EDITS_VERSION>-56</EDITS_VERSION>
            <RECORD>
              <OPCODE>OP_START_LOG_SEGMENT</OPCODE>
              <DATA>
                <TXID>5251</TXID>
              </DATA>
            </RECORD>
            <RECORD>
              <OPCODE>OP_SET_ACL</OPCODE>
              <DATA>
                <TXID>5252</TXID>
                <SRC>/user/root</SRC>
                <ENTRY>
                  <SCOPE>ACCESS</SCOPE>
                  <TYPE>USER</TYPE>
                  <NAME></NAME>
                  <PERM>rwx</PERM>
                </ENTRY>
          
          Show
          Akira AJISAKA added a comment - I confirmed the patch fixed the issue locally. [root@trunk hadoop-3.0.0-SNAPSHOT]# bin/hdfs oev -i edits_inprogress_0000000000000005251 -o fsedits.out [root@trunk hadoop-3.0.0-SNAPSHOT]# cat fsedits.out <?xml version= "1.0" encoding= "UTF-8" ?> <EDITS> <EDITS_VERSION>-56</EDITS_VERSION> <RECORD> <OPCODE>OP_START_LOG_SEGMENT</OPCODE> <DATA> <TXID>5251</TXID> </DATA> </RECORD> <RECORD> <OPCODE>OP_SET_ACL</OPCODE> <DATA> <TXID>5252</TXID> <SRC>/user/root</SRC> <ENTRY> <SCOPE>ACCESS</SCOPE> <TYPE>USER</TYPE> <NAME></NAME> <PERM>rwx</PERM> </ENTRY>
          Hide
          Akira AJISAKA added a comment -

          I reproduced the error. It occurs because XMLUtils.addSaxString can't handle null ACL entry name. The name is an optional value, so it can be null.
          I attached a patch to add null check.

          Show
          Akira AJISAKA added a comment - I reproduced the error. It occurs because XMLUtils.addSaxString can't handle null ACL entry name. The name is an optional value, so it can be null. I attached a patch to add null check.

            People

            • Assignee:
              Akira AJISAKA
              Reporter:
              Stephen Chu
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development