From d325919f628b4279dab95690d0f5d7611539948c Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 19 Jul 2017 10:15:03 -0700 Subject: [PATCH] core: Use Class.forName(String) in provider for Android Class.forName(String) is understood by ProGuard, removing the need for manual ProGuard configuration and allows ProGuard to rename the provider classes. Previously the provider classes could not be renamed. Fixes #2633 --- .../app/proguard-rules.pro | 2 - .../java/io/grpc/ManagedChannelProvider.java | 12 ++- .../java/io/grpc/NameResolverProvider.java | 10 +- .../io/grpc/ManagedChannelProviderTest.java | 58 ++++++++--- .../io/grpc/NameResolverProviderTest.java | 99 ++++++++++++++++--- .../io/grpc/StaticTestingClassLoader.java | 76 ++++++++++++++ .../internal/DnsNameResolverProviderTest.java | 3 +- .../android/helloworld/app/proguard-rules.pro | 2 - .../android/routeguide/app/proguard-rules.pro | 2 - .../grpc/netty/NettyChannelProviderTest.java | 3 +- .../okhttp/OkHttpChannelProviderTest.java | 3 +- 11 files changed, 221 insertions(+), 49 deletions(-) create mode 100644 core/src/test/java/io/grpc/StaticTestingClassLoader.java diff --git a/android-interop-testing/app/proguard-rules.pro b/android-interop-testing/app/proguard-rules.pro index 46fb601f9a..e02d143aae 100644 --- a/android-interop-testing/app/proguard-rules.pro +++ b/android-interop-testing/app/proguard-rules.pro @@ -18,5 +18,3 @@ -dontwarn sun.reflect.** # Ignores: can't find referenced class javax.lang.model.element.Modifier -dontwarn com.google.errorprone.annotations.** --keep class io.grpc.internal.DnsNameResolverProvider --keep class io.grpc.okhttp.OkHttpChannelProvider diff --git a/core/src/main/java/io/grpc/ManagedChannelProvider.java b/core/src/main/java/io/grpc/ManagedChannelProvider.java index a9e47bd883..f7056bf8f4 100644 --- a/core/src/main/java/io/grpc/ManagedChannelProvider.java +++ b/core/src/main/java/io/grpc/ManagedChannelProvider.java @@ -40,7 +40,7 @@ public abstract class ManagedChannelProvider { static ManagedChannelProvider load(ClassLoader classLoader) { Iterable candidates; if (isAndroid()) { - candidates = getCandidatesViaHardCoded(classLoader); + candidates = getCandidatesViaHardCoded(); } else { candidates = getCandidatesViaServiceLoader(classLoader); } @@ -79,16 +79,18 @@ public abstract class ManagedChannelProvider { * be used on Android is free to be added here. */ @VisibleForTesting - public static Iterable getCandidatesViaHardCoded( - ClassLoader classLoader) { + public static Iterable getCandidatesViaHardCoded() { + // Class.forName(String) is used to remove the need for ProGuard configuration. Note that + // ProGuard does not detect usages of Class.forName(String, boolean, ClassLoader): + // https://sourceforge.net/p/proguard/bugs/418/ List list = new ArrayList(); try { - list.add(create(Class.forName("io.grpc.okhttp.OkHttpChannelProvider", true, classLoader))); + list.add(create(Class.forName("io.grpc.okhttp.OkHttpChannelProvider"))); } catch (ClassNotFoundException ex) { // ignore } try { - list.add(create(Class.forName("io.grpc.netty.NettyChannelProvider", true, classLoader))); + list.add(create(Class.forName("io.grpc.netty.NettyChannelProvider"))); } catch (ClassNotFoundException ex) { // ignore } diff --git a/core/src/main/java/io/grpc/NameResolverProvider.java b/core/src/main/java/io/grpc/NameResolverProvider.java index 66a94200e0..e90cc04e09 100644 --- a/core/src/main/java/io/grpc/NameResolverProvider.java +++ b/core/src/main/java/io/grpc/NameResolverProvider.java @@ -50,7 +50,7 @@ public abstract class NameResolverProvider extends NameResolver.Factory { static List load(ClassLoader classLoader) { Iterable candidates; if (isAndroid()) { - candidates = getCandidatesViaHardCoded(classLoader); + candidates = getCandidatesViaHardCoded(); } else { candidates = getCandidatesViaServiceLoader(classLoader); } @@ -83,11 +83,13 @@ public abstract class NameResolverProvider extends NameResolver.Factory { * be used on Android is free to be added here. */ @VisibleForTesting - public static Iterable getCandidatesViaHardCoded(ClassLoader classLoader) { + public static Iterable getCandidatesViaHardCoded() { + // Class.forName(String) is used to remove the need for ProGuard configuration. Note that + // ProGuard does not detect usages of Class.forName(String, boolean, ClassLoader): + // https://sourceforge.net/p/proguard/bugs/418/ List list = new ArrayList(); try { - list.add(create( - Class.forName("io.grpc.internal.DnsNameResolverProvider", true, classLoader))); + list.add(create(Class.forName("io.grpc.internal.DnsNameResolverProvider"))); } catch (ClassNotFoundException ex) { // ignore } diff --git a/core/src/test/java/io/grpc/ManagedChannelProviderTest.java b/core/src/test/java/io/grpc/ManagedChannelProviderTest.java index 77e771d52c..a759b8f3a1 100644 --- a/core/src/test/java/io/grpc/ManagedChannelProviderTest.java +++ b/core/src/test/java/io/grpc/ManagedChannelProviderTest.java @@ -22,7 +22,10 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.ServiceConfigurationError; +import java.util.regex.Pattern; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -52,15 +55,22 @@ public class ManagedChannelProviderTest { } @Test - public void getCandidatesViaHardCoded_usesProvidedClassLoader() { + public void getCandidatesViaHardCoded_triesToLoadClasses() throws Exception { + ClassLoader cl = getClass().getClassLoader(); final RuntimeException toThrow = new RuntimeException(); - try { - ManagedChannelProvider.getCandidatesViaHardCoded(new ClassLoader() { - @Override - public Class loadClass(String name) { + cl = new ClassLoader(cl) { + @Override + public Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (name.startsWith("io.grpc.netty.") || name.startsWith("io.grpc.okhttp.")) { throw toThrow; + } else { + return super.loadClass(name, resolve); } - }); + } + }; + cl = new StaticTestingClassLoader(cl, Pattern.compile("io\\.grpc\\.[^.]*")); + try { + invokeGetCandidatesViaHardCoded(cl); fail("Expected exception"); } catch (RuntimeException ex) { assertSame(toThrow, ex); @@ -68,14 +78,20 @@ public class ManagedChannelProviderTest { } @Test - public void getCandidatesViaHardCoded_ignoresMissingClasses() { - Iterable i = - ManagedChannelProvider.getCandidatesViaHardCoded(new ClassLoader() { - @Override - public Class loadClass(String name) throws ClassNotFoundException { - throw new ClassNotFoundException(); - } - }); + public void getCandidatesViaHardCoded_ignoresMissingClasses() throws Exception { + ClassLoader cl = getClass().getClassLoader(); + cl = new ClassLoader(cl) { + @Override + public Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (name.startsWith("io.grpc.netty.") || name.startsWith("io.grpc.okhttp.")) { + throw new ClassNotFoundException(); + } else { + return super.loadClass(name, resolve); + } + } + }; + cl = new StaticTestingClassLoader(cl, Pattern.compile("io\\.grpc\\.[^.]*")); + Iterable i = invokeGetCandidatesViaHardCoded(cl); assertFalse("Iterator should be empty", i.iterator().hasNext()); } @@ -92,6 +108,20 @@ public class ManagedChannelProviderTest { } } + private static Iterable invokeGetCandidatesViaHardCoded(ClassLoader cl) throws Exception { + // An error before the invoke likely means there is a bug in the test + Class klass = Class.forName(ManagedChannelProvider.class.getName(), true, cl); + Method getCandidatesViaHardCoded = klass.getMethod("getCandidatesViaHardCoded"); + try { + return (Iterable) getCandidatesViaHardCoded.invoke(null); + } catch (InvocationTargetException ex) { + if (ex.getCause() instanceof Exception) { + throw (Exception) ex.getCause(); + } + throw ex; + } + } + private static class BaseProvider extends ManagedChannelProvider { private final boolean isAvailable; private final int priority; diff --git a/core/src/test/java/io/grpc/NameResolverProviderTest.java b/core/src/test/java/io/grpc/NameResolverProviderTest.java index 7d192a04a7..e8173d29fe 100644 --- a/core/src/test/java/io/grpc/NameResolverProviderTest.java +++ b/core/src/test/java/io/grpc/NameResolverProviderTest.java @@ -25,10 +25,17 @@ import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import io.grpc.internal.DnsNameResolverProvider; +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.net.URI; +import java.net.URL; import java.util.Collections; +import java.util.Enumeration; import java.util.List; +import java.util.NoSuchElementException; import java.util.ServiceConfigurationError; +import java.util.regex.Pattern; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -120,15 +127,24 @@ public class NameResolverProviderTest { } @Test - public void getCandidatesViaHardCoded_usesProvidedClassLoader() { + public void getCandidatesViaHardCoded_triesToLoadClasses() throws Exception { + ClassLoader cl = getClass().getClassLoader(); final RuntimeException toThrow = new RuntimeException(); - try { - NameResolverProvider.getCandidatesViaHardCoded(new ClassLoader() { - @Override - public Class loadClass(String name) { + // Prevent DnsNameResolverProvider from being known + cl = new FilteringClassLoader(cl, serviceFile); + cl = new ClassLoader(cl) { + @Override + public Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (name.startsWith("io.grpc.internal.")) { throw toThrow; + } else { + return super.loadClass(name, resolve); } - }); + } + }; + cl = new StaticTestingClassLoader(cl, Pattern.compile("io\\.grpc\\.[^.]*")); + try { + invokeGetCandidatesViaHardCoded(cl); fail("Expected exception"); } catch (RuntimeException ex) { assertSame(toThrow, ex); @@ -136,14 +152,22 @@ public class NameResolverProviderTest { } @Test - public void getCandidatesViaHardCoded_ignoresMissingClasses() { - Iterable i = - NameResolverProvider.getCandidatesViaHardCoded(new ClassLoader() { - @Override - public Class loadClass(String name) throws ClassNotFoundException { - throw new ClassNotFoundException(); - } - }); + public void getCandidatesViaHardCoded_ignoresMissingClasses() throws Exception { + ClassLoader cl = getClass().getClassLoader(); + // Prevent DnsNameResolverProvider from being known + cl = new FilteringClassLoader(cl, serviceFile); + cl = new ClassLoader(cl) { + @Override + public Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (name.startsWith("io.grpc.internal.")) { + throw new ClassNotFoundException(); + } else { + return super.loadClass(name, resolve); + } + } + }; + cl = new StaticTestingClassLoader(cl, Pattern.compile("io\\.grpc\\.[^.]*")); + Iterable i = invokeGetCandidatesViaHardCoded(cl); assertFalse("Iterator should be empty", i.iterator().hasNext()); } @@ -160,6 +184,53 @@ public class NameResolverProviderTest { } } + private static Iterable invokeGetCandidatesViaHardCoded(ClassLoader cl) throws Exception { + // An error before the invoke likely means there is a bug in the test + Class klass = Class.forName(NameResolverProvider.class.getName(), true, cl); + Method getCandidatesViaHardCoded = klass.getMethod("getCandidatesViaHardCoded"); + try { + return (Iterable) getCandidatesViaHardCoded.invoke(null); + } catch (InvocationTargetException ex) { + if (ex.getCause() instanceof Exception) { + throw (Exception) ex.getCause(); + } + throw ex; + } + } + + private static class FilteringClassLoader extends ClassLoader { + private final String resource; + + public FilteringClassLoader(ClassLoader parent, String resource) { + super(parent); + this.resource = resource; + } + + @Override + public URL getResource(String name) { + if (resource.equals(name)) { + return null; + } + return super.getResource(name); + } + + @Override + public Enumeration getResources(String name) throws IOException { + if (resource.equals(name)) { + return new Enumeration() { + @Override public boolean hasMoreElements() { + return false; + } + + @Override public URL nextElement() { + throw new NoSuchElementException(); + } + }; + } + return super.getResources(name); + } + } + private static class BaseProvider extends NameResolverProvider { private final boolean isAvailable; private final int priority; diff --git a/core/src/test/java/io/grpc/StaticTestingClassLoader.java b/core/src/test/java/io/grpc/StaticTestingClassLoader.java new file mode 100644 index 0000000000..b0695900c8 --- /dev/null +++ b/core/src/test/java/io/grpc/StaticTestingClassLoader.java @@ -0,0 +1,76 @@ +/* + * Copyright 2017, gRPC Authors All rights reserved. + * + * Licensed 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 io.grpc; + +import com.google.common.base.Preconditions; +import io.grpc.internal.IoUtils; +import java.io.IOException; +import java.io.InputStream; +import java.util.regex.Pattern; + +/** + * A class loader that can be used to repeatedly trigger static initialization of a class. A new + * instance is required per test. + */ +public final class StaticTestingClassLoader extends ClassLoader { + private final Pattern classesToDefine; + + public StaticTestingClassLoader(ClassLoader parent, Pattern classesToDefine) { + super(parent); + this.classesToDefine = Preconditions.checkNotNull(classesToDefine, "classesToDefine"); + } + + @Override + protected Class findClass(String name) throws ClassNotFoundException { + if (!classesToDefine.matcher(name).matches()) { + throw new ClassNotFoundException(name); + } + InputStream is = getResourceAsStream(name.replace('.', '/') + ".class"); + if (is == null) { + throw new ClassNotFoundException(name); + } + byte[] b; + try { + b = IoUtils.toByteArray(is); + } catch (IOException ex) { + throw new ClassNotFoundException(name, ex); + } + return defineClass(name, b, 0, b.length); + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + // Reverse normal loading order; check this class loader before its parent + synchronized (getClassLoadingLock(name)) { + Class klass = findLoadedClass(name); + if (klass == null) { + try { + klass = findClass(name); + } catch (ClassNotFoundException e) { + // This ClassLoader doesn't know a class with that name; that's part of normal operation + } + } + if (klass == null) { + klass = super.loadClass(name, false); + } + if (resolve) { + resolveClass(klass); + } + return klass; + } + } +} diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java index 3e87038dea..d69888fcd5 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java @@ -46,8 +46,7 @@ public class DnsNameResolverProviderTest { @Test public void providedHardCoded() { - for (NameResolverProvider current - : NameResolverProvider.getCandidatesViaHardCoded(getClass().getClassLoader())) { + for (NameResolverProvider current : NameResolverProvider.getCandidatesViaHardCoded()) { if (current instanceof DnsNameResolverProvider) { return; } diff --git a/examples/android/helloworld/app/proguard-rules.pro b/examples/android/helloworld/app/proguard-rules.pro index 8da2d63238..1507a52678 100644 --- a/examples/android/helloworld/app/proguard-rules.pro +++ b/examples/android/helloworld/app/proguard-rules.pro @@ -15,5 +15,3 @@ -dontwarn javax.naming.** -dontwarn okio.** -dontwarn sun.misc.Unsafe --keep class io.grpc.internal.DnsNameResolverProvider --keep class io.grpc.okhttp.OkHttpChannelProvider diff --git a/examples/android/routeguide/app/proguard-rules.pro b/examples/android/routeguide/app/proguard-rules.pro index 6471f5f364..e356773f27 100644 --- a/examples/android/routeguide/app/proguard-rules.pro +++ b/examples/android/routeguide/app/proguard-rules.pro @@ -14,5 +14,3 @@ -dontwarn okio.** # Ignores: can't find referenced class javax.lang.model.element.Modifier -dontwarn com.google.errorprone.annotations.** --keep class io.grpc.internal.DnsNameResolverProvider --keep class io.grpc.okhttp.OkHttpChannelProvider diff --git a/netty/src/test/java/io/grpc/netty/NettyChannelProviderTest.java b/netty/src/test/java/io/grpc/netty/NettyChannelProviderTest.java index b1d86c4883..5968a2e1b6 100644 --- a/netty/src/test/java/io/grpc/netty/NettyChannelProviderTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyChannelProviderTest.java @@ -44,8 +44,7 @@ public class NettyChannelProviderTest { @Test public void providedHardCoded() { - for (ManagedChannelProvider current - : ManagedChannelProvider.getCandidatesViaHardCoded(getClass().getClassLoader())) { + for (ManagedChannelProvider current : ManagedChannelProvider.getCandidatesViaHardCoded()) { if (current instanceof NettyChannelProvider) { return; } diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelProviderTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelProviderTest.java index 00abe9f19d..6a2ef7f37e 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelProviderTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelProviderTest.java @@ -43,8 +43,7 @@ public class OkHttpChannelProviderTest { @Test public void providedHardCoded() { - for (ManagedChannelProvider current - : ManagedChannelProvider.getCandidatesViaHardCoded(getClass().getClassLoader())) { + for (ManagedChannelProvider current : ManagedChannelProvider.getCandidatesViaHardCoded()) { if (current instanceof OkHttpChannelProvider) { return; }