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

Commit 46882534 authored by Winson's avatar Winson
Browse files

Query PlatformCompat for targetSdk check in AndroidPackageParsingTestBase 

There are prebuilts in the tree and perhaps APKs in general
which don't conform to the manifest checks added in R, so the code
that parses the APK to verify v1 vs v2 needs to actually query
PlatformCompat to do the proper targetSdkVersion check and allow
the APK through if it's not updated yet.

A related change to default enabled inside ParsingPackageImpl to
true was also made, as if any malformed APK was allowed through
without an <application> tag, it would never hit the code
where enabled gets assigned to default true.

Any further errors spit out by this test are critical and need to
updated so that their APKs can be installed on R.

Bug: 153058196

Test: manual verify error behavior with broken APK
Test: atest com.android.server.pm.parsing

Change-Id: I2b117f2098d8bd62b92921178a098e838b55b06d
parent b164ea9d

core/java/android/content/pm/parsing/ParsingPackageImpl.java

+6 −1
Original line number Diff line number Diff line
@@ -322,7 +322,12 @@ public class ParsingPackageImpl implements ParsingPackage, Parcelable { 
    private String className;

    private int compatibleWidthLimitDp;

    private int descriptionRes;

    private boolean enabled;



    // Usually there's code to set this to true during parsing, but it's possible to install an APK

    // targeting <R that doesn't contain an <application> tag. That code would be skipped and never

    // assign this, so initialize this to true for those cases.

    private boolean enabled = true;



    private boolean crossProfile;

    private int fullBackupContent;

    private int iconRes;

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

+9 −2
Original line number Diff line number Diff line
@@ -18,7 +18,9 @@ package com.android.server.pm.parsing 


import android.content.pm.PackageManager

import android.platform.test.annotations.Presubmit

import com.google.common.truth.Expect

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

import org.junit.Rule

import org.junit.Test



/**
@@ -28,6 +30,9 @@ import org.junit.Test 
@Presubmit

class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() {



    @get:Rule

    val expect = Expect.create()



    @Test

    fun applicationInfoEquality() {

        val flags = PackageManager.GET_META_DATA or PackageManager.GET_SHARED_LIBRARY_FILES
@@ -41,7 +46,8 @@ class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() { 
            } else {

                "$firstName | $secondName"

            }

            assertWithMessage(packageName).that(it.first?.dumpToString())

            expect.withMessage("${it.first?.sourceDir} $packageName")

                    .that(it.first?.dumpToString())

                    .isEqualTo(it.second?.dumpToString())

        }

    }
@@ -71,7 +77,8 @@ class AndroidPackageParsingEquivalenceTest : AndroidPackageParsingTestBase() { 
            } else {

                "$firstName | $secondName"

            }

            assertWithMessage(packageName).that(it.first?.dumpToString())

            expect.withMessage("${it.first?.applicationInfo?.sourceDir} $packageName")

                    .that(it.first?.dumpToString())

                    .isEqualTo(it.second?.dumpToString())

        }

    }

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

+65 −23
Original line number Diff line number Diff line
@@ -29,14 +29,17 @@ import android.content.pm.PermissionInfo 
import android.content.pm.ProviderInfo

import android.os.Debug

import android.os.Environment

import android.os.ServiceManager

import android.util.SparseArray

import androidx.test.platform.app.InstrumentationRegistry

import com.android.internal.compat.IPlatformCompat

import com.android.server.pm.PackageManagerService

import com.android.server.pm.PackageSetting

import com.android.server.pm.parsing.pkg.AndroidPackage

import com.android.server.pm.pkg.PackageStateUnserialized

import com.android.server.testutils.mockThrowOnUnmocked

import com.android.server.testutils.whenever

import org.junit.After

import org.junit.BeforeClass

import org.mockito.Mockito

import org.mockito.Mockito.anyInt
@@ -59,7 +62,27 @@ open class AndroidPackageParsingTestBase { 
            setCallback { false /* hasFeature */ }

        }



        protected val packageParser2 = TestPackageParser2()

        private val platformCompat = IPlatformCompat.Stub

                .asInterface(ServiceManager.getService(Context.PLATFORM_COMPAT_SERVICE))



        protected val packageParser2 = PackageParser2(null /* separateProcesses */,

                false /* onlyCoreApps */, context.resources.displayMetrics, null /* cacheDir */,

                object : PackageParser2.Callback() {

                    override fun isChangeEnabled(

                        changeId: Long,

                        appInfo: ApplicationInfo

                    ): Boolean {

                        // This test queries PlatformCompat because prebuilts in the tree

                        // may not be updated to be compliant with the latest enforcement checks.

                        return platformCompat.isChangeEnabled(changeId, appInfo)

                    }



                    // Assume the device doesn't support anything. This will affect permission

                    // parsing and will force <uses-permission/> declarations to include all

                    // requiredNotFeature permissions and exclude all requiredFeature permissions.

                    // This mirrors the old behavior.

                    override fun hasFeature(feature: String) = false

                })



        /**

         * It would be difficult to mock all possibilities, so just use the APKs on device.
@@ -91,21 +114,23 @@ open class AndroidPackageParsingTestBase { 


        lateinit var newPackages: List<AndroidPackage>



        var failureInBeforeClass: Throwable? = null

        private val thrownInSetUp = mutableListOf<Throwable>()



        @Suppress("ConstantConditionIf")

        @JvmStatic

        @BeforeClass

        fun setUpPackages() {

            failureInBeforeClass = null

            try {

                this.oldPackages = apks.map {

            this.oldPackages = apks.mapNotNull {

                tryOrNull {

                    packageParser.parsePackage(it, PackageParser.PARSE_IS_SYSTEM_DIR, false)

                }

            }



                this.newPackages = apks.map {

            this.newPackages = apks.mapNotNull {

                tryOrNull {

                    packageParser2.parsePackage(it, PackageParser.PARSE_IS_SYSTEM_DIR, false)

                }

            }



            if (DUMP_HPROF_TO_EXTERNAL) {

                System.gc()
@@ -115,12 +140,6 @@ open class AndroidPackageParsingTestBase { 
                        .absolutePath

                        .run(Debug::dumpHprofData)

            }

            } catch (t: Throwable) {

                // If we crash here we cause a tool failure (because we don't run any of the tests

                // in the subclasses, leading to a difference between expected and actual test

                // result counts).

                failureInBeforeClass = t

            }

        }



        fun oldAppInfo(pkg: PackageParser.Package, flags: Int = 0): ApplicationInfo? {
@@ -146,13 +165,36 @@ open class AndroidPackageParsingTestBase { 
            this.pkg = aPkg

            whenever(pkgState) { PackageStateUnserialized() }

        }



        private fun <T> tryOrNull(block: () -> T) = try {

            block()

        } catch (t: Throwable) {

            thrownInSetUp.add(t)

            null

        }

    }



    @org.junit.Before

    @After

    fun verifySetUpPackages() {

        failureInBeforeClass?.let {

            throw AssertionError("setUpPackages failed:", it)

        }

        if (thrownInSetUp.isEmpty()) return

        val exception = AssertionError("setUpPackages failed with ${thrownInSetUp.size} errors:\n" +

                thrownInSetUp.joinToString(separator = "\n") { it.message.orEmpty() })



        /*

            Testing infrastructure doesn't currently support errors thrown in @AfterClass,

            so instead it's thrown here. But to avoid throwing a massive repeated stack for every

            test method, only throw on the first method run in the class, clearing the list so that

            subsequent methods can run without failing. Doing this in @After lets true method

            failures propagate, as those should throw before this does.



            This will cause the failure to be attached to a different method depending on run order,

            which could make comparisons difficult. So if a failure points here, it's worth

            checking failures for all methods in all subclasses.



            TODO: When infrastructure supports @AfterClass errors, move this

        */

        thrownInSetUp.clear()

        throw exception

    }



    // The following methods dump an exact set of fields from the object to compare, because