From c61f28bc5e0cc20583f0da83b9510fdf419f0ae8 Mon Sep 17 00:00:00 2001 From: Mike Drob Date: Mon, 12 Mar 2018 16:05:39 -0500 Subject: [PATCH] HBASE-20180 Avoid Class::newInstance --- .../hadoop/hbase/backup/BackupClientFactory.java | 4 +-- hbase-build-configuration/pom.xml | 1 + .../hbase/coprocessor/AggregateImplementation.java | 12 ++++----- .../hadoop/hbase/constraint/Constraints.java | 10 +++----- .../hadoop/hbase/coprocessor/CoprocessorHost.java | 3 +++ .../hadoop/hbase/master/MasterCoprocessorHost.java | 30 ++++++++++++++-------- .../master/replication/ReplicationPeerManager.java | 4 +-- .../hadoop/hbase/regionserver/RSRpcServices.java | 12 +++++---- .../hbase/regionserver/RegionCoprocessorHost.java | 30 ++++++++++++++-------- .../regionserver/RegionServerCoprocessorHost.java | 30 ++++++++++++++-------- .../hbase/regionserver/wal/WALCoprocessorHost.java | 14 +++++++--- .../regionserver/ReplicationSource.java | 11 ++++++-- .../regionserver/ReplicationSourceFactory.java | 5 ++-- .../hbase/coprocessor/TestCoprocessorHost.java | 8 +++++- 14 files changed, 109 insertions(+), 65 deletions(-) diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupClientFactory.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupClientFactory.java index 4c962290e8..1a2228b704 100644 --- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupClientFactory.java +++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupClientFactory.java @@ -38,8 +38,8 @@ public final class BackupClientFactory { try { String clsName = conf.get(TableBackupClient.BACKUP_CLIENT_IMPL_CLASS); if (clsName != null) { - Class clientImpl = Class.forName(clsName); - TableBackupClient client = (TableBackupClient) clientImpl.newInstance(); + Class clientImpl = Class.forName(clsName).asSubclass(TableBackupClient.class); + TableBackupClient client = clientImpl.getDeclaredConstructor().newInstance(); client.init(conn, backupId, request); return client; } diff --git a/hbase-build-configuration/pom.xml b/hbase-build-configuration/pom.xml index ac98044335..311b1c0394 100644 --- a/hbase-build-configuration/pom.xml +++ b/hbase-build-configuration/pom.xml @@ -82,6 +82,7 @@ -XepDisableWarningsInGeneratedCode -Xep:FallThrough:OFF + -Xep:ClassNewInstance:ERROR diff --git a/hbase-endpoint/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateImplementation.java b/hbase-endpoint/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateImplementation.java index 6beb3f66fd..3d8a264446 100644 --- a/hbase-endpoint/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateImplementation.java +++ b/hbase-endpoint/src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateImplementation.java @@ -27,6 +27,7 @@ import com.google.protobuf.RpcController; import com.google.protobuf.Service; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; @@ -482,19 +483,16 @@ extends AggregateService implements RegionCoprocessor { String className = request.getInterpreterClassName(); Class cls; try { - cls = Class.forName(className); - ColumnInterpreter ci = (ColumnInterpreter) cls.newInstance(); + cls = Class.forName(className).asSubclass(ColumnInterpreter.class); + ColumnInterpreter ci = (ColumnInterpreter) cls.getDeclaredConstructor().newInstance(); if (request.hasInterpreterSpecificBytes()) { ByteString b = request.getInterpreterSpecificBytes(); P initMsg = getParsedGenericInstance(ci.getClass(), 2, b); ci.initialize(initMsg); } return ci; - } catch (ClassNotFoundException e) { - throw new IOException(e); - } catch (InstantiationException e) { - throw new IOException(e); - } catch (IllegalAccessException e) { + } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | + NoSuchMethodException | InvocationTargetException e) { throw new IOException(e); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java index 426e516153..759d2520b0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/Constraints.java @@ -21,6 +21,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.DataOutputStream; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -593,14 +594,11 @@ public final class Constraints { // add the constraint, now that we expect it to be valid. Class clazz = classloader.loadClass(key) .asSubclass(Constraint.class); - Constraint constraint = clazz.newInstance(); + Constraint constraint = clazz.getDeclaredConstructor().newInstance(); constraint.setConf(conf); constraints.add(constraint); - } catch (ClassNotFoundException e1) { - throw new IOException(e1); - } catch (InstantiationException e1) { - throw new IOException(e1); - } catch (IllegalAccessException e1) { + } catch (InvocationTargetException | NoSuchMethodException | ClassNotFoundException | + InstantiationException | IllegalAccessException e1) { throw new IOException(e1); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java index 4c5605650d..d8b2432fa9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.coprocessor; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -669,4 +670,6 @@ public abstract class CoprocessorHost implClass) throws InstantiationException, IllegalAccessException { - if (MasterCoprocessor.class.isAssignableFrom(implClass)) { - return (MasterCoprocessor)implClass.newInstance(); - } else if (CoprocessorService.class.isAssignableFrom(implClass)) { - // For backward compatibility with old CoprocessorService impl which don't extend - // MasterCoprocessor. - return new CoprocessorServiceBackwardCompatiblity.MasterCoprocessorService( - (CoprocessorService)implClass.newInstance()); - } else { - LOG.error(implClass.getName() + " is not of type MasterCoprocessor. Check the " - + "configuration " + CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY); - return null; + try { + if (MasterCoprocessor.class.isAssignableFrom(implClass)) { + return implClass.asSubclass(MasterCoprocessor.class).getDeclaredConstructor().newInstance(); + } else if (CoprocessorService.class.isAssignableFrom(implClass)) { + // For backward compatibility with old CoprocessorService impl which don't extend + // MasterCoprocessor. + CoprocessorService tmp = implClass.asSubclass(CoprocessorService.class).getDeclaredConstructor().newInstance(); + return new CoprocessorServiceBackwardCompatiblity.MasterCoprocessorService(tmp); + } else { + LOG.error("{} is not of type MasterCoprocessor. Check the configuration of {}", + implClass.getName(), CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY); + } + } catch (NoSuchMethodException e) { + throw (InstantiationException) new InstantiationException(implClass.getName()).initCause(e); + } catch (InvocationTargetException e) { + LOG.error("Exception while instantiating " + implClass.getName(), e); } + return null; } private ObserverGetter masterObserverGetter = diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java index 19fc7f40e5..19cd89dcc3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/replication/ReplicationPeerManager.java @@ -310,10 +310,10 @@ public class ReplicationPeerManager { String[] filters = filterCSV.split(","); for (String filter : filters) { try { - Class.forName(filter).newInstance(); + Class.forName(filter).getDeclaredConstructor().newInstance(); } catch (Exception e) { throw new DoNotRetryIOException("Configured WALEntryFilter " + filter + - " could not be created. Failing add/update " + "peer operation.", e); + " could not be created. Failing add/update peer operation.", e); } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 803d3e8b3c..f02864024f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.regionserver; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InterruptedIOException; +import java.lang.reflect.InvocationTargetException; import java.net.BindException; import java.net.InetSocketAddress; import java.net.UnknownHostException; @@ -90,6 +91,7 @@ import org.apache.hadoop.hbase.ipc.PriorityFunction; import org.apache.hadoop.hbase.ipc.QosPriority; import org.apache.hadoop.hbase.ipc.RpcCallContext; import org.apache.hadoop.hbase.ipc.RpcCallback; +import org.apache.hadoop.hbase.ipc.RpcScheduler; import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.ipc.RpcServer.BlockingServiceAndInterface; import org.apache.hadoop.hbase.ipc.RpcServerFactory; @@ -1179,11 +1181,11 @@ public class RSRpcServices implements HBaseRPCErrorHandler, rowSizeWarnThreshold = rs.conf.getInt(BATCH_ROWS_THRESHOLD_NAME, BATCH_ROWS_THRESHOLD_DEFAULT); RpcSchedulerFactory rpcSchedulerFactory; try { - Class rpcSchedulerFactoryClass = rs.conf.getClass( + Class cls = rs.conf.getClass( REGION_SERVER_RPC_SCHEDULER_FACTORY_CLASS, SimpleRpcSchedulerFactory.class); - rpcSchedulerFactory = ((RpcSchedulerFactory) rpcSchedulerFactoryClass.newInstance()); - } catch (InstantiationException | IllegalAccessException e) { + rpcSchedulerFactory = cls.asSubclass(RpcSchedulerFactory.class).getDeclaredConstructor().newInstance(); + } catch (NoSuchMethodException | InvocationTargetException | InstantiationException | IllegalAccessException e) { throw new IllegalArgumentException(e); } // Server to handle client requests. @@ -3543,8 +3545,8 @@ public class RSRpcServices implements HBaseRPCErrorHandler, for (RemoteProcedureRequest req : request.getProcList()) { RSProcedureCallable callable; try { - callable = - Class.forName(req.getProcClass()).asSubclass(RSProcedureCallable.class).newInstance(); + callable = Class.forName(req.getProcClass()).asSubclass(RSProcedureCallable.class) + .getDeclaredConstructor().newInstance(); } catch (Exception e) { regionServer.remoteProcedureComplete(req.getProcId(), e); continue; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java index 47b389af11..f7a703492b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java @@ -22,6 +22,8 @@ import com.google.protobuf.Message; import com.google.protobuf.Service; import java.io.IOException; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -465,18 +467,24 @@ public class RegionCoprocessorHost @Override public RegionCoprocessor checkAndGetInstance(Class implClass) throws InstantiationException, IllegalAccessException { - if (RegionCoprocessor.class.isAssignableFrom(implClass)) { - return (RegionCoprocessor)implClass.newInstance(); - } else if (CoprocessorService.class.isAssignableFrom(implClass)) { - // For backward compatibility with old CoprocessorService impl which don't extend - // RegionCoprocessor. - return new CoprocessorServiceBackwardCompatiblity.RegionCoprocessorService( - (CoprocessorService)implClass.newInstance()); - } else { - LOG.error(implClass.getName() + " is not of type RegionCoprocessor. Check the " - + "configuration " + CoprocessorHost.REGION_COPROCESSOR_CONF_KEY); - return null; + try { + if (RegionCoprocessor.class.isAssignableFrom(implClass)) { + return implClass.asSubclass(RegionCoprocessor.class).getDeclaredConstructor().newInstance(); + } else if (CoprocessorService.class.isAssignableFrom(implClass)) { + // For backward compatibility with old CoprocessorService impl which don't extend + // RegionCoprocessor. + CoprocessorService tmp = implClass.asSubclass(CoprocessorService.class).getDeclaredConstructor().newInstance(); + return new CoprocessorServiceBackwardCompatiblity.RegionCoprocessorService(tmp); + } else { + LOG.error("{} is not of type RegionCoprocessor. Check the configuration of {}", + implClass.getName(), CoprocessorHost.REGION_COPROCESSOR_CONF_KEY); + } + } catch (NoSuchMethodException e) { + throw (InstantiationException) new InstantiationException(implClass.getName()).initCause(e); + } catch (InvocationTargetException e) { + LOG.error("Exception while instantiating " + implClass.getName(), e); } + return null; } private ObserverGetter regionObserverGetter = diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java index f4122cef2b..ee244053d5 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.regionserver; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import com.google.protobuf.Service; @@ -82,18 +83,25 @@ public class RegionServerCoprocessorHost extends @Override public RegionServerCoprocessor checkAndGetInstance(Class implClass) throws InstantiationException, IllegalAccessException { - if (RegionServerCoprocessor.class.isAssignableFrom(implClass)) { - return (RegionServerCoprocessor)implClass.newInstance(); - } else if (SingletonCoprocessorService.class.isAssignableFrom(implClass)) { - // For backward compatibility with old CoprocessorService impl which don't extend - // RegionCoprocessor. - return new CoprocessorServiceBackwardCompatiblity.RegionServerCoprocessorService( - (SingletonCoprocessorService)implClass.newInstance()); - } else { - LOG.error(implClass.getName() + " is not of type RegionServerCoprocessor. Check the " - + "configuration " + CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY); - return null; + try { + if (RegionServerCoprocessor.class.isAssignableFrom(implClass)) { + return implClass.asSubclass(RegionServerCoprocessor.class).getDeclaredConstructor().newInstance(); + } else if (SingletonCoprocessorService.class.isAssignableFrom(implClass)) { + // For backward compatibility with old CoprocessorService impl which don't extend + // RegionCoprocessor. + SingletonCoprocessorService tmp = implClass.asSubclass(SingletonCoprocessorService.class) + .getDeclaredConstructor().newInstance(); + return new CoprocessorServiceBackwardCompatiblity.RegionServerCoprocessorService(tmp); + } else { + LOG.error("{} is not of type RegionServerCoprocessor. Check the configuration of {}", + implClass.getName(), CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY); + } + } catch (NoSuchMethodException e) { + throw (InstantiationException) new InstantiationException(implClass.getName()).initCause(e); + } catch (InvocationTargetException e) { + LOG.error("Exception while instantiating " + implClass.getName(), e); } + return null; } private ObserverGetter rsObserverGetter = diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java index 7b6182e6b6..05231185e6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java @@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.regionserver.wal; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; @@ -118,15 +119,20 @@ public class WALCoprocessorHost } @Override - public WALCoprocessor checkAndGetInstance(Class implClass) - throws InstantiationException, IllegalAccessException { + public WALCoprocessor checkAndGetInstance(Class implClass) throws IllegalAccessException, InstantiationException { if (WALCoprocessor.class.isAssignableFrom(implClass)) { - return (WALCoprocessor)implClass.newInstance(); + try { + return implClass.asSubclass(WALCoprocessor.class).getDeclaredConstructor().newInstance(); + } catch (NoSuchMethodException e) { + throw (InstantiationException) new InstantiationException(implClass.getName()).initCause(e); + } catch (InvocationTargetException e) { + LOG.error("Exception while instantiating " + implClass.getName(), e); + } } else { LOG.error(implClass.getName() + " is not of type WALCoprocessor. Check the " + "configuration " + CoprocessorHost.WAL_COPROCESSOR_CONF_KEY); - return null; } + return null; } private ObserverGetter walObserverGetter = diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java index 80d08ef66f..52998b826a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.replication.regionserver; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collection; import java.util.Comparator; @@ -246,8 +247,14 @@ public class ReplicationSource implements ReplicationSourceInterface { // Default to HBase inter-cluster replication endpoint replicationEndpointImpl = HBaseInterClusterReplicationEndpoint.class.getName(); } - ReplicationEndpoint replicationEndpoint = - Class.forName(replicationEndpointImpl).asSubclass(ReplicationEndpoint.class).newInstance(); + try { + ReplicationEndpoint replicationEndpoint = Class.forName(replicationEndpointImpl) + .asSubclass(ReplicationEndpoint.class) + .getDeclaredConstructor() + .newInstance(); + } catch (NoSuchMethodException | InvocationTargetException e) { + throw new IllegalArgumentException(e); + } if (rsServerHost != null) { ReplicationEndpoint newReplicationEndPoint = rsServerHost.postCreateReplicationEndPoint(replicationEndpoint); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceFactory.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceFactory.java index 93e83318d7..d613049d38 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceFactory.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceFactory.java @@ -40,10 +40,9 @@ public class ReplicationSourceFactory { String defaultReplicationSourceImpl = isQueueRecovered ? RecoveredReplicationSource.class.getCanonicalName() : ReplicationSource.class.getCanonicalName(); - @SuppressWarnings("rawtypes") - Class c = Class.forName( + Class c = Class.forName( conf.get("replication.replicationsource.implementation", defaultReplicationSourceImpl)); - src = (ReplicationSourceInterface) c.newInstance(); + src = c.asSubclass(ReplicationSourceInterface.class).getDeclaredConstructor().newInstance(); } catch (Exception e) { LOG.warn("Passed replication source implementation throws errors, " + "defaulting to ReplicationSource", diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorHost.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorHost.java index c541647e19..e0c289d4e1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorHost.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorHost.java @@ -32,6 +32,8 @@ import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; +import java.lang.reflect.InvocationTargetException; + @Category({SmallTests.class}) public class TestCoprocessorHost { @@ -66,7 +68,11 @@ public class TestCoprocessorHost { public RegionCoprocessor checkAndGetInstance(Class implClass) throws InstantiationException, IllegalAccessException { if(RegionCoprocessor.class.isAssignableFrom(implClass)) { - return (RegionCoprocessor)implClass.newInstance(); + try { + return implClass.asSubclass(RegionCoprocessor.class).getDeclaredConstructor().newInstance(); + } catch (InvocationTargetException | NoSuchMethodException e) { + throw (InstantiationException) new InstantiationException().initCause(e); + } } return null; } -- 2.16.1