You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by hu...@apache.org on 2020/12/16 05:54:11 UTC

[hbase] branch master updated: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionName(byte[] regionName) (#2753)

This is an automated email from the ASF dual-hosted git repository.

huaxiangsun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/master by this push:
     new c327680  HBASE-25368 Filter out more invalid encoded name in isEncodedRegionName(byte[] regionName) (#2753)
c327680 is described below

commit c3276801256aa16a62e5cdba7a37d4e18d59e880
Author: huaxiangsun <hu...@apache.org>
AuthorDate: Tue Dec 15 21:52:54 2020 -0800

    HBASE-25368 Filter out more invalid encoded name in isEncodedRegionName(byte[] regionName) (#2753)
    
    Signed-off-by: stack <st...@apache.com>
---
 .../hadoop/hbase/client/RawAsyncHBaseAdmin.java    | 87 ++++++++++++----------
 .../org/apache/hadoop/hbase/client/RegionInfo.java | 18 ++++-
 .../org/apache/hadoop/hbase/client/TestAdmin1.java | 19 +++++
 3 files changed, 82 insertions(+), 42 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
index 512e7a9..7823963 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
@@ -2388,51 +2388,56 @@ class RawAsyncHBaseAdmin implements AsyncAdmin {
     if (regionNameOrEncodedRegionName == null) {
       return failedFuture(new IllegalArgumentException("Passed region name can't be null"));
     }
-    try {
-      CompletableFuture<Optional<HRegionLocation>> future;
-      if (RegionInfo.isEncodedRegionName(regionNameOrEncodedRegionName)) {
-        String encodedName = Bytes.toString(regionNameOrEncodedRegionName);
-        if (encodedName.length() < RegionInfo.MD5_HEX_LENGTH) {
-          // old format encodedName, should be meta region
-          future = connection.registry.getMetaRegionLocations()
-            .thenApply(locs -> Stream.of(locs.getRegionLocations())
-              .filter(loc -> loc.getRegion().getEncodedName().equals(encodedName)).findFirst());
-        } else {
-          future = ClientMetaTableAccessor.getRegionLocationWithEncodedName(metaTable,
-            regionNameOrEncodedRegionName);
-        }
+
+    CompletableFuture<Optional<HRegionLocation>> future;
+    if (RegionInfo.isEncodedRegionName(regionNameOrEncodedRegionName)) {
+      String encodedName = Bytes.toString(regionNameOrEncodedRegionName);
+      if (encodedName.length() < RegionInfo.MD5_HEX_LENGTH) {
+        // old format encodedName, should be meta region
+        future = connection.registry.getMetaRegionLocations()
+          .thenApply(locs -> Stream.of(locs.getRegionLocations())
+            .filter(loc -> loc.getRegion().getEncodedName().equals(encodedName)).findFirst());
       } else {
-        RegionInfo regionInfo =
-          CatalogFamilyFormat.parseRegionInfoFromRegionName(regionNameOrEncodedRegionName);
-        if (regionInfo.isMetaRegion()) {
-          future = connection.registry.getMetaRegionLocations()
-            .thenApply(locs -> Stream.of(locs.getRegionLocations())
-              .filter(loc -> loc.getRegion().getReplicaId() == regionInfo.getReplicaId())
-              .findFirst());
-        } else {
-          future =
-            ClientMetaTableAccessor.getRegionLocation(metaTable, regionNameOrEncodedRegionName);
-        }
+        future = ClientMetaTableAccessor.getRegionLocationWithEncodedName(metaTable,
+          regionNameOrEncodedRegionName);
+      }
+    } else {
+      // Not all regionNameOrEncodedRegionName here is going to be a valid region name,
+      // it needs to throw out IllegalArgumentException in case tableName is passed in.
+      RegionInfo regionInfo;
+      try {
+        regionInfo = CatalogFamilyFormat.parseRegionInfoFromRegionName(
+          regionNameOrEncodedRegionName);
+      } catch (IOException ioe) {
+        throw new IllegalArgumentException(ioe.getMessage());
       }
 
-      CompletableFuture<HRegionLocation> returnedFuture = new CompletableFuture<>();
-      addListener(future, (location, err) -> {
-        if (err != null) {
-          returnedFuture.completeExceptionally(err);
-          return;
-        }
-        if (!location.isPresent() || location.get().getRegion() == null) {
-          returnedFuture.completeExceptionally(
-            new UnknownRegionException("Invalid region name or encoded region name: " +
-              Bytes.toStringBinary(regionNameOrEncodedRegionName)));
-        } else {
-          returnedFuture.complete(location.get());
-        }
-      });
-      return returnedFuture;
-    } catch (IOException e) {
-      return failedFuture(e);
+      if (regionInfo.isMetaRegion()) {
+        future = connection.registry.getMetaRegionLocations()
+          .thenApply(locs -> Stream.of(locs.getRegionLocations())
+            .filter(loc -> loc.getRegion().getReplicaId() == regionInfo.getReplicaId())
+            .findFirst());
+      } else {
+        future =
+          ClientMetaTableAccessor.getRegionLocation(metaTable, regionNameOrEncodedRegionName);
+      }
     }
+
+    CompletableFuture<HRegionLocation> returnedFuture = new CompletableFuture<>();
+    addListener(future, (location, err) -> {
+      if (err != null) {
+        returnedFuture.completeExceptionally(err);
+        return;
+      }
+      if (!location.isPresent() || location.get().getRegion() == null) {
+        returnedFuture.completeExceptionally(
+          new UnknownRegionException("Invalid region name or encoded region name: " +
+            Bytes.toStringBinary(regionNameOrEncodedRegionName)));
+      } else {
+        returnedFuture.complete(location.get());
+      }
+    });
+    return returnedFuture;
   }
 
   /**
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
index d7460e9..b6bdd01 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
@@ -363,7 +363,23 @@ public interface RegionInfo extends Comparable<RegionInfo> {
   @InterfaceAudience.Private // For use by internals only.
   public static boolean isEncodedRegionName(byte[] regionName) {
     // If not parseable as region name, presume encoded. TODO: add stringency; e.g. if hex.
-    return parseRegionNameOrReturnNull(regionName) == null && regionName.length <= MD5_HEX_LENGTH;
+    if (parseRegionNameOrReturnNull(regionName) == null) {
+      if (regionName.length > MD5_HEX_LENGTH) {
+        return false;
+      } else if (regionName.length == MD5_HEX_LENGTH) {
+        return true;
+      } else {
+        String encodedName = Bytes.toString(regionName);
+        try {
+          Integer.parseInt(encodedName);
+          // If this is a valid integer, it could be hbase:meta's encoded region name.
+          return true;
+        } catch(NumberFormatException er) {
+          return false;
+        }
+      }
+    }
+    return false;
   }
 
   /**
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
index a0ed836..cfd61d2 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
@@ -100,6 +100,25 @@ public class TestAdmin1 extends TestAdminBase {
   }
 
   @Test
+  public void testCompactATableWithSuperLongTableName() throws Exception {
+    TableName tableName = TableName.valueOf(name.getMethodName());
+    TableDescriptor htd = TableDescriptorBuilder.newBuilder(tableName)
+      .setColumnFamily(ColumnFamilyDescriptorBuilder.of("fam1")).build();
+    try {
+      ADMIN.createTable(htd);
+      try {
+        ADMIN.majorCompactRegion(tableName.getName());
+        ADMIN.majorCompactRegion(Bytes.toBytes("abcd"));
+      } catch (IllegalArgumentException iae) {
+        LOG.info("This is expected");
+      }
+    } finally {
+      ADMIN.disableTable(tableName);
+      ADMIN.deleteTable(tableName);
+    }
+  }
+
+  @Test
   public void testCompactionTimestamps() throws Exception {
     TableName tableName = TableName.valueOf(name.getMethodName());
     TableDescriptor htd = TableDescriptorBuilder.newBuilder(tableName)