From 83eddae76669693508d3d6b8bd4c286c9178f627 Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Mon, 2 Mar 2020 09:21:25 -0800 Subject: [PATCH] Always use TypedArray to obtain manifest values In some paths, the "isolatedSplits" attribute was read directly from the XML file. In others, it was read using a TypedArray [ie. it used the application's resource table]. It's possible we should never allow some manifest values to be set via resource table. This change does not attempt to resolve that. Instead, it merely ensures consistency to how manifest values are retrieved. Bug: 150184652 Test: atest PackageParserTest Change-Id: Ib7c6a5ae081959e2d3aba4abdf60dd52db05991a --- .../android/content/pm/PackageParser.java | 10 +-- .../pm/parsing/ParsingPackageImpl.java | 4 ++ .../pm/parsing/ParsingPackageUtils.java | 29 ++++---- services/tests/servicestests/Android.bp | 10 ++- .../android/server/pm/PackageParserTest.java | 66 ++++++++++++++++++- .../test-apps/PackageParserApp/Android.bp | 53 +++++++++++++++ .../PackageParserApp/AndroidManifestApp1.xml | 25 +++++++ .../PackageParserApp/AndroidManifestApp2.xml | 26 ++++++++ .../PackageParserApp/AndroidManifestApp3.xml | 26 ++++++++ .../PackageParserApp/res/values/values.xml | 19 ++++++ .../apps/packageparserapp/TestActivity.java | 28 ++++++++ 11 files changed, 276 insertions(+), 20 deletions(-) create mode 100644 services/tests/servicestests/test-apps/PackageParserApp/Android.bp create mode 100644 services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp1.xml create mode 100644 services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp2.xml create mode 100644 services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp3.xml create mode 100644 services/tests/servicestests/test-apps/PackageParserApp/res/values/values.xml create mode 100644 services/tests/servicestests/test-apps/PackageParserApp/src/com/android/servicestests/apps/packageparserapp/TestActivity.java diff --git a/core/java/android/content/pm/PackageParser.java b/core/java/android/content/pm/PackageParser.java index 20ddba41bda1d..1dadbda1918bf 100644 --- a/core/java/android/content/pm/PackageParser.java +++ b/core/java/android/content/pm/PackageParser.java @@ -1837,6 +1837,12 @@ public class PackageParser { pkg.coreApp = parser.getAttributeBooleanValue(null, "coreApp", false); + final boolean isolatedSplits = sa.getBoolean( + com.android.internal.R.styleable.AndroidManifest_isolatedSplits, false); + if (isolatedSplits) { + pkg.applicationInfo.privateFlags |= ApplicationInfo.PRIVATE_FLAG_ISOLATED_SPLIT_LOADING; + } + pkg.mCompileSdkVersion = sa.getInteger( com.android.internal.R.styleable.AndroidManifest_compileSdkVersion, 0); pkg.applicationInfo.compileSdkVersion = pkg.mCompileSdkVersion; @@ -1912,10 +1918,6 @@ public class PackageParser { pkg.applicationInfo.flags |= ApplicationInfo.FLAG_EXTERNAL_STORAGE; } - if (sa.getBoolean(com.android.internal.R.styleable.AndroidManifest_isolatedSplits, false)) { - pkg.applicationInfo.privateFlags |= ApplicationInfo.PRIVATE_FLAG_ISOLATED_SPLIT_LOADING; - } - // Resource boolean are -1, so 1 means we don't know the value. int supportsSmallScreens = 1; int supportsNormalScreens = 1; diff --git a/core/java/android/content/pm/parsing/ParsingPackageImpl.java b/core/java/android/content/pm/parsing/ParsingPackageImpl.java index 079a47056c24c..894ad55849227 100644 --- a/core/java/android/content/pm/parsing/ParsingPackageImpl.java +++ b/core/java/android/content/pm/parsing/ParsingPackageImpl.java @@ -433,6 +433,10 @@ public class ParsingPackageImpl implements ParsingPackage, Parcelable { R.styleable.AndroidManifest_compileSdkVersion, 0)); setCompileSdkVersionCodename(manifestArray.getNonConfigurationString( R.styleable.AndroidManifest_compileSdkVersionCodename, 0)); + + setIsolatedSplitLoading(manifestArray.getBoolean( + R.styleable.AndroidManifest_isolatedSplits, false)); + } } diff --git a/core/java/android/content/pm/parsing/ParsingPackageUtils.java b/core/java/android/content/pm/parsing/ParsingPackageUtils.java index ec771286d980f..e90ccdffa8a89 100644 --- a/core/java/android/content/pm/parsing/ParsingPackageUtils.java +++ b/core/java/android/content/pm/parsing/ParsingPackageUtils.java @@ -386,15 +386,14 @@ public class ParsingPackageUtils { return input.error(PackageManager.INSTALL_PARSE_FAILED_BAD_PACKAGE_NAME); } - TypedArray manifestArray = res.obtainAttributes(parser, R.styleable.AndroidManifest); + final TypedArray manifestArray = res.obtainAttributes(parser, R.styleable.AndroidManifest); try { - boolean isCoreApp = parser.getAttributeBooleanValue(null, "coreApp", false); - - ParsingPackage pkg = mCallback.startParsingPackage(pkgName, apkPath, codePath, - manifestArray, isCoreApp); - - ParseResult result = parseBaseApkTags(input, pkg, manifestArray, - res, parser, flags); + final boolean isCoreApp = + parser.getAttributeBooleanValue(null, "coreApp", false); + final ParsingPackage pkg = mCallback.startParsingPackage( + pkgName, apkPath, codePath, manifestArray, isCoreApp); + final ParseResult result = + parseBaseApkTags(input, pkg, manifestArray, res, parser, flags); if (result.isError()) { return result; } @@ -620,14 +619,12 @@ public class ParsingPackageUtils { return sharedUserResult; } - pkg.setInstallLocation(anInt(PackageParser.PARSE_DEFAULT_INSTALL_LOCATION, + pkg.setInstallLocation(anInteger(PackageParser.PARSE_DEFAULT_INSTALL_LOCATION, R.styleable.AndroidManifest_installLocation, sa)) - .setTargetSandboxVersion(anInt(PackageParser.PARSE_DEFAULT_TARGET_SANDBOX, + .setTargetSandboxVersion(anInteger(PackageParser.PARSE_DEFAULT_TARGET_SANDBOX, R.styleable.AndroidManifest_targetSandboxVersion, sa)) /* Set the global "on SD card" flag */ - .setExternalStorage((flags & PackageParser.PARSE_EXTERNAL_STORAGE) != 0) - .setIsolatedSplitLoading( - bool(false, R.styleable.AndroidManifest_isolatedSplits, sa)); + .setExternalStorage((flags & PackageParser.PARSE_EXTERNAL_STORAGE) != 0); boolean foundApp = false; final int depth = parser.getDepth(); @@ -2658,6 +2655,10 @@ public class ParsingPackageUtils { return sa.getInt(attribute, defaultValue); } + private static int anInteger(int defaultValue, @StyleableRes int attribute, TypedArray sa) { + return sa.getInteger(attribute, defaultValue); + } + private static int anInt(@StyleableRes int attribute, TypedArray sa) { return sa.getInt(attribute, 0); } @@ -2688,6 +2689,6 @@ public class ParsingPackageUtils { boolean hasFeature(String feature); ParsingPackage startParsingPackage(String packageName, String baseCodePath, String codePath, - TypedArray manifestArray, boolean isCoreApp); + @NonNull TypedArray manifestArray, boolean isCoreApp); } } diff --git a/services/tests/servicestests/Android.bp b/services/tests/servicestests/Android.bp index 449e75cd11a08..e58e911799313 100644 --- a/services/tests/servicestests/Android.bp +++ b/services/tests/servicestests/Android.bp @@ -91,7 +91,15 @@ android_test { enabled: false, }, - data: [":JobTestApp"], + data: [ + ":JobTestApp", + ], + + java_resources: [ + ":PackageParserTestApp1", + ":PackageParserTestApp2", + ":PackageParserTestApp3", + ], resource_zips: [":FrameworksServicesTests_apks_as_resources"], } diff --git a/services/tests/servicestests/src/com/android/server/pm/PackageParserTest.java b/services/tests/servicestests/src/com/android/server/pm/PackageParserTest.java index a19d919763079..d760629552b8a 100644 --- a/services/tests/servicestests/src/com/android/server/pm/PackageParserTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/PackageParserTest.java @@ -17,11 +17,15 @@ package com.android.server.pm; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; + import android.annotation.NonNull; +import android.content.Context; import android.content.pm.ActivityInfo; import android.content.pm.ApplicationInfo; import android.content.pm.ConfigurationInfo; @@ -30,7 +34,6 @@ import android.content.pm.FeatureInfo; import android.content.pm.PackageInfo; import android.content.pm.PackageParser; import android.content.pm.PackageUserState; -import android.content.pm.ProviderInfo; import android.content.pm.ServiceInfo; import android.content.pm.Signature; import android.content.pm.parsing.ParsingPackage; @@ -49,6 +52,7 @@ import android.util.ArraySet; import android.util.DisplayMetrics; import androidx.annotation.Nullable; +import androidx.test.InstrumentationRegistry; import androidx.test.filters.MediumTest; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -69,9 +73,11 @@ import org.junit.runner.RunWith; import java.io.File; 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.Collections; import java.util.HashSet; @@ -90,6 +96,9 @@ public class PackageParserTest { private File mTmpDir; private static final File FRAMEWORK = new File("/system/framework/framework-res.apk"); + private static final String TEST_APP1_APK = "PackageParserTestApp1.apk"; + private static final String TEST_APP2_APK = "PackageParserTestApp2.apk"; + private static final String TEST_APP3_APK = "PackageParserTestApp3.apk"; @Before public void setUp() throws IOException { @@ -209,6 +218,61 @@ public class PackageParserTest { assertSame(deserialized.getSharedUserId(), deserialized2.getSharedUserId()); } + private static PackageParser2 makeParser() { + return new PackageParser2(null, false, null, null, null); + } + + private File extractFile(String filename) throws Exception { + final Context context = InstrumentationRegistry.getTargetContext(); + final File tmpFile = File.createTempFile(filename, ".apk"); + try (InputStream inputStream = context.getAssets().openNonAsset(filename)) { + Files.copy(inputStream, tmpFile.toPath(), REPLACE_EXISTING); + } + return tmpFile; + } + + /** + * Tests AndroidManifest.xml with no android:isolatedSplits attribute. + */ + @Test + public void testParseIsolatedSplitsDefault() throws Exception { + final File testFile = extractFile(TEST_APP1_APK); + try { + final ParsedPackage pkg = makeParser().parsePackage(testFile, 0, false); + assertFalse("isolatedSplits", pkg.isIsolatedSplitLoading()); + } finally { + testFile.delete(); + } + } + + /** + * Tests AndroidManifest.xml with an android:isolatedSplits attribute set to a constant. + */ + @Test + public void testParseIsolatedSplitsConstant() throws Exception { + final File testFile = extractFile(TEST_APP2_APK); + try { + final ParsedPackage pkg = makeParser().parsePackage(testFile, 0, false); + assertTrue("isolatedSplits", pkg.isIsolatedSplitLoading()); + } finally { + testFile.delete(); + } + } + + /** + * Tests AndroidManifest.xml with an android:isolatedSplits attribute set to a resource. + */ + @Test + public void testParseIsolatedSplitsResource() throws Exception { + final File testFile = extractFile(TEST_APP3_APK); + try { + final ParsedPackage pkg = makeParser().parsePackage(testFile, 0, false); + assertTrue("isolatedSplits", pkg.isIsolatedSplitLoading()); + } finally { + testFile.delete(); + } + } + /** * A trivial subclass of package parser that only caches the package name, and throws away * all other information. diff --git a/services/tests/servicestests/test-apps/PackageParserApp/Android.bp b/services/tests/servicestests/test-apps/PackageParserApp/Android.bp new file mode 100644 index 0000000000000..c409438e94ae6 --- /dev/null +++ b/services/tests/servicestests/test-apps/PackageParserApp/Android.bp @@ -0,0 +1,53 @@ +// Copyright (C) 2020 The Android Open Source Project +// +// 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. + +android_test_helper_app { + name: "PackageParserTestApp1", + sdk_version: "current", + srcs: ["**/*.java"], + dex_preopt: { + enabled: false, + }, + optimize: { + enabled: false, + }, + manifest: "AndroidManifestApp1.xml", +} + +android_test_helper_app { + name: "PackageParserTestApp2", + sdk_version: "current", + srcs: ["**/*.java"], + dex_preopt: { + enabled: false, + }, + optimize: { + enabled: false, + }, + manifest: "AndroidManifestApp2.xml", +} + +android_test_helper_app { + name: "PackageParserTestApp3", + sdk_version: "current", + srcs: ["**/*.java"], + dex_preopt: { + enabled: false, + }, + optimize: { + enabled: false, + }, + resource_dirs: ["res"], + manifest: "AndroidManifestApp3.xml", +} diff --git a/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp1.xml b/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp1.xml new file mode 100644 index 0000000000000..01d335d2cbbd7 --- /dev/null +++ b/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp1.xml @@ -0,0 +1,25 @@ + + + + + + + + + + \ No newline at end of file diff --git a/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp2.xml b/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp2.xml new file mode 100644 index 0000000000000..567946cc70d77 --- /dev/null +++ b/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp2.xml @@ -0,0 +1,26 @@ + + + + + + + + + + \ No newline at end of file diff --git a/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp3.xml b/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp3.xml new file mode 100644 index 0000000000000..77285a805950f --- /dev/null +++ b/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp3.xml @@ -0,0 +1,26 @@ + + + + + + + + + + \ No newline at end of file diff --git a/services/tests/servicestests/test-apps/PackageParserApp/res/values/values.xml b/services/tests/servicestests/test-apps/PackageParserApp/res/values/values.xml new file mode 100644 index 0000000000000..6a4cc653494f7 --- /dev/null +++ b/services/tests/servicestests/test-apps/PackageParserApp/res/values/values.xml @@ -0,0 +1,19 @@ + + + + + true + \ No newline at end of file diff --git a/services/tests/servicestests/test-apps/PackageParserApp/src/com/android/servicestests/apps/packageparserapp/TestActivity.java b/services/tests/servicestests/test-apps/PackageParserApp/src/com/android/servicestests/apps/packageparserapp/TestActivity.java new file mode 100644 index 0000000000000..2eacb96fb2aa0 --- /dev/null +++ b/services/tests/servicestests/test-apps/PackageParserApp/src/com/android/servicestests/apps/packageparserapp/TestActivity.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * 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 com.android.servicestests.apps.packageparserapp; + +import android.app.Activity; +import android.os.Bundle; + +public class TestActivity extends Activity { + @Override + public void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + finish(); + } +}