From 8cf7a9d51ee92444e5e2c730045d2289ea97e82a Mon Sep 17 00:00:00 2001 From: Mike Drob Date: Thu, 8 Feb 2018 16:12:46 -0600 Subject: [PATCH] HBASE-19920 Lazy init for ProtobufUtil classloader --- .../hbase/ipc/RemoteWithExtrasException.java | 23 ++++-- .../hadoop/hbase/protobuf/ProtobufUtil.java | 30 +++---- .../hbase/shaded/protobuf/ProtobufUtil.java | 35 +++++--- .../hbase/security/token/TokenUtil.java | 13 ++- .../hbase/security/token/TestTokenUtil.java | 80 +++++++++++++++++++ 5 files changed, 146 insertions(+), 35 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenUtil.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java index 1374ab0430f..3a7540ebeea 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RemoteWithExtrasException.java @@ -19,6 +19,8 @@ package org.apache.hadoop.hbase.ipc; import java.io.IOException; import java.lang.reflect.Constructor; +import java.security.AccessController; +import java.security.PrivilegedAction; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.DoNotRetryIOException; @@ -36,19 +38,24 @@ import org.apache.hadoop.ipc.RemoteException; */ @SuppressWarnings("serial") @InterfaceAudience.Public -@edu.umd.cs.findbugs.annotations.SuppressWarnings( - value = "DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification = "None. Address sometime.") public class RemoteWithExtrasException extends RemoteException { private final String hostname; private final int port; private final boolean doNotRetry; - private final static ClassLoader CLASS_LOADER; + /** + * Dynamic class loader to load filter/comparators + */ + private final static class ClassLoaderHolder { + private final static ClassLoader CLASS_LOADER; - static { - ClassLoader parent = RemoteWithExtrasException.class.getClassLoader(); - Configuration conf = HBaseConfiguration.create(); - CLASS_LOADER = new DynamicClassLoader(conf, parent); + static { + ClassLoader parent = RemoteWithExtrasException.class.getClassLoader(); + Configuration conf = HBaseConfiguration.create(); + CLASS_LOADER = AccessController.doPrivileged((PrivilegedAction) + () -> new DynamicClassLoader(conf, parent) + ); + } } public RemoteWithExtrasException(String className, String msg, final boolean doNotRetry) { @@ -69,7 +76,7 @@ public class RemoteWithExtrasException extends RemoteException { try { // try to load a exception class from where the HBase classes are loaded or from Dynamic // classloader. - realClass = Class.forName(getClassName(), false, CLASS_LOADER); + realClass = Class.forName(getClassName(), false, ClassLoaderHolder.CLASS_LOADER); } catch (ClassNotFoundException cnfe) { try { // cause could be a hadoop exception, try to load from hadoop classpath diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java index 29ff2a2a618..3c01fd66f8e 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java @@ -33,6 +33,8 @@ import com.google.protobuf.TextFormat; import java.io.IOException; import java.lang.reflect.Constructor; import java.lang.reflect.Method; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -114,8 +116,6 @@ import org.apache.yetus.audience.InterfaceAudience; * @see ProtobufUtil */ // TODO: Generate this class from the shaded version. -@edu.umd.cs.findbugs.annotations.SuppressWarnings( - value="DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification="None. Address sometime.") @InterfaceAudience.Private // TODO: some clients (Hive, etc) use this class. public final class ProtobufUtil { private ProtobufUtil() { @@ -169,14 +169,18 @@ public final class ProtobufUtil { } /** - * Dynamic class loader to load filter/comparators - */ - private final static ClassLoader CLASS_LOADER; + * Dynamic class loader to load filter/comparators + */ + private final static class ClassLoaderHolder { + private final static ClassLoader CLASS_LOADER; - static { - ClassLoader parent = ProtobufUtil.class.getClassLoader(); - Configuration conf = HBaseConfiguration.create(); - CLASS_LOADER = new DynamicClassLoader(conf, parent); + static { + ClassLoader parent = ProtobufUtil.class.getClassLoader(); + Configuration conf = HBaseConfiguration.create(); + CLASS_LOADER = AccessController.doPrivileged((PrivilegedAction) + () -> new DynamicClassLoader(conf, parent) + ); + } } /** @@ -1431,8 +1435,7 @@ public final class ProtobufUtil { String funcName = "parseFrom"; byte [] value = proto.getSerializedComparator().toByteArray(); try { - Class c = - (Class)Class.forName(type, true, CLASS_LOADER); + Class c = Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER); Method parseFrom = c.getMethod(funcName, byte[].class); if (parseFrom == null) { throw new IOException("Unable to locate function: " + funcName + " in type: " + type); @@ -1455,8 +1458,7 @@ public final class ProtobufUtil { final byte [] value = proto.getSerializedFilter().toByteArray(); String funcName = "parseFrom"; try { - Class c = - (Class)Class.forName(type, true, CLASS_LOADER); + Class c = Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER); Method parseFrom = c.getMethod(funcName, byte[].class); if (parseFrom == null) { throw new IOException("Unable to locate function: " + funcName + " in type: " + type); @@ -1542,7 +1544,7 @@ public final class ProtobufUtil { String type = parameter.getName(); try { Class c = - (Class)Class.forName(type, true, CLASS_LOADER); + (Class)Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER); Constructor cn = null; try { cn = c.getDeclaredConstructor(String.class); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java index 5bb3b4ba049..2ce8d7ba231 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java @@ -25,6 +25,8 @@ import java.io.InputStream; import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.nio.ByteBuffer; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -106,6 +108,7 @@ import org.apache.hadoop.hbase.util.VersionInfo; import org.apache.hadoop.ipc.RemoteException; import org.apache.yetus.audience.InterfaceAudience; +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hbase.thirdparty.com.google.common.io.ByteStreams; import org.apache.hbase.thirdparty.com.google.gson.JsonArray; import org.apache.hbase.thirdparty.com.google.gson.JsonElement; @@ -192,8 +195,6 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.ZooKeeperProtos; * @see ProtobufUtil */ // TODO: Generate the non-shaded protobufutil from this one. -@edu.umd.cs.findbugs.annotations.SuppressWarnings( - value="DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification="None. Address sometime.") @InterfaceAudience.Private // TODO: some clients (Hive, etc) use this class public final class ProtobufUtil { private ProtobufUtil() { @@ -245,15 +246,27 @@ public final class ProtobufUtil { EMPTY_RESULT_PB_STALE = builder.build(); } + private static volatile boolean classLoaderLoaded = false; + /** * Dynamic class loader to load filter/comparators */ - private final static ClassLoader CLASS_LOADER; + private final static class ClassLoaderHolder { + private final static ClassLoader CLASS_LOADER; - static { - ClassLoader parent = ProtobufUtil.class.getClassLoader(); - Configuration conf = HBaseConfiguration.create(); - CLASS_LOADER = new DynamicClassLoader(conf, parent); + static { + ClassLoader parent = ProtobufUtil.class.getClassLoader(); + Configuration conf = HBaseConfiguration.create(); + CLASS_LOADER = AccessController.doPrivileged((PrivilegedAction) + () -> new DynamicClassLoader(conf, parent) + ); + classLoaderLoaded = true; + } + } + + @VisibleForTesting + public static boolean isClassLoaderLoaded() { + return classLoaderLoaded; } /** @@ -1588,8 +1601,7 @@ public final class ProtobufUtil { String funcName = "parseFrom"; byte [] value = proto.getSerializedComparator().toByteArray(); try { - Class c = - (Class)Class.forName(type, true, CLASS_LOADER); + Class c = Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER); Method parseFrom = c.getMethod(funcName, byte[].class); if (parseFrom == null) { throw new IOException("Unable to locate function: " + funcName + " in type: " + type); @@ -1612,8 +1624,7 @@ public final class ProtobufUtil { final byte [] value = proto.getSerializedFilter().toByteArray(); String funcName = "parseFrom"; try { - Class c = - (Class)Class.forName(type, true, CLASS_LOADER); + Class c = Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER); Method parseFrom = c.getMethod(funcName, byte[].class); if (parseFrom == null) { throw new IOException("Unable to locate function: " + funcName + " in type: " + type); @@ -1699,7 +1710,7 @@ public final class ProtobufUtil { String type = parameter.getName(); try { Class c = - (Class)Class.forName(type, true, CLASS_LOADER); + (Class)Class.forName(type, true, ClassLoaderHolder.CLASS_LOADER); Constructor cn = null; try { cn = c.getDeclaredConstructor(String.class); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java index 54617601374..c54d9058844 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/token/TokenUtil.java @@ -26,7 +26,6 @@ import com.google.protobuf.ByteString; import com.google.protobuf.ServiceException; import org.apache.hadoop.hbase.zookeeper.ZKWatcher; -import org.apache.yetus.audience.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.TableName; @@ -41,6 +40,7 @@ import org.apache.hadoop.io.Text; import org.apache.hadoop.mapred.JobConf; import org.apache.hadoop.mapreduce.Job; import org.apache.hadoop.security.token.Token; +import org.apache.yetus.audience.InterfaceAudience; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,6 +53,15 @@ public class TokenUtil { // This class is referenced indirectly by User out in common; instances are created by reflection private static final Logger LOG = LoggerFactory.getLogger(TokenUtil.class); + // Set in TestTokenUtil via reflection + private static ServiceException injectedException; + + private static void injectFault() throws ServiceException { + if (injectedException != null) { + throw injectedException; + } + } + /** * Obtain and return an authentication token for the current user. * @param conn The HBase cluster connection @@ -63,6 +72,8 @@ public class TokenUtil { Connection conn) throws IOException { Table meta = null; try { + injectFault(); + meta = conn.getTable(TableName.META_TABLE_NAME); CoprocessorRpcChannel rpcChannel = meta.coprocessorService(HConstants.EMPTY_START_ROW); AuthenticationProtos.AuthenticationService.BlockingInterface service = diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenUtil.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenUtil.java new file mode 100644 index 00000000000..32fcddb85ff --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/token/TestTokenUtil.java @@ -0,0 +1,80 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.security.token; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.fail; + +import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.net.URL; +import java.net.URLClassLoader; + +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; + +@Category(SmallTests.class) +public class TestTokenUtil { + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestTokenUtil.class); + + @Test + public void testObtainToken() throws Exception { + URL urlPU = ProtobufUtil.class.getProtectionDomain().getCodeSource().getLocation(); + URL urlTU = TokenUtil.class.getProtectionDomain().getCodeSource().getLocation(); + + ClassLoader cl = new URLClassLoader(new URL[] { urlPU, urlTU }, getClass().getClassLoader()); + + Throwable injected = new com.google.protobuf.ServiceException("injected"); + + Class tokenUtil = cl.loadClass(TokenUtil.class.getCanonicalName()); + Field shouldInjectFault = tokenUtil.getDeclaredField("injectedException"); + shouldInjectFault.setAccessible(true); + shouldInjectFault.set(null, injected); + + try { + tokenUtil.getMethod("obtainToken", Connection.class) + .invoke(null, new Object[] { null }); + fail("Should have injected exception."); + } catch (InvocationTargetException e) { + Throwable t = e; + boolean serviceExceptionFound = false; + while ((t = t.getCause()) != null) { + if (t == injected) { // reference equality + serviceExceptionFound = true; + break; + } + } + if (!serviceExceptionFound) { + throw e; // wrong exception, fail the test + } + } + + Boolean loaded = (Boolean) cl.loadClass(ProtobufUtil.class.getCanonicalName()) + .getDeclaredMethod("isClassLoaderLoaded") + .invoke(null); + assertFalse("Should not have loaded DynamicClassLoader", loaded); + } +}