Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit eb832a8a authored by Chun-Wei Wang's avatar Chun-Wei Wang
Browse files

Fix wrong path is returned when 2 files have the same name 

Before the fix, the cache is reused when 2 files have the same name.
Now the full file path is included when calculating the cache key.
This will reduce the chance of key collision. We also check if the
paths match to ensure 2 different files are never mapped to the same
cache entry.

Bug: 242639590
Test: atest PackageParserTest
Change-Id: I4d240407f6fd6de9a64f9ea46339d73202d40927
parent 8b082eae

services/core/java/com/android/server/pm/parsing/PackageCacher.java

+8 −1
Original line number Diff line number Diff line
@@ -62,6 +62,8 @@ public class PackageCacher { 
        StringBuilder sb = new StringBuilder(packageFile.getName());

        sb.append('-');

        sb.append(flags);

        sb.append('-');

        sb.append(packageFile.getAbsolutePath().hashCode());



        return sb.toString();

    }
@@ -171,7 +173,12 @@ public class PackageCacher { 
            }



            final byte[] bytes = IoUtils.readFileAsByteArray(cacheFile.getAbsolutePath());

            return fromCacheEntry(bytes);

            ParsedPackage parsed = fromCacheEntry(bytes);

            if (!packageFile.getAbsolutePath().equals(parsed.getPath())) {

                // Don't use this cache if the path doesn't match

                return null;

            }

            return parsed;

        } catch (Throwable e) {

            Slog.w(TAG, "Error reading package cache: ", e);

services/tests/servicestests/src/com/android/server/pm/PackageParserTest.java

+38 −11
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ package com.android.server.pm; 


import static com.android.server.pm.permission.CompatibilityPermissionInfo.COMPAT_PERMS;



import static com.google.common.truth.Truth.assertThat;

import static com.google.common.truth.Truth.assertWithMessage;



import static org.junit.Assert.assertArrayEquals;
@@ -101,7 +102,6 @@ import java.io.IOException; 
import java.io.InputStream;

import java.lang.reflect.Array;

import java.lang.reflect.Field;

import java.nio.charset.StandardCharsets;

import java.nio.file.Files;

import java.util.ArrayList;

import java.util.Arrays;
@@ -259,6 +259,38 @@ public class PackageParserTest { 
        return tmpFile;

    }



    /**

     * Extracts the asset file to $mTmpDir/$dirname/$filename.

     */

    private File extractFile(String filename, String dirname) throws Exception {

        final Context context = InstrumentationRegistry.getTargetContext();

        File dir = new File(mTmpDir, dirname);

        dir.mkdir();

        final File tmpFile = new File(dir, filename);

        try (InputStream inputStream = context.getAssets().openNonAsset(filename)) {

            Files.copy(inputStream, tmpFile.toPath(), REPLACE_EXISTING);

        }

        return tmpFile;

    }



    /**

     * Tests the path of cached ParsedPackage.

     */

    @Test

    public void testCache_SameFileName() throws Exception {

        // Prepare 2 package files with the same name but different paths

        TestPackageParser2 parser = new TestPackageParser2(mTmpDir);

        final File f1 = extractFile(TEST_APP1_APK, "dir1");

        final File f2 = extractFile(TEST_APP1_APK, "dir2");

        // Sleep for a while so that the cache file will be newer and valid

        Thread.sleep(1000);

        ParsedPackage pr1 = parser.parsePackage(f1, 0, true);

        ParsedPackage pr2 = parser.parsePackage(f2, 0, true);

        // Check the path of cached ParsedPackage

        assertThat(pr1.getPath()).isEqualTo(f1.getAbsolutePath());

        assertThat(pr2.getPath()).isEqualTo(f2.getAbsolutePath());

    }



    /**

     * Tests AndroidManifest.xml with no android:isolatedSplits attribute.

     */
@@ -632,8 +664,8 @@ public class PackageParserTest { 
    }



    /**

     * A trivial subclass of package parser that only caches the package name, and throws away

     * all other information.

     * A subclass of package parser that adds a "cache_" prefix to the package name for the cached

     * results. This is used by tests to tell if a ParsedPackage is generated from the cache or not.

     */

    public static class CachePackageNameParser extends PackageParser2 {
@@ -656,16 +688,11 @@ public class PackageParserTest { 


        void setCacheDir(@NonNull File cacheDir) {

            this.mCacher = new PackageCacher(cacheDir) {

                @Override

                public byte[] toCacheEntry(ParsedPackage pkg) {

                    return ("cache_" + pkg.getPackageName()).getBytes(StandardCharsets.UTF_8);

                }



                @Override

                public ParsedPackage fromCacheEntry(byte[] cacheEntry) {

                    return ((ParsedPackage) PackageImpl.forTesting(

                            new String(cacheEntry, StandardCharsets.UTF_8))

                            .hideAsParsed());

                    ParsedPackage parsed = super.fromCacheEntry(cacheEntry);

                    parsed.setPackageName("cache_" + parsed.getPackageName());

                    return parsed;

                }

            };

        }

services/tests/servicestests/src/com/android/server/pm/parsing/TestPackageParser2.kt

+4 −2
Original line number Diff line number Diff line
@@ -17,9 +17,11 @@ 
package com.android.server.pm.parsing



import android.content.pm.ApplicationInfo

import java.io.File



class TestPackageParser2 : PackageParser2(null /* separateProcesses */, null /* displayMetrics */,

        null /* cacheDir */, object : PackageParser2.Callback() {

class TestPackageParser2(var cacheDir: File? = null) : PackageParser2(

        null /* separateProcesses */, null /* displayMetrics */,

        cacheDir /* cacheDir */, object : PackageParser2.Callback() {

    override fun isChangeEnabled(changeId: Long, appInfo: ApplicationInfo): Boolean {

        return true

    }