Skip to content

Commit 5c13c68

Browse files
huaxiangsunhuaxiangsun
authored
HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa… (#2868)
HBASE-25368 Filter out more invalid encoded name in isEncodedRegionName(byte[] regionName) Signed-off-by: Duo Zhang <[email protected]>
1 parent a37e727 commit 5c13c68

File tree

4 files changed

+85
-47
lines changed

4 files changed

+85
-47
lines changed

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java‎

Lines changed: 46 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2388,51 +2388,56 @@ CompletableFuture<HRegionLocation> getRegionLocation(byte[] regionNameOrEncodedR
23882388
if (regionNameOrEncodedRegionName == null) {
23892389
return failedFuture(new IllegalArgumentException("Passed region name can't be null"));
23902390
}
2391-
try {
2392-
CompletableFuture<Optional<HRegionLocation>> future;
2393-
if (RegionInfo.isEncodedRegionName(regionNameOrEncodedRegionName)) {
2394-
String encodedName = Bytes.toString(regionNameOrEncodedRegionName);
2395-
if (encodedName.length() < RegionInfo.MD5_HEX_LENGTH) {
2396-
// old format encodedName, should be meta region
2397-
future = connection.registry.getMetaRegionLocations()
2398-
.thenApply(locs -> Stream.of(locs.getRegionLocations())
2399-
.filter(loc -> loc.getRegion().getEncodedName().equals(encodedName)).findFirst());
2400-
} else {
2401-
future = ClientMetaTableAccessor.getRegionLocationWithEncodedName(metaTable,
2402-
regionNameOrEncodedRegionName);
2403-
}
2391+
2392+
CompletableFuture<Optional<HRegionLocation>> future;
2393+
if (RegionInfo.isEncodedRegionName(regionNameOrEncodedRegionName)) {
2394+
String encodedName = Bytes.toString(regionNameOrEncodedRegionName);
2395+
if (encodedName.length() < RegionInfo.MD5_HEX_LENGTH) {
2396+
// old format encodedName, should be meta region
2397+
future = connection.registry.getMetaRegionLocations()
2398+
.thenApply(locs -> Stream.of(locs.getRegionLocations())
2399+
.filter(loc -> loc.getRegion().getEncodedName().equals(encodedName)).findFirst());
24042400
} else {
2405-
RegionInfo regionInfo =
2406-
CatalogFamilyFormat.parseRegionInfoFromRegionName(regionNameOrEncodedRegionName);
2407-
if (regionInfo.isMetaRegion()) {
2408-
future = connection.registry.getMetaRegionLocations()
2409-
.thenApply(locs -> Stream.of(locs.getRegionLocations())
2410-
.filter(loc -> loc.getRegion().getReplicaId() == regionInfo.getReplicaId())
2411-
.findFirst());
2412-
} else {
2413-
future =
2414-
ClientMetaTableAccessor.getRegionLocation(metaTable, regionNameOrEncodedRegionName);
2415-
}
2401+
future = ClientMetaTableAccessor.getRegionLocationWithEncodedName(metaTable,
2402+
regionNameOrEncodedRegionName);
2403+
}
2404+
} else {
2405+
// Not all regionNameOrEncodedRegionName here is going to be a valid region name,
2406+
// it needs to throw out IllegalArgumentException in case tableName is passed in.
2407+
RegionInfo regionInfo;
2408+
try {
2409+
regionInfo = CatalogFamilyFormat.parseRegionInfoFromRegionName(
2410+
regionNameOrEncodedRegionName);
2411+
} catch (IOException ioe) {
2412+
return failedFuture(new IllegalArgumentException(ioe.getMessage()));
24162413
}
24172414

2418-
CompletableFuture<HRegionLocation> returnedFuture = new CompletableFuture<>();
2419-
addListener(future, (location, err) -> {
2420-
if (err != null) {
2421-
returnedFuture.completeExceptionally(err);
2422-
return;
2423-
}
2424-
if (!location.isPresent() || location.get().getRegion() == null) {
2425-
returnedFuture.completeExceptionally(
2426-
new UnknownRegionException("Invalid region name or encoded region name: " +
2427-
Bytes.toStringBinary(regionNameOrEncodedRegionName)));
2428-
} else {
2429-
returnedFuture.complete(location.get());
2430-
}
2431-
});
2432-
return returnedFuture;
2433-
} catch (IOException e) {
2434-
return failedFuture(e);
2415+
if (regionInfo.isMetaRegion()) {
2416+
future = connection.registry.getMetaRegionLocations()
2417+
.thenApply(locs -> Stream.of(locs.getRegionLocations())
2418+
.filter(loc -> loc.getRegion().getReplicaId() == regionInfo.getReplicaId())
2419+
.findFirst());
2420+
} else {
2421+
future =
2422+
ClientMetaTableAccessor.getRegionLocation(metaTable, regionNameOrEncodedRegionName);
2423+
}
24352424
}
2425+
2426+
CompletableFuture<HRegionLocation> returnedFuture = new CompletableFuture<>();
2427+
addListener(future, (location, err) -> {
2428+
if (err != null) {
2429+
returnedFuture.completeExceptionally(err);
2430+
return;
2431+
}
2432+
if (!location.isPresent() || location.get().getRegion() == null) {
2433+
returnedFuture.completeExceptionally(
2434+
new UnknownRegionException("Invalid region name or encoded region name: " +
2435+
Bytes.toStringBinary(regionNameOrEncodedRegionName)));
2436+
} else {
2437+
returnedFuture.complete(location.get());
2438+
}
2439+
});
2440+
return returnedFuture;
24362441
}
24372442

24382443
/**

‎hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,23 @@ static byte[] getStartKey(final byte[] regionName) throws IOException {
363363
@InterfaceAudience.Private // For use by internals only.
364364
public static boolean isEncodedRegionName(byte[] regionName) {
365365
// If not parseable as region name, presume encoded. TODO: add stringency; e.g. if hex.
366-
return parseRegionNameOrReturnNull(regionName) == null && regionName.length <= MD5_HEX_LENGTH;
366+
if (parseRegionNameOrReturnNull(regionName) == null) {
367+
if (regionName.length > MD5_HEX_LENGTH) {
368+
return false;
369+
} else if (regionName.length == MD5_HEX_LENGTH) {
370+
return true;
371+
} else {
372+
String encodedName = Bytes.toString(regionName);
373+
try {
374+
Integer.parseInt(encodedName);
375+
// If this is a valid integer, it could be hbase:meta's encoded region name.
376+
return true;
377+
} catch(NumberFormatException er) {
378+
return false;
379+
}
380+
}
381+
}
382+
return false;
367383
}
368384

369385
/**

‎hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.junit.Assert.assertEquals;
2121
import static org.junit.Assert.assertFalse;
2222
import static org.junit.Assert.assertNotNull;
23+
import static org.junit.Assert.assertThrows;
2324
import static org.junit.Assert.assertTrue;
2425
import static org.junit.Assert.fail;
2526

@@ -99,6 +100,24 @@ public void testSplitFlushCompactUnknownTable() throws InterruptedException {
99100
assertTrue(exception instanceof TableNotFoundException);
100101
}
101102

103+
@Test
104+
public void testCompactATableWithSuperLongTableName() throws Exception {
105+
TableName tableName = TableName.valueOf(name.getMethodName());
106+
TableDescriptor htd = TableDescriptorBuilder.newBuilder(tableName)
107+
.setColumnFamily(ColumnFamilyDescriptorBuilder.of("fam1")).build();
108+
try {
109+
ADMIN.createTable(htd);
110+
assertThrows(IllegalArgumentException.class,
111+
() -> ADMIN.majorCompactRegion(tableName.getName()));
112+
113+
assertThrows(IllegalArgumentException.class,
114+
() -> ADMIN.majorCompactRegion(Bytes.toBytes("abcd")));
115+
} finally {
116+
ADMIN.disableTable(tableName);
117+
ADMIN.deleteTable(tableName);
118+
}
119+
}
120+
102121
@Test
103122
public void testCompactionTimestamps() throws Exception {
104123
TableName tableName = TableName.valueOf(name.getMethodName());

‎hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -298,11 +298,9 @@ public void testCloseRegionIfInvalidRegionNameIsPassed() throws Exception {
298298
if (!regionInfo.isMetaRegion()) {
299299
if (regionInfo.getRegionNameAsString().contains(name)) {
300300
info = regionInfo;
301-
try {
302-
ADMIN.unassign(Bytes.toBytes("sample"), true);
303-
} catch (UnknownRegionException nsre) {
304-
// expected, ignore it
305-
}
301+
assertThrows(UnknownRegionException.class,
302+
() -> ADMIN.unassign(Bytes.toBytes(
303+
"test,,1358563771069.acc1ad1b7962564fc3a43e5907e8db33."), true));
306304
}
307305
}
308306
}

0 commit comments

Comments
 (0)