From b6c3a604b040df93a80326799f4f89a8503a4f92 Mon Sep 17 00:00:00 2001 From: Alan Stokes Date: Fri, 2 Nov 2018 12:10:42 +0000 Subject: [PATCH] DexLoadReporter needs to handle arbitrary class loaders. Also made sure we can handle null classpaths. Test: atest -p services/core/java/com/android/server/pm/dex Bug: 111336847 Change-Id: Idabf3fb9a09a0764e805679ac29cd8455e8dc267 --- core/java/android/app/DexLoadReporter.java | 8 ++--- .../android/content/pm/IPackageManager.aidl | 2 +- .../com/android/server/pm/dex/DexManager.java | 14 ++++++-- .../android/server/pm/dex/DexoptUtils.java | 3 +- .../server/pm/dex/DexManagerTests.java | 34 +++++++++++++++---- .../server/pm/dex/DexoptUtilsTest.java | 12 +++++++ 6 files changed, 58 insertions(+), 15 deletions(-) diff --git a/core/java/android/app/DexLoadReporter.java b/core/java/android/app/DexLoadReporter.java index 0643414727cfd..229bee55e9115 100644 --- a/core/java/android/app/DexLoadReporter.java +++ b/core/java/android/app/DexLoadReporter.java @@ -87,7 +87,7 @@ import java.util.Set; } @Override - public void report(List classLoadersChain, List classPaths) { + public void report(List classLoadersChain, List classPaths) { if (classLoadersChain.size() != classPaths.size()) { Slog.wtf(TAG, "Bad call to DexLoadReporter: argument size mismatch"); return; @@ -113,12 +113,12 @@ import java.util.Set; registerSecondaryDexForProfiling(dexPathsForRegistration); } - private void notifyPackageManager(List classLoadersChain, + private void notifyPackageManager(List classLoadersChain, List classPaths) { // Get the class loader names for the binder call. List classLoadersNames = new ArrayList<>(classPaths.size()); - for (BaseDexClassLoader bdc : classLoadersChain) { - classLoadersNames.add(bdc.getClass().getName()); + for (ClassLoader classLoader : classLoadersChain) { + classLoadersNames.add(classLoader.getClass().getName()); } String packageName = ActivityThread.currentPackageName(); try { diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl index bc5b32c69b59f..da7d6643c3fb0 100644 --- a/core/java/android/content/pm/IPackageManager.aidl +++ b/core/java/android/content/pm/IPackageManager.aidl @@ -475,7 +475,7 @@ interface IPackageManager { * @param classPaths the class paths corresponding to the class loaders names from * {@param classLoadersNames}. The the first element corresponds to the first class loader * and so on. A classpath is represented as a list of dex files separated by - * {@code File.pathSeparator}. + * {@code File.pathSeparator}, or null if the class loader's classpath is not known. * The dex files found in the first class path will be recorded in the usage file. * @param loaderIsa the ISA of the loader process */ diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java index 753c2833b0568..580e4f481a270 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -35,6 +35,7 @@ import android.util.Slog; import android.util.jar.StrictJarFile; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.ArrayUtils; import com.android.server.pm.Installer; import com.android.server.pm.Installer.InstallerException; @@ -153,7 +154,7 @@ public class DexManager { * @param classPaths the class paths corresponding to the class loaders names from * {@param classLoadersNames}. The the first element corresponds to the first class loader * and so on. A classpath is represented as a list of dex files separated by - * {@code File.pathSeparator}. + * {@code File.pathSeparator}, or null if the class loader's classpath is not known. * The dex files found in the first class path will be recorded in the usage file. * @param loaderIsa the ISA of the app loading the dex files * @param loaderUserId the user id which runs the code loading the dex files @@ -169,7 +170,8 @@ public class DexManager { } } - private void notifyDexLoadInternal(ApplicationInfo loadingAppInfo, + @VisibleForTesting + /*package*/ void notifyDexLoadInternal(ApplicationInfo loadingAppInfo, List classLoaderNames, List classPaths, String loaderIsa, int loaderUserId) { if (classLoaderNames.size() != classPaths.size()) { @@ -186,8 +188,14 @@ public class DexManager { return; } + // The first classpath should never be null because the first classloader + // should always be an instance of BaseDexClassLoader. + String firstClassPath = classPaths.get(0); + if (firstClassPath == null) { + return; + } // The classpath is represented as a list of dex files separated by File.pathSeparator. - String[] dexPathsToRegister = classPaths.get(0).split(File.pathSeparator); + String[] dexPathsToRegister = firstClassPath.split(File.pathSeparator); // Encode the class loader contexts for the dexPathsToRegister. String[] classLoaderContexts = DexoptUtils.processContextForDexLoad( diff --git a/services/core/java/com/android/server/pm/dex/DexoptUtils.java b/services/core/java/com/android/server/pm/dex/DexoptUtils.java index e1310a2f1ab37..d2600b5060b5a 100644 --- a/services/core/java/com/android/server/pm/dex/DexoptUtils.java +++ b/services/core/java/com/android/server/pm/dex/DexoptUtils.java @@ -318,7 +318,8 @@ public final class DexoptUtils { // is fine (they come over binder). Even if something changes we expect the sizes to be // very small and it shouldn't matter much. for (int i = 1; i < classLoadersNames.size(); i++) { - if (!ClassLoaderFactory.isValidClassLoaderName(classLoadersNames.get(i))) { + if (!ClassLoaderFactory.isValidClassLoaderName(classLoadersNames.get(i)) + || classPaths.get(i) == null) { return null; } String classpath = encodeClasspath(classPaths.get(i).split(File.pathSeparator)); diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java index 030f9cca265ea..b30c043099558 100644 --- a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java +++ b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java @@ -69,6 +69,7 @@ public class DexManagerTests { private static final String PATH_CLASS_LOADER_NAME = PathClassLoader.class.getName(); private static final String DELEGATE_LAST_CLASS_LOADER_NAME = DelegateLastClassLoader.class.getName(); + private static final String UNSUPPORTED_CLASS_LOADER_NAME = "unsupported.class_loader"; @Rule public MockitoRule mockito = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS); @Mock Installer mInstaller; @@ -106,7 +107,7 @@ public class DexManagerTests { mDoesNotExist = new TestData("DOES.NOT.EXIST", isa, mUser1); mBarUser0UnsupportedClassLoader = new TestData(bar, isa, mUser0, - "unsupported.class_loader"); + UNSUPPORTED_CLASS_LOADER_NAME); mBarUser0DelegateLastClassLoader = new TestData(bar, isa, mUser0, DELEGATE_LAST_CLASS_LOADER_NAME); @@ -405,6 +406,24 @@ public class DexManagerTests { assertNoUseInfo(mBarUser0UnsupportedClassLoader); } + @Test + public void testNotifySupportedAndUnsupportedClassLoader() { + String classPath = String.join(File.pathSeparator, mBarUser0.getSecondaryDexPaths()); + List classLoaders = + Arrays.asList(PATH_CLASS_LOADER_NAME, UNSUPPORTED_CLASS_LOADER_NAME); + List classPaths = Arrays.asList(classPath, classPath); + notifyDexLoad(mBarUser0, classLoaders, classPaths, mUser0); + + assertNoUseInfo(mBarUser0); + } + + @Test + public void testNotifyNullClassPath() { + notifyDexLoad(mBarUser0, null, mUser0); + + assertNoUseInfo(mBarUser0); + } + @Test public void testNotifyVariableClassLoader() { // Record bar secondaries with the default PathClassLoader. @@ -500,14 +519,17 @@ public class DexManagerTests { // By default, assume a single class loader in the chain. // This makes writing tests much easier. List classLoaders = Arrays.asList(testData.mClassLoader); - List classPaths = Arrays.asList(String.join(File.pathSeparator, dexPaths)); + List classPaths = (dexPaths == null) + ? Arrays.asList((String) null) + : Arrays.asList(String.join(File.pathSeparator, dexPaths)); notifyDexLoad(testData, classLoaders, classPaths, loaderUserId); } - private void notifyDexLoad(TestData testData, List classLoader, List classPaths, - int loaderUserId) { - mDexManager.notifyDexLoad(testData.mPackageInfo.applicationInfo, classLoader, classPaths, - testData.mLoaderIsa, loaderUserId); + private void notifyDexLoad(TestData testData, List classLoaders, + List classPaths, int loaderUserId) { + // We call the internal function so any exceptions thrown cause test failures. + mDexManager.notifyDexLoadInternal(testData.mPackageInfo.applicationInfo, classLoaders, + classPaths, testData.mLoaderIsa, loaderUserId); } private PackageUseInfo getPackageUseInfo(TestData testData) { diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/DexoptUtilsTest.java b/services/tests/servicestests/src/com/android/server/pm/dex/DexoptUtilsTest.java index 150f7f0c948c2..6e0f56c8e7c80 100644 --- a/services/tests/servicestests/src/com/android/server/pm/dex/DexoptUtilsTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/dex/DexoptUtilsTest.java @@ -352,6 +352,18 @@ public class DexoptUtilsTest { assertNull(context); } + @Test + public void testProcessContextForDexLoadNoClassPath() { + List classLoaders = Arrays.asList( + DELEGATE_LAST_CLASS_LOADER_NAME, + PATH_CLASS_LOADER_NAME); + List classPaths = Arrays.asList( + String.join(File.pathSeparator, "foo.dex", "bar.dex"), + null); + String[] context = DexoptUtils.processContextForDexLoad(classLoaders, classPaths); + assertNull(context); + } + @Test public void testProcessContextForDexLoadIllegalCallEmptyList() { boolean gotException = false;