HBase
  1. HBase
  2. HBASE-3585

isLegalFamilyName() can throw ArrayOutOfBoundException

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.90.1, 0.95.2
    • Fix Version/s: 0.95.0
    • Component/s: Client
    • Labels:
      None

      Description

      org.apache.hadoop.hbase.HColumnDescriptor.isLegalFamilyName(byte[]) accesses byte[0] w/o first checking the array length.

      1. HBASE-3585.patch
        3 kB
        Uma Maheswara Rao G

        Issue Links

          Activity

          Prakash Khemani created issue -
          Hide
          stack added a comment -

          @Prakash What did you enter to generate the issue? Thanks.

          Show
          stack added a comment - @Prakash What did you enter to generate the issue? Thanks.
          Hide
          ryan rawson added a comment -

          interesting catch, can you provide a test and patch that fixes it? we should probably throw a better exception I think huh?

          Show
          ryan rawson added a comment - interesting catch, can you provide a test and patch that fixes it? we should probably throw a better exception I think huh?
          Hide
          Prakash Khemani added a comment -

          The ArrayOutOfBound exception happened when doing admin.create(htd) where the HTableDescriptor htd had zero length "" family-name.

          Show
          Prakash Khemani added a comment - The ArrayOutOfBound exception happened when doing admin.create(htd) where the HTableDescriptor htd had zero length "" family-name.
          stack made changes -
          Field Original Value New Value
          Fix Version/s 0.92.0 [ 12314223 ]
          Hide
          stack added a comment -

          Moving out of 0.92. Move it back in if you think differently.

          Show
          stack added a comment - Moving out of 0.92. Move it back in if you think differently.
          stack made changes -
          Fix Version/s 0.92.0 [ 12314223 ]
          Hide
          stack added a comment -

          Moving out of 0.92. Move it back in if you think differently.

          Show
          stack added a comment - Moving out of 0.92. Move it back in if you think differently.
          Hide
          Uma Maheswara Rao G added a comment -

          Currently in trunk, isLegalFamilyName used in below constructor and that is deprecated constructor.

           @Deprecated
            public HColumnDescriptor(final byte[] familyName, final int minVersions,
                final int maxVersions, final boolean keepDeletedCells,
                final String compression, final boolean encodeOnDisk,
                final String dataBlockEncoding, final boolean inMemory,
                final boolean blockCacheEnabled, final int blocksize,
                final int timeToLive, final String bloomFilter, final int scope) {
              isLegalFamilyName(familyName);
          

          And the usable constructor is making use of this deprecated constructors internally. Also validated for familyName length in this usable constructor already.

          public HColumnDescriptor(final byte [] familyName) {
              this (familyName == null || familyName.length <= 0?
                HConstants.EMPTY_BYTE_ARRAY: familyName, DEFAULT_VERSIONS,
                DEFAULT_COMPRESSION, DEFAULT_IN_MEMORY, DEFAULT_BLOCKCACHE,
                DEFAULT_TTL, DEFAULT_BLOOMFILTER);
            }
          

          But isLegalFamilyName is marked as public and used only in HColumnDescriptor class.

          Anyway ths simple fix couldbe to move down that validation to last constructor, where we are finally populating the values with setter metods.

           public HColumnDescriptor(final byte[] familyName, final int minVersions,
                final int maxVersions, final boolean keepDeletedCells,
                final String compression, final boolean encodeOnDisk,
                final String dataBlockEncoding, final boolean inMemory,
                final boolean blockCacheEnabled, final int blocksize,
                final int timeToLive, final String bloomFilter, final int scope) {
          
          Show
          Uma Maheswara Rao G added a comment - Currently in trunk, isLegalFamilyName used in below constructor and that is deprecated constructor. @Deprecated public HColumnDescriptor( final byte [] familyName, final int minVersions, final int maxVersions, final boolean keepDeletedCells, final String compression, final boolean encodeOnDisk, final String dataBlockEncoding, final boolean inMemory, final boolean blockCacheEnabled, final int blocksize, final int timeToLive, final String bloomFilter, final int scope) { isLegalFamilyName(familyName); And the usable constructor is making use of this deprecated constructors internally. Also validated for familyName length in this usable constructor already. public HColumnDescriptor( final byte [] familyName) { this (familyName == null || familyName.length <= 0? HConstants.EMPTY_BYTE_ARRAY: familyName, DEFAULT_VERSIONS, DEFAULT_COMPRESSION, DEFAULT_IN_MEMORY, DEFAULT_BLOCKCACHE, DEFAULT_TTL, DEFAULT_BLOOMFILTER); } But isLegalFamilyName is marked as public and used only in HColumnDescriptor class. Anyway ths simple fix couldbe to move down that validation to last constructor, where we are finally populating the values with setter metods. public HColumnDescriptor( final byte [] familyName, final int minVersions, final int maxVersions, final boolean keepDeletedCells, final String compression, final boolean encodeOnDisk, final String dataBlockEncoding, final boolean inMemory, final boolean blockCacheEnabled, final int blocksize, final int timeToLive, final String bloomFilter, final int scope) {
          Hide
          stack added a comment -

          Have a patch Uma?

          Show
          stack added a comment - Have a patch Uma?
          Uma Maheswara Rao G made changes -
          Assignee Uma Maheswara Rao G [ umamaheswararao ]
          Hide
          Uma Maheswara Rao G added a comment -

          @Stack, I will post the patch in some time today.

          Show
          Uma Maheswara Rao G added a comment - @Stack, I will post the patch in some time today.
          Uma Maheswara Rao G made changes -
          Attachment HBASE-3585.patch [ 12522987 ]
          Hide
          Uma Maheswara Rao G added a comment -

          Still the ctor wraps it to empty array only. SO, it can throw AIOE in isLegalFamilyName.

          Added the preconditions check for empty FamilyName. It will throw IllegalArgumentException if the familyName empty.

          Show
          Uma Maheswara Rao G added a comment - Still the ctor wraps it to empty array only. SO, it can throw AIOE in isLegalFamilyName. Added the preconditions check for empty FamilyName. It will throw IllegalArgumentException if the familyName empty.
          Uma Maheswara Rao G made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12522987/HBASE-3585.patch
          against trunk revision .

          +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 appears to introduce 4 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 unit tests:
          org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1552//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1552//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1552//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/12522987/HBASE-3585.patch against trunk revision . +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 appears to introduce 4 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 unit tests: org.apache.hadoop.hbase.regionserver.TestServerCustomProtocol Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1552//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1552//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1552//console This message is automatically generated.
          Hide
          Uma Maheswara Rao G added a comment -

          Test failure and findbugs are unrelated to this change.

          Show
          Uma Maheswara Rao G added a comment - Test failure and findbugs are unrelated to this change.
          Uma Maheswara Rao G made changes -
          Affects Version/s 0.96.0 [ 12320040 ]
          Hide
          stack added a comment -

          Committed to trunk. Thanks Uma.

          Show
          stack added a comment - Committed to trunk. Thanks Uma.
          stack made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Fix Version/s 0.96.0 [ 12320040 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #2783 (See https://builds.apache.org/job/HBase-TRUNK/2783/)
          HBASE-3585 isLegalFamilyName() can throw ArrayOutOfBoundException (Revision 1327666)

          Result = FAILURE
          stack :
          Files :

          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
          • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #2783 (See https://builds.apache.org/job/HBase-TRUNK/2783/ ) HBASE-3585 isLegalFamilyName() can throw ArrayOutOfBoundException (Revision 1327666) Result = FAILURE stack : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-security #175 (See https://builds.apache.org/job/HBase-TRUNK-security/175/)
          HBASE-3585 isLegalFamilyName() can throw ArrayOutOfBoundException (Revision 1327666)

          Result = FAILURE
          stack :
          Files :

          • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
          • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-security #175 (See https://builds.apache.org/job/HBase-TRUNK-security/175/ ) HBASE-3585 isLegalFamilyName() can throw ArrayOutOfBoundException (Revision 1327666) Result = FAILURE stack : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java
          stack made changes -
          Fix Version/s 0.95.0 [ 12324094 ]
          Fix Version/s 0.96.0 [ 12320040 ]
          stack made changes -
          Fix Version/s 0.98.0 [ 12323143 ]
          stack made changes -
          Fix Version/s 0.98.0 [ 12323143 ]
          Hide
          stack added a comment -

          Marking closed.

          Show
          stack added a comment - Marking closed.
          stack made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Sean Busbey made changes -
          Link This issue relates to HBASE-11712 [ HBASE-11712 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          412d 23h 25m 1 Uma Maheswara Rao G 17/Apr/12 17:45
          Patch Available Patch Available Resolved Resolved
          1d 2h 40m 1 stack 18/Apr/12 20:26
          Resolved Resolved Closed Closed
          522d 22h 4m 1 stack 23/Sep/13 18:30

            People

            • Assignee:
              Uma Maheswara Rao G
              Reporter:
              Prakash Khemani
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development