ResourcesManager: Return null on failure to create Resources

A lot of calling framework code expects a null value on failure,
and didn't catch the previous exception. There were some strange
corner cases where previously a null value was not checked for
in framework code, allowing the null Resources object to be
returned to the caller. Introducing an exception changed the
semantics and can crash certain apps.

Bug:30422475
Change-Id: I51d34ae43c9ec605a8790989c56cf85b815ff5b8

diff --git a/core/java/android/app/ApplicationPackageManager.java b/core/java/android/app/ApplicationPackageManager.java
index 8cc1bc4..37faa2e 100644 (file)
--- a/core/java/android/app/ApplicationPackageManager.java
+++ b/core/java/android/app/ApplicationPackageManager.java
@@ -1245,18 +1245,16 @@ public class ApplicationPackageManager extends PackageManager {
             return mContext.mMainThread.getSystemContext().getResources();
         }
         final boolean sameUid = (app.uid == Process.myUid());
-        try {
-            return mContext.mMainThread.getTopLevelResources(
+        final Resources r = mContext.mMainThread.getTopLevelResources(
                     sameUid ? app.sourceDir : app.publicSourceDir,
                     sameUid ? app.splitSourceDirs : app.splitPublicSourceDirs,
                     app.resourceDirs, app.sharedLibraryFiles, Display.DEFAULT_DISPLAY,
                     mContext.mPackageInfo);
-        } catch (Resources.NotFoundException cause) {
-            final NameNotFoundException ex =
-                    new NameNotFoundException("Unable to open " + app.publicSourceDir);
-            ex.initCause(cause);
-            throw ex;
+        if (r != null) {
+            return r;
         }
+        throw new NameNotFoundException("Unable to open " + app.publicSourceDir);
+
     }
 
     @Override

diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java
index 9a9f793..d2e0327 100644 (file)
--- a/core/java/android/app/ResourcesManager.java
+++ b/core/java/android/app/ResourcesManager.java
@@ -242,7 +242,7 @@ public class ResourcesManager {
      * @return a new AssetManager.
     */
     @VisibleForTesting
-    protected @NonNull AssetManager createAssetManager(@NonNull final ResourcesKey key) {
+    protected @Nullable AssetManager createAssetManager(@NonNull final ResourcesKey key) {
         AssetManager assets = new AssetManager();
 
         // resDir can be null if the 'android' package is creating a new Resources object.
@@ -250,15 +250,16 @@ public class ResourcesManager {
         // already.
         if (key.mResDir != null) {
             if (assets.addAssetPath(key.mResDir) == 0) {
-                throw new Resources.NotFoundException("failed to add asset path " + key.mResDir);
+                Log.e(TAG, "failed to add asset path " + key.mResDir);
+                return null;
             }
         }
 
         if (key.mSplitResDirs != null) {
             for (final String splitResDir : key.mSplitResDirs) {
                 if (assets.addAssetPath(splitResDir) == 0) {
-                    throw new Resources.NotFoundException(
-                            "failed to add split asset path " + splitResDir);
+                    Log.e(TAG, "failed to add split asset path " + splitResDir);
+                    return null;
                 }
             }
         }
@@ -303,11 +304,15 @@ public class ResourcesManager {
         return config;
     }
 
-    private @NonNull ResourcesImpl createResourcesImpl(@NonNull ResourcesKey key) {
+    private @Nullable ResourcesImpl createResourcesImpl(@NonNull ResourcesKey key) {
         final DisplayAdjustments daj = new DisplayAdjustments(key.mOverrideConfiguration);
         daj.setCompatibilityInfo(key.mCompatInfo);
 
         final AssetManager assets = createAssetManager(key);
+        if (assets == null) {
+            return null;
+        }
+
         final DisplayMetrics dm = getDisplayMetrics(key.mDisplayId, daj);
         final Configuration config = generateConfig(key, dm);
         final ResourcesImpl impl = new ResourcesImpl(assets, dm, config, daj);
@@ -323,7 +328,7 @@ public class ResourcesManager {
      * @param key The key to match.
      * @return a ResourcesImpl if the key matches a cache entry, null otherwise.
      */
-    private ResourcesImpl findResourcesImplForKeyLocked(@NonNull ResourcesKey key) {
+    private @Nullable ResourcesImpl findResourcesImplForKeyLocked(@NonNull ResourcesKey key) {
         WeakReference<ResourcesImpl> weakImplRef = mResourceImpls.get(key);
         ResourcesImpl impl = weakImplRef != null ? weakImplRef.get() : null;
         if (impl != null && impl.getAssets().isUpToDate()) {
@@ -338,12 +343,14 @@ public class ResourcesManager {
      * @param key The key to match.
      * @return a ResourcesImpl object matching the key.
      */
-    private @NonNull ResourcesImpl findOrCreateResourcesImplForKeyLocked(
+    private @Nullable ResourcesImpl findOrCreateResourcesImplForKeyLocked(
             @NonNull ResourcesKey key) {
         ResourcesImpl impl = findResourcesImplForKeyLocked(key);
         if (impl == null) {
             impl = createResourcesImpl(key);
-            mResourceImpls.put(key, new WeakReference<>(impl));
+            if (impl != null) {
+                mResourceImpls.put(key, new WeakReference<>(impl));
+            }
         }
         return impl;
     }
@@ -352,7 +359,8 @@ public class ResourcesManager {
      * Find the ResourcesKey that this ResourcesImpl object is associated with.
      * @return the ResourcesKey or null if none was found.
      */
-    private ResourcesKey findKeyForResourceImplLocked(@NonNull ResourcesImpl resourceImpl) {
+    private @Nullable ResourcesKey findKeyForResourceImplLocked(
+            @NonNull ResourcesImpl resourceImpl) {
         final int refCount = mResourceImpls.size();
         for (int i = 0; i < refCount; i++) {
             WeakReference<ResourcesImpl> weakImplRef = mResourceImpls.valueAt(i);
@@ -480,7 +488,7 @@ public class ResourcesManager {
      *                    {@link ClassLoader#getSystemClassLoader()} is used.
      * @return a Resources object from which to access resources.
      */
-    public @NonNull Resources createBaseActivityResources(@NonNull IBinder activityToken,
+    public @Nullable Resources createBaseActivityResources(@NonNull IBinder activityToken,
             @Nullable String resDir,
             @Nullable String[] splitResDirs,
             @Nullable String[] overlayDirs,
@@ -534,7 +542,7 @@ public class ResourcesManager {
      *         {@link #applyConfigurationToResourcesLocked(Configuration, CompatibilityInfo)}
      *         is called.
      */
-    private @NonNull Resources getOrCreateResources(@Nullable IBinder activityToken,
+    private @Nullable Resources getOrCreateResources(@Nullable IBinder activityToken,
             @NonNull ResourcesKey key, @NonNull ClassLoader classLoader) {
         synchronized (this) {
             if (DEBUG) {
@@ -589,6 +597,9 @@ public class ResourcesManager {
 
         // If we're here, we didn't find a suitable ResourcesImpl to use, so create one now.
         ResourcesImpl resourcesImpl = createResourcesImpl(key);
+        if (resourcesImpl == null) {
+            return null;
+        }
 
         synchronized (this) {
             ResourcesImpl existingResourcesImpl = findResourcesImplForKeyLocked(key);
@@ -642,7 +653,7 @@ public class ResourcesManager {
      * {@link ClassLoader#getSystemClassLoader()} is used.
      * @return a Resources object from which to access resources.
      */
-    public @NonNull Resources getResources(@Nullable IBinder activityToken,
+    public @Nullable Resources getResources(@Nullable IBinder activityToken,
             @Nullable String resDir,
             @Nullable String[] splitResDirs,
             @Nullable String[] overlayDirs,
@@ -765,10 +776,12 @@ public class ResourcesManager {
                     ResourcesImpl resourcesImpl = findResourcesImplForKeyLocked(newKey);
                     if (resourcesImpl == null) {
                         resourcesImpl = createResourcesImpl(newKey);
-                        mResourceImpls.put(newKey, new WeakReference<>(resourcesImpl));
+                        if (resourcesImpl != null) {
+                            mResourceImpls.put(newKey, new WeakReference<>(resourcesImpl));
+                        }
                     }
 
-                    if (resourcesImpl != resources.getImpl()) {
+                    if (resourcesImpl != null && resourcesImpl != resources.getImpl()) {
                         // Set the ResourcesImpl, updating it for all users of this Resources
                         // object.
                         resources.setImpl(resourcesImpl);
@@ -910,7 +923,11 @@ public class ResourcesManager {
                 if (r != null) {
                     final ResourcesKey key = updatedResourceKeys.get(r.getImpl());
                     if (key != null) {
-                        r.setImpl(findOrCreateResourcesImplForKeyLocked(key));
+                        final ResourcesImpl impl = findOrCreateResourcesImplForKeyLocked(key);
+                        if (impl == null) {
+                            throw new Resources.NotFoundException("failed to load " + libAsset);
+                        }
+                        r.setImpl(impl);
                     }
                 }
             }
@@ -923,7 +940,11 @@ public class ResourcesManager {
                     if (r != null) {
                         final ResourcesKey key = updatedResourceKeys.get(r.getImpl());
                         if (key != null) {
-                            r.setImpl(findOrCreateResourcesImplForKeyLocked(key));
+                            final ResourcesImpl impl = findOrCreateResourcesImplForKeyLocked(key);
+                            if (impl == null) {
+                                throw new Resources.NotFoundException("failed to load " + libAsset);
+                            }
+                            r.setImpl(impl);
                         }
                     }
                 }