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

          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.
          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
          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?
          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.
          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.
          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.
          Hide
          stack added a comment -

          Committed to trunk. Thanks Uma.

          Show
          stack added a comment - Committed to trunk. Thanks Uma.
          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
          Hide
          stack added a comment -

          Marking closed.

          Show
          stack added a comment - Marking closed.

            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