Gate stricter manifest enforcement on targetSdk R
Two package parsing issues have been promoted to failures in R:
a missing <application>/<instrumentation> tag, and an empty
"android:name" attribute. The latter due to a bug in the parsing
code.
These need to be gated by targetSdkVersion so that APKs built for
previous versions can still scan/install properly. This change
introduces support for this through a framework that leverages
@ChangeId to introduce individually toggle-able errors, in
case a developer needs to install an app that isn't completely
migrated for a new SDK version yet.
The ignoreError method was removed from ParseResult as the errors
it was used for were manually compared to PackageParser and it's
likely they can be hard errors without breaking anything.
This also adds tests for general ParseInput/Result behavior.
Exempt-From-Owner-Approval: AppIntegrity already approved in older PS.
Bug: 150776642
Test: atest android.content.pm.parsing.result.ParseInputAndResultTest
Test: atest com.android.server.pm.parsing.PackageParsingDeferErrorTest
Test: atest com.android.server.integrity.AppIntegrityManagerServiceImplTest
Test: atest com.android.server.pm.parsing
Change-Id: Id53a2e65f6e5e4dee9a41cc77007275b3a220ac3
diff --git a/apct-tests/perftests/core/src/android/os/PackageParsingPerfTest.kt b/apct-tests/perftests/core/src/android/os/PackageParsingPerfTest.kt
index 9e46365..29721c5 100644
--- a/apct-tests/perftests/core/src/android/os/PackageParsingPerfTest.kt
+++ b/apct-tests/perftests/core/src/android/os/PackageParsingPerfTest.kt
@@ -22,6 +22,7 @@
import android.content.pm.parsing.ParsingPackageImpl
import android.content.pm.parsing.ParsingPackageRead
import android.content.pm.parsing.ParsingPackageUtils
+import android.content.pm.parsing.result.ParseInput
import android.content.pm.parsing.result.ParseTypeImpl
import android.content.res.TypedArray
import android.perftests.utils.BenchmarkState
@@ -173,7 +174,10 @@
class ParallelParser2(cacher: PackageCacher2? = null)
: ParallelParser<ParsingPackageRead>(cacher) {
- val input = ThreadLocal.withInitial { ParseTypeImpl() }
+ val input = ThreadLocal.withInitial {
+ // For testing, just disable enforcement to avoid hooking up to compat framework
+ ParseTypeImpl(ParseInput.Callback { _, _, _ -> false })
+ }
val parser = ParsingPackageUtils(false, null, null,
object : ParsingPackageUtils.Callback {
override fun hasFeature(feature: String) = true
diff --git a/core/java/android/content/pm/parsing/ParsingPackageUtils.java b/core/java/android/content/pm/parsing/ParsingPackageUtils.java
index e90ccdf..12328cf 100644
--- a/core/java/android/content/pm/parsing/ParsingPackageUtils.java
+++ b/core/java/android/content/pm/parsing/ParsingPackageUtils.java
@@ -65,7 +65,9 @@
import android.content.pm.parsing.component.ParsedService;
import android.content.pm.parsing.component.ParsedServiceUtils;
import android.content.pm.parsing.result.ParseInput;
+import android.content.pm.parsing.result.ParseInput.DeferredError;
import android.content.pm.parsing.result.ParseResult;
+import android.content.pm.parsing.result.ParseTypeImpl;
import android.content.pm.permission.SplitPermissionInfoParcelable;
import android.content.pm.split.DefaultSplitAssetLoader;
import android.content.pm.split.SplitAssetDependencyLoader;
@@ -126,6 +128,51 @@
public static final String TAG = ParsingUtils.TAG;
+ /**
+ * For cases outside of PackageManagerService when an APK needs to be parsed as a one-off
+ * request, without caching the input object and without querying the internal system state
+ * for feature support.
+ */
+ @NonNull
+ public static ParseResult<ParsingPackage> parseDefaultOneTime(File file, int flags,
+ @NonNull ParseInput.Callback inputCallback, @NonNull Callback callback) {
+ if ((flags & (PackageManager.MATCH_DIRECT_BOOT_UNAWARE
+ | PackageManager.MATCH_DIRECT_BOOT_AWARE)) == 0) {
+ // Caller expressed no opinion about what encryption
+ // aware/unaware components they want to see, so match both
+ flags |= PackageManager.MATCH_DIRECT_BOOT_AWARE
+ | PackageManager.MATCH_DIRECT_BOOT_UNAWARE;
+ }
+
+ ParseInput input = new ParseTypeImpl(inputCallback).reset();
+ ParseResult<ParsingPackage> result;
+
+
+ ParsingPackageUtils parser = new ParsingPackageUtils(false, null, null, callback);
+ try {
+ result = parser.parsePackage(input, file, flags);
+ if (result.isError()) {
+ return result;
+ }
+ } catch (PackageParser.PackageParserException e) {
+ return input.error(PackageManager.INSTALL_PARSE_FAILED_UNEXPECTED_EXCEPTION,
+ "Error parsing package", e);
+ }
+
+ try {
+ ParsingPackage pkg = result.getResult();
+ if ((flags & PackageManager.GET_SIGNATURES) != 0
+ || (flags & PackageManager.GET_SIGNING_CERTIFICATES) != 0) {
+ ParsingPackageUtils.collectCertificates(pkg, false /* skipVerify */);
+ }
+
+ return input.success(pkg);
+ } catch (PackageParser.PackageParserException e) {
+ return input.error(PackageManager.INSTALL_PARSE_FAILED_UNEXPECTED_EXCEPTION,
+ "Error collecting package certificates", e);
+ }
+ }
+
private boolean mOnlyCoreApps;
private String[] mSeparateProcesses;
private DisplayMetrics mDisplayMetrics;
@@ -456,10 +503,11 @@
}
if (!foundApp) {
- return input.error(
- PackageManager.INSTALL_PARSE_FAILED_MANIFEST_EMPTY,
- "<manifest> does not contain an <application>"
- );
+ ParseResult<?> deferResult = input.deferError(
+ "<manifest> does not contain an <application>", DeferredError.MISSING_APP_TAG);
+ if (deferResult.isError()) {
+ return input.error(deferResult);
+ }
}
return input.success(pkg);
@@ -663,10 +711,12 @@
}
if (!foundApp && ArrayUtils.size(pkg.getInstrumentations()) == 0) {
- return input.error(
- PackageManager.INSTALL_PARSE_FAILED_MANIFEST_EMPTY,
- "<manifest> does not contain an <application> or <instrumentation>"
- );
+ ParseResult<?> deferResult = input.deferError(
+ "<manifest> does not contain an <application> or <instrumentation>",
+ DeferredError.MISSING_APP_TAG);
+ if (deferResult.isError()) {
+ return input.error(deferResult);
+ }
}
if (!ParsedAttribution.isCombinationValid(pkg.getAttributions())) {
@@ -758,11 +808,9 @@
return input.success(pkg);
}
- ParseResult nameResult = validateName(input, str, true, true);
- if (nameResult.isError()) {
- if ("android".equals(pkg.getPackageName())) {
- nameResult.ignoreError();
- } else {
+ if (!"android".equals(pkg.getPackageName())) {
+ ParseResult<?> nameResult = validateName(input, str, true, true);
+ if (nameResult.isError()) {
return input.error(PackageManager.INSTALL_PARSE_FAILED_BAD_SHARED_USER_ID,
"<manifest> specifies bad sharedUserId name \"" + str + "\": "
+ nameResult.getErrorMessage());
@@ -1166,6 +1214,20 @@
targetCode = minCode;
}
+ ParseResult<Integer> targetSdkVersionResult = computeTargetSdkVersion(
+ targetVers, targetCode, PackageParser.SDK_CODENAMES, input);
+ if (targetSdkVersionResult.isError()) {
+ return input.error(targetSdkVersionResult);
+ }
+
+ int targetSdkVersion = targetSdkVersionResult.getResult();
+
+ ParseResult<?> deferResult =
+ input.enableDeferredError(pkg.getPackageName(), targetSdkVersion);
+ if (deferResult.isError()) {
+ return input.error(deferResult);
+ }
+
ParseResult<Integer> minSdkVersionResult = computeMinSdkVersion(minVers, minCode,
PackageParser.SDK_VERSION, PackageParser.SDK_CODENAMES, input);
if (minSdkVersionResult.isError()) {
@@ -1174,14 +1236,6 @@
int minSdkVersion = minSdkVersionResult.getResult();
- ParseResult<Integer> targetSdkVersionResult = computeTargetSdkVersion(
- targetVers, targetCode, PackageParser.SDK_CODENAMES, input);
- if (targetSdkVersionResult.isError()) {
- return input.error(targetSdkVersionResult);
- }
-
- int targetSdkVersion = minSdkVersionResult.getResult();
-
pkg.setMinSdkVersion(minSdkVersion)
.setTargetSdkVersion(targetSdkVersion);
@@ -1763,9 +1817,15 @@
// Add a hidden app detail activity to normal apps which forwards user to App Details
// page.
ParseResult<ParsedActivity> a = generateAppDetailsHiddenActivity(input, pkg);
- // Backwards-compat, assume success
+ if (a.isError()) {
+ // Error should be impossible here, as the only failure case as of SDK R is a
+ // string validation error on a constant ":app_details" string passed in by the
+ // parsing code itself. For this reason, this is just a hard failure instead of
+ // deferred.
+ return input.error(a);
+ }
+
pkg.addActivity(a.getResult());
- a.ignoreError();
}
if (hasActivityOrder) {
@@ -2122,18 +2182,14 @@
private static ParseResult<ParsedActivity> generateAppDetailsHiddenActivity(ParseInput input,
ParsingPackage pkg) {
String packageName = pkg.getPackageName();
- ParseResult<String> taskAffinityResult = ComponentParseUtils.buildTaskAffinityName(
+ ParseResult<String> result = ComponentParseUtils.buildTaskAffinityName(
packageName, packageName, ":app_details", input);
-
- String taskAffinity;
- if (taskAffinityResult.isSuccess()) {
- taskAffinity = taskAffinityResult.getResult();
- } else {
- // Backwards-compat, do not fail
- taskAffinity = null;
- taskAffinityResult.ignoreError();
+ if (result.isError()) {
+ return input.error(result);
}
+ String taskAffinity = result.getResult();
+
// Build custom App Details activity info instead of parsing it from xml
return input.success(ParsedActivity.makeAppDetailsActivity(packageName,
pkg.getProcessName(), pkg.getUiOptions(), taskAffinity,
@@ -2688,7 +2744,8 @@
public interface Callback {
boolean hasFeature(String feature);
- ParsingPackage startParsingPackage(String packageName, String baseCodePath, String codePath,
+ ParsingPackage startParsingPackage(@NonNull String packageName,
+ @NonNull String baseCodePath, @NonNull String codePath,
@NonNull TypedArray manifestArray, boolean isCoreApp);
}
}
diff --git a/core/java/android/content/pm/parsing/component/ParsedActivityUtils.java b/core/java/android/content/pm/parsing/component/ParsedActivityUtils.java
index 6e5c51a..f64560a 100644
--- a/core/java/android/content/pm/parsing/component/ParsedActivityUtils.java
+++ b/core/java/android/content/pm/parsing/component/ParsedActivityUtils.java
@@ -179,13 +179,12 @@
ParseResult<String> affinityNameResult = ComponentParseUtils.buildTaskAffinityName(
packageName, pkg.getTaskAffinity(), taskAffinity, input);
- if (affinityNameResult.isSuccess()) {
- activity.taskAffinity = affinityNameResult.getResult();
- } else {
- // Backwards-compat, ignore error
- affinityNameResult.ignoreError();
+ if (affinityNameResult.isError()) {
+ return input.error(affinityNameResult);
}
+ activity.taskAffinity = affinityNameResult.getResult();
+
boolean visibleToEphemeral = sa.getBoolean(R.styleable.AndroidManifestActivity_visibleToInstantApps, false);
if (visibleToEphemeral) {
activity.flags |= ActivityInfo.FLAG_VISIBLE_TO_INSTANT_APP;
diff --git a/core/java/android/content/pm/parsing/component/ParsedIntentInfoUtils.java b/core/java/android/content/pm/parsing/component/ParsedIntentInfoUtils.java
index a7b950b..390f769 100644
--- a/core/java/android/content/pm/parsing/component/ParsedIntentInfoUtils.java
+++ b/core/java/android/content/pm/parsing/component/ParsedIntentInfoUtils.java
@@ -29,7 +29,6 @@
import android.content.res.TypedArray;
import android.content.res.XmlResourceParser;
import android.os.PatternMatcher;
-import android.text.TextUtils;
import android.util.Slog;
import android.util.TypedValue;
@@ -97,8 +96,13 @@
case "action": {
String value = parser.getAttributeValue(PackageParser.ANDROID_RESOURCES,
"name");
- if (TextUtils.isEmpty(value)) {
+ if (value == null) {
result = input.error("No value supplied for <android:name>");
+ } else if (value.isEmpty()) {
+ intentInfo.addAction(value);
+ // Prior to R, this was not a failure
+ result = input.deferError("No value supplied for <android:name>",
+ ParseInput.DeferredError.EMPTY_INTENT_ACTION_CATEGORY);
} else {
intentInfo.addAction(value);
result = input.success(null);
@@ -108,8 +112,13 @@
case "category": {
String value = parser.getAttributeValue(PackageParser.ANDROID_RESOURCES,
"name");
- if (TextUtils.isEmpty(value)) {
+ if (value == null) {
result = input.error("No value supplied for <android:name>");
+ } else if (value.isEmpty()) {
+ intentInfo.addCategory(value);
+ // Prior to R, this was not a failure
+ result = input.deferError("No value supplied for <android:name>",
+ ParseInput.DeferredError.EMPTY_INTENT_ACTION_CATEGORY);
} else {
intentInfo.addCategory(value);
result = input.success(null);
diff --git a/core/java/android/content/pm/parsing/component/ParsedMainComponentUtils.java b/core/java/android/content/pm/parsing/component/ParsedMainComponentUtils.java
index 6188f89..f4c9914 100644
--- a/core/java/android/content/pm/parsing/component/ParsedMainComponentUtils.java
+++ b/core/java/android/content/pm/parsing/component/ParsedMainComponentUtils.java
@@ -20,6 +20,9 @@
import android.annotation.Nullable;
import android.content.IntentFilter;
import android.content.pm.parsing.ParsingPackage;
+import android.content.pm.parsing.ParsingPackageUtils;
+import android.content.pm.parsing.result.ParseInput;
+import android.content.pm.parsing.result.ParseResult;
import android.content.res.Configuration;
import android.content.res.Resources;
import android.content.res.TypedArray;
@@ -28,9 +31,6 @@
import android.util.Slog;
import com.android.internal.annotations.VisibleForTesting;
-import android.content.pm.parsing.ParsingPackageUtils;
-import android.content.pm.parsing.result.ParseInput;
-import android.content.pm.parsing.result.ParseResult;
import org.xmlpull.v1.XmlPullParserException;
@@ -83,12 +83,11 @@
ParseResult<String> processNameResult = ComponentParseUtils.buildProcessName(
pkg.getPackageName(), pkg.getProcessName(), processName, flags,
separateProcesses, input);
- if (processNameResult.isSuccess()) {
- component.setProcessName(processNameResult.getResult());
- } else {
- // Backwards-compat, ignore error
- processNameResult.ignoreError();
+ if (processNameResult.isError()) {
+ return input.error(processNameResult);
}
+
+ component.setProcessName(processNameResult.getResult());
}
if (splitNameAttr != null) {
diff --git a/core/java/android/content/pm/parsing/result/ParseInput.java b/core/java/android/content/pm/parsing/result/ParseInput.java
index c468506..5385100 100644
--- a/core/java/android/content/pm/parsing/result/ParseInput.java
+++ b/core/java/android/content/pm/parsing/result/ParseInput.java
@@ -16,10 +16,12 @@
package android.content.pm.parsing.result;
-import android.annotation.Hide;
import android.annotation.NonNull;
import android.annotation.Nullable;
+import android.compat.annotation.ChangeId;
+import android.compat.annotation.EnabledAfter;
import android.content.pm.PackageManager;
+import android.os.Build;
/**
* Used as a method parameter which is then transformed into a {@link ParseResult}. This is
@@ -30,8 +32,52 @@
*/
public interface ParseInput {
+ /**
+ * Errors encountered during parsing may rely on the targetSDK version of the application to
+ * determine whether or not to fail. These are passed into {@link #deferError(String, long)}
+ * when encountered, and the implementation will handle how to defer the errors until the
+ * targetSdkVersion is known and sent to {@link #enableDeferredError(String, int)}.
+ *
+ * All of these must be marked {@link ChangeId}, as that is the mechanism used to check if the
+ * error must be propagated. This framework also allows developers to pre-disable specific
+ * checks if they wish to target a newer SDK version in a development environment without
+ * having to migrate their entire app to validate on a newer platform.
+ */
+ final class DeferredError {
+ /**
+ * Missing an "application" or "instrumentation" tag.
+ */
+ @ChangeId
+ @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.Q)
+ public static final long MISSING_APP_TAG = 150776642;
+
+ /**
+ * An intent filter's actor or category is an empty string. A bug in the platform before R
+ * allowed this to pass through without an error. This does not include cases when the
+ * attribute is null/missing, as that has always been a failure.
+ */
+ @ChangeId
+ @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.Q)
+ public static final long EMPTY_INTENT_ACTION_CATEGORY = 151163173;
+ }
+
<ResultType> ParseResult<ResultType> success(ResultType result);
+ /**
+ * Used for errors gated by {@link DeferredError}. Will return an error result if the
+ * targetSdkVersion is already known and this must be returned as a real error. The result
+ * contains null and should not be unwrapped.
+ *
+ * @see #error(String)
+ */
+ ParseResult<?> deferError(@NonNull String parseError, long deferredError);
+
+ /**
+ * Called after targetSdkVersion is known. Returns an error result if a previously deferred
+ * error was registered. The result contains null and should not be unwrapped.
+ */
+ ParseResult<?> enableDeferredError(String packageName, int targetSdkVersion);
+
/** @see #error(int, String, Exception) */
<ResultType> ParseResult<ResultType> error(int parseError);
@@ -52,9 +98,6 @@
* The calling site of that method is then expected to check the result for error, and
* continue to bubble up if it is an error.
*
- * Or, if the code explicitly handles an error,
- * {@link ParseResult#ignoreError()} should be called.
- *
* If the result {@link ParseResult#isSuccess()}, then it can be used as-is, as
* overlapping/consecutive successes are allowed.
*/
@@ -66,5 +109,17 @@
* but cast the type of the {@link ParseResult} for type safety, since the parameter
* and the receiver should be the same object.
*/
- <ResultType> ParseResult<ResultType> error(ParseResult result);
+ <ResultType> ParseResult<ResultType> error(ParseResult<?> result);
+
+ /**
+ * Implemented instead of a direct reference to
+ * {@link com.android.internal.compat.IPlatformCompat}, allowing caching and testing logic to
+ * be separated out.
+ */
+ interface Callback {
+ /**
+ * @return true if the changeId should be enabled
+ */
+ boolean isChangeEnabled(long changeId, @NonNull String packageName, int targetSdkVersion);
+ }
}
diff --git a/core/java/android/content/pm/parsing/result/ParseResult.java b/core/java/android/content/pm/parsing/result/ParseResult.java
index 338048c..518395d 100644
--- a/core/java/android/content/pm/parsing/result/ParseResult.java
+++ b/core/java/android/content/pm/parsing/result/ParseResult.java
@@ -17,32 +17,19 @@
package android.content.pm.parsing.result;
import android.annotation.Nullable;
-import android.content.pm.PackageParser;
/**
* The output side of {@link ParseInput}, which must result from a method call on
* {@link ParseInput}.
*
* When using this class, keep in mind that all {@link ParseInput}s and {@link ParseResult}s
- * are the exact same object, scoped to a per {@link PackageParser} instance, per thread basis,
- * thrown around and casted everywhere for type safety.
+ * are the exact same object, scoped per thread, thrown around and casted for type safety.
*
* @hide
*/
public interface ParseResult<ResultType> {
/**
- * Un-marks this result as an error, also allowing it to be re-used as {@link ParseInput}.
- *
- * This should only be used in cases where it's absolutely certain that error handling is
- * irrelevant. Such as for backwards compatibility where it previously didn't fail and that
- * behavior has to be maintained.
- *
- * Mostly an alias for readability.
- */
- void ignoreError();
-
- /**
* Returns true if the result is not an error and thus contains a valid object.
*
* For backwards-compat reasons, it's possible to have a successful result with a null
diff --git a/core/java/android/content/pm/parsing/result/ParseTypeImpl.java b/core/java/android/content/pm/parsing/result/ParseTypeImpl.java
index 9b22f09..b26bf71 100644
--- a/core/java/android/content/pm/parsing/result/ParseTypeImpl.java
+++ b/core/java/android/content/pm/parsing/result/ParseTypeImpl.java
@@ -20,51 +20,130 @@
import android.annotation.Nullable;
import android.content.pm.PackageManager;
import android.content.pm.parsing.ParsingUtils;
+import android.util.ArrayMap;
import android.util.Log;
+import android.util.Slog;
-import java.util.Arrays;
+import com.android.internal.util.CollectionUtils;
/** @hide */
public class ParseTypeImpl implements ParseInput, ParseResult<Object> {
private static final String TAG = ParsingUtils.TAG;
- private static final boolean DEBUG_FILL_STACK_TRACE = false;
+ public static final boolean DEBUG_FILL_STACK_TRACE = false;
- private static final boolean DEBUG_LOG_ON_ERROR = false;
+ public static final boolean DEBUG_LOG_ON_ERROR = false;
- private Object result;
+ public static final boolean DEBUG_THROW_ALL_ERRORS = false;
- private int errorCode = PackageManager.INSTALL_SUCCEEDED;
+ @NonNull
+ private Callback mCallback;
+
+ private Object mResult;
+
+ private int mErrorCode = PackageManager.INSTALL_SUCCEEDED;
@Nullable
- private String errorMessage;
+ private String mErrorMessage;
@Nullable
- private Exception exception;
+ private Exception mException;
+
+ /**
+ * Errors encountered before targetSdkVersion is known.
+ * The size upper bound is the number of longs in {@link DeferredError}
+ */
+ @Nullable
+ private ArrayMap<Long, String> mDeferredErrors = null;
+
+ private String mPackageName;
+ private Integer mTargetSdkVersion;
+
+ /**
+ * @param callback if nullable, fallback to manual targetSdk > Q check
+ */
+ public ParseTypeImpl(@NonNull Callback callback) {
+ mCallback = callback;
+ }
public ParseInput reset() {
- this.result = null;
- this.errorCode = PackageManager.INSTALL_SUCCEEDED;
- this.errorMessage = null;
- this.exception = null;
+ mResult = null;
+ mErrorCode = PackageManager.INSTALL_SUCCEEDED;
+ mErrorMessage = null;
+ mException = null;
+ if (mDeferredErrors != null) {
+ // If the memory was already allocated, don't bother freeing and re-allocating,
+ // as this could occur hundreds of times depending on what the caller is doing and
+ // how many APKs they're going through.
+ mDeferredErrors.erase();
+ }
return this;
}
@Override
- public void ignoreError() {
- reset();
+ public <ResultType> ParseResult<ResultType> success(ResultType result) {
+ if (mErrorCode != PackageManager.INSTALL_SUCCEEDED) {
+ Slog.wtf(ParsingUtils.TAG, "Cannot set to success after set to error, was "
+ + mErrorMessage, mException);
+ }
+ mResult = result;
+ //noinspection unchecked
+ return (ParseResult<ResultType>) this;
}
@Override
- public <ResultType> ParseResult<ResultType> success(ResultType result) {
- if (errorCode != PackageManager.INSTALL_SUCCEEDED || errorMessage != null) {
- throw new IllegalStateException("Cannot set to success after set to error, was "
- + errorMessage, exception);
+ public ParseResult<?> deferError(@NonNull String parseError, long deferredError) {
+ if (DEBUG_THROW_ALL_ERRORS) {
+ return error(parseError);
}
- this.result = result;
- //noinspection unchecked
- return (ParseResult<ResultType>) this;
+ if (mTargetSdkVersion != null) {
+ if (mDeferredErrors != null && mDeferredErrors.containsKey(deferredError)) {
+ // If the map already contains the key, that means it's already been checked and
+ // found to be disabled. Otherwise it would've failed when mTargetSdkVersion was
+ // set to non-null.
+ return success(null);
+ }
+
+ if (mCallback.isChangeEnabled(deferredError, mPackageName, mTargetSdkVersion)) {
+ return error(parseError);
+ } else {
+ if (mDeferredErrors == null) {
+ mDeferredErrors = new ArrayMap<>();
+ }
+ mDeferredErrors.put(deferredError, null);
+ return success(null);
+ }
+ }
+
+ if (mDeferredErrors == null) {
+ mDeferredErrors = new ArrayMap<>();
+ }
+
+ // Only save the first occurrence of any particular error
+ mDeferredErrors.putIfAbsent(deferredError, parseError);
+ return success(null);
+ }
+
+ @Override
+ public ParseResult<?> enableDeferredError(String packageName, int targetSdkVersion) {
+ mPackageName = packageName;
+ mTargetSdkVersion = targetSdkVersion;
+
+ int size = CollectionUtils.size(mDeferredErrors);
+ for (int index = size - 1; index >= 0; index--) {
+ long changeId = mDeferredErrors.keyAt(index);
+ String errorMessage = mDeferredErrors.valueAt(index);
+ if (mCallback.isChangeEnabled(changeId, mPackageName, mTargetSdkVersion)) {
+ return error(errorMessage);
+ } else {
+ // No point holding onto the string, but need to maintain the key to signal
+ // that the error was checked with isChangeEnabled and found to be disabled.
+ mDeferredErrors.setValueAt(index, null);
+ }
+ }
+
+ return success(null);
}
@Override
@@ -84,25 +163,26 @@
}
@Override
- public <ResultType> ParseResult<ResultType> error(ParseResult intentResult) {
- return error(intentResult.getErrorCode(), intentResult.getErrorMessage());
+ public <ResultType> ParseResult<ResultType> error(ParseResult<?> intentResult) {
+ return error(intentResult.getErrorCode(), intentResult.getErrorMessage(),
+ intentResult.getException());
}
@Override
public <ResultType> ParseResult<ResultType> error(int errorCode, @Nullable String errorMessage,
Exception exception) {
- this.errorCode = errorCode;
- this.errorMessage = errorMessage;
- this.exception = exception;
+ mErrorCode = errorCode;
+ mErrorMessage = errorMessage;
+ mException = exception;
if (DEBUG_FILL_STACK_TRACE) {
if (exception == null) {
- this.exception = new Exception();
+ mException = new Exception();
}
}
if (DEBUG_LOG_ON_ERROR) {
- Exception exceptionToLog = this.exception != null ? this.exception : new Exception();
+ Exception exceptionToLog = mException != null ? mException : new Exception();
Log.w(TAG, "ParseInput set to error " + errorCode + ", " + errorMessage,
exceptionToLog);
}
@@ -113,12 +193,12 @@
@Override
public Object getResult() {
- return this.result;
+ return mResult;
}
@Override
public boolean isSuccess() {
- return errorCode == PackageManager.INSTALL_SUCCEEDED;
+ return mErrorCode == PackageManager.INSTALL_SUCCEEDED;
}
@Override
@@ -128,18 +208,18 @@
@Override
public int getErrorCode() {
- return errorCode;
+ return mErrorCode;
}
@Nullable
@Override
public String getErrorMessage() {
- return errorMessage;
+ return mErrorMessage;
}
@Nullable
@Override
public Exception getException() {
- return exception;
+ return mException;
}
}
diff --git a/core/tests/coretests/src/android/content/pm/parsing/result/ParseInputAndResultTest.kt b/core/tests/coretests/src/android/content/pm/parsing/result/ParseInputAndResultTest.kt
new file mode 100644
index 0000000..d45fee9
--- /dev/null
+++ b/core/tests/coretests/src/android/content/pm/parsing/result/ParseInputAndResultTest.kt
@@ -0,0 +1,281 @@
+/*
+ * 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 android.content.pm.parsing.result
+
+import android.content.pm.PackageManager
+import android.os.Build
+import com.google.common.truth.Truth.assertThat
+import org.junit.After
+import org.junit.Assume.assumeFalse
+import org.junit.Before
+import org.junit.BeforeClass
+import org.junit.Test
+import org.mockito.ArgumentMatchers.anyLong
+import org.mockito.Mockito.anyInt
+import org.mockito.Mockito.anyString
+import org.mockito.Mockito.never
+import org.mockito.Mockito.spy
+import org.mockito.Mockito.times
+import org.mockito.Mockito.verify
+import org.mockito.Mockito.verifyNoMoreInteractions
+import java.io.IOException
+
+class ParseInputAndResultTest {
+
+ companion object {
+
+ private const val TEST_PACKAGE = "com.android.test"
+
+ private const val ENABLED_ERROR = 11L
+ private const val DISABLED_ERROR = 22L
+
+ @JvmStatic
+ @BeforeClass
+ fun assumeNotDebug() {
+ // None of these tests consider cases where debugging logic is enabled
+ assumeFalse(ParseTypeImpl.DEBUG_FILL_STACK_TRACE)
+ assumeFalse(ParseTypeImpl.DEBUG_LOG_ON_ERROR)
+ assumeFalse(ParseTypeImpl.DEBUG_THROW_ALL_ERRORS)
+ }
+ }
+
+ private lateinit var mockCallback: ParseInput.Callback
+ private lateinit var input: ParseInput
+
+ @Before
+ fun createInput() {
+ // Use an open class instead off a lambda so it can be spied
+ open class TestCallback : ParseInput.Callback {
+ override fun isChangeEnabled(changeId: Long, pkgName: String, targetSdk: Int): Boolean {
+ return when (changeId) {
+ ENABLED_ERROR -> targetSdk > Build.VERSION_CODES.Q
+ DISABLED_ERROR -> false
+ else -> throw IllegalStateException("changeId $changeId is not mocked for test")
+ }
+ }
+ }
+
+ mockCallback = spy(TestCallback())
+ input = ParseTypeImpl(mockCallback)
+ }
+
+ @Test
+ fun errorCode() {
+ val errorCode = PackageManager.INSTALL_FAILED_UPDATE_INCOMPATIBLE
+ val result = input.error<Any?>(errorCode)
+ assertError(result)
+ assertThat(result.errorCode).isEqualTo(errorCode)
+ assertThat(result.errorMessage).isNull()
+ assertThat(result.exception).isNull()
+ }
+
+ @Test
+ fun errorMessage() {
+ val errorMessage = "Test error"
+ val result = input.error<Any?>(errorMessage)
+ assertError(result)
+ assertThat(result.errorCode).isNotEqualTo(PackageManager.INSTALL_SUCCEEDED)
+ assertThat(result.errorMessage).isEqualTo(errorMessage)
+ assertThat(result.exception).isNull()
+ }
+
+ @Test
+ fun errorCodeAndMessage() {
+ val errorCode = PackageManager.INSTALL_FAILED_UPDATE_INCOMPATIBLE
+ val errorMessage = "Test error"
+ val result = input.error<Any?>(errorCode, errorMessage)
+ assertError(result)
+ assertThat(result.errorCode).isEqualTo(errorCode)
+ assertThat(result.errorMessage).isEqualTo(errorMessage)
+ assertThat(result.exception).isNull()
+ }
+
+ @Test
+ fun errorCodeAndMessageAndException() {
+ val errorCode = PackageManager.INSTALL_FAILED_UPDATE_INCOMPATIBLE
+ val errorMessage = "Test error"
+ val exception = IOException()
+ val result = input.error<Any?>(errorCode, errorMessage, exception)
+ assertError(result)
+ assertThat(result.errorCode).isEqualTo(errorCode)
+ assertThat(result.errorMessage).isEqualTo(errorMessage)
+ assertThat(result.exception).isSameAs(exception)
+ }
+
+ @Test
+ fun errorCarryResult() {
+ val errorCode = PackageManager.INSTALL_FAILED_UPDATE_INCOMPATIBLE
+ val errorMessage = "Test error"
+ val exception = IOException()
+ val result = input.error<Any?>(errorCode, errorMessage, exception)
+ assertError(result)
+ assertThat(result.errorCode).isEqualTo(errorCode)
+ assertThat(result.errorMessage).isEqualTo(errorMessage)
+ assertThat(result.exception).isSameAs(exception)
+
+ val carriedResult = input.error<Int>(result)
+ assertError(carriedResult)
+ assertThat(carriedResult.errorCode).isEqualTo(errorCode)
+ assertThat(carriedResult.errorMessage).isEqualTo(errorMessage)
+ assertThat(carriedResult.exception).isSameAs(exception)
+ }
+
+ @Test
+ fun success() {
+ val value = "Test success"
+ assertSuccess(value, input.success(value))
+ }
+
+ @Test
+ fun deferErrorEnableFirstSdkQ() {
+ assertSuccess(input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.Q))
+
+ assertSuccess(input.deferError("Test error", ENABLED_ERROR))
+ }
+
+ @Test
+ fun deferErrorEnableLastSdkQ() {
+ assertSuccess(input.deferError("Test error", ENABLED_ERROR))
+
+ assertSuccess(input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.Q))
+ }
+
+ @Test
+ fun deferErrorEnableFirstSdkR() {
+ val error = "Test error"
+ assertSuccess(input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.R))
+
+ val deferResult = input.deferError(error, ENABLED_ERROR)
+ assertError(deferResult)
+ assertThat(deferResult.errorCode).isNotEqualTo(PackageManager.INSTALL_SUCCEEDED)
+ assertThat(deferResult.errorMessage).isEqualTo(error)
+ assertThat(deferResult.exception).isNull()
+ }
+
+ @Test
+ fun deferErrorEnableLastSdkR() {
+ val error = "Test error"
+ assertSuccess(input.deferError(error, ENABLED_ERROR))
+
+ val result = input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.R)
+ assertError(result)
+ assertThat(result.errorCode).isNotEqualTo(PackageManager.INSTALL_SUCCEEDED)
+ assertThat(result.errorMessage).isEqualTo(error)
+ assertThat(result.exception).isNull()
+ }
+
+ @Test
+ fun enableDeferredErrorAndSuccessSdkQ() {
+ val value = "Test success"
+ assertSuccess(input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.Q))
+
+ assertSuccess(value, input.success(value))
+ }
+
+ @Test
+ fun enableDeferredErrorAndSuccessSdkR() {
+ val value = "Test success"
+ assertSuccess(input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.R))
+
+ assertSuccess(value, input.success(value))
+ }
+
+ @Test
+ fun multipleDeferErrorKeepsFirst() {
+ val errorOne = "Test error one"
+ val errorTwo = "Test error two"
+
+ assertSuccess(input.deferError(errorOne, ENABLED_ERROR))
+ assertSuccess(input.deferError(errorTwo, ENABLED_ERROR))
+
+ val result = input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.R)
+ assertError(result)
+ assertThat(result.errorCode).isNotEqualTo(PackageManager.INSTALL_SUCCEEDED)
+ assertThat(result.errorMessage).isEqualTo(errorOne)
+ assertThat(result.exception).isNull()
+ }
+
+ @Test
+ fun multipleDisabledErrorsQueriesOnceEnableFirst() {
+ val errorOne = "Test error one"
+ val errorTwo = "Test error two"
+
+ assertSuccess(input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.R))
+
+ assertSuccess(input.deferError(errorOne, DISABLED_ERROR))
+
+ verify(mockCallback, times(1)).isChangeEnabled(anyLong(), anyString(), anyInt())
+
+ assertSuccess(input.deferError(errorTwo, DISABLED_ERROR))
+
+ verifyNoMoreInteractions(mockCallback)
+ }
+
+ @Test
+ fun multipleDisabledErrorsQueriesOnceEnableSecond() {
+ val errorOne = "Test error one"
+ val errorTwo = "Test error two"
+
+ assertSuccess(input.deferError(errorOne, DISABLED_ERROR))
+
+ verify(mockCallback, never()).isChangeEnabled(anyLong(), anyString(), anyInt())
+
+ assertSuccess(input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.R))
+
+ verify(mockCallback, times(1)).isChangeEnabled(anyLong(), anyString(), anyInt())
+
+ assertSuccess(input.deferError(errorTwo, DISABLED_ERROR))
+
+ verifyNoMoreInteractions(mockCallback)
+ }
+
+ @After
+ fun verifyReset() {
+ var result = (input as ParseTypeImpl).reset() as ParseResult<*>
+ result.assertReset()
+
+ // The deferred error is not directly accessible, so attempt to re-enable the deferred
+ // error and assert it was also reset.
+ result = input.enableDeferredError(TEST_PACKAGE, Build.VERSION_CODES.R)
+ result.assertReset()
+ }
+
+ private fun assertSuccess(result: ParseResult<*>) = assertSuccess(null, result)
+
+ private fun assertSuccess(expected: Any? = null, result: ParseResult<*>) {
+ assertThat(result.isError).isFalse()
+ assertThat(result.isSuccess).isTrue()
+ assertThat(result.result).isSameAs(expected)
+ assertThat(result.errorCode).isEqualTo(PackageManager.INSTALL_SUCCEEDED)
+ assertThat(result.errorMessage).isNull()
+ assertThat(result.exception).isNull()
+ }
+
+ private fun assertError(result: ParseResult<*>) {
+ assertThat(result.isError).isTrue()
+ assertThat(result.isSuccess).isFalse()
+ assertThat(result.result).isNull()
+ }
+
+ private fun ParseResult<*>.assertReset() {
+ assertThat(this.isSuccess).isTrue()
+ assertThat(this.isError).isFalse()
+ assertThat(this.errorCode).isEqualTo(PackageManager.INSTALL_SUCCEEDED)
+ assertThat(this.errorMessage).isNull()
+ assertThat(this.exception).isNull()
+ }
+}
diff --git a/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java b/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java
index 44ab438..273fd48 100644
--- a/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java
+++ b/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java
@@ -91,6 +91,7 @@
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
+import java.util.function.Supplier;
/** Implementation of {@link AppIntegrityManagerService}. */
public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
@@ -125,6 +126,7 @@
private final Context mContext;
private final Handler mHandler;
private final PackageManagerInternal mPackageManagerInternal;
+ private final Supplier<PackageParser2> mParserSupplier;
private final RuleEvaluationEngine mEvaluationEngine;
private final IntegrityFileManager mIntegrityFileManager;
@@ -136,6 +138,7 @@
return new AppIntegrityManagerServiceImpl(
context,
LocalServices.getService(PackageManagerInternal.class),
+ PackageParser2::forParsingFileWithDefaults,
RuleEvaluationEngine.getRuleEvaluationEngine(),
IntegrityFileManager.getInstance(),
handlerThread.getThreadHandler());
@@ -145,11 +148,13 @@
AppIntegrityManagerServiceImpl(
Context context,
PackageManagerInternal packageManagerInternal,
+ Supplier<PackageParser2> parserSupplier,
RuleEvaluationEngine evaluationEngine,
IntegrityFileManager integrityFileManager,
Handler handler) {
mContext = context;
mPackageManagerInternal = packageManagerInternal;
+ mParserSupplier = parserSupplier;
mEvaluationEngine = evaluationEngine;
mIntegrityFileManager = integrityFileManager;
mHandler = handler;
@@ -562,8 +567,7 @@
throw new IllegalArgumentException("Installation path is null, package not found");
}
- PackageParser2 parser = new PackageParser2(null, false, null, null, null);
- try {
+ try (PackageParser2 parser = mParserSupplier.get()) {
ParsedPackage pkg = parser.parsePackage(installationPath, 0, false);
int flags = PackageManager.GET_SIGNING_CERTIFICATES | PackageManager.GET_META_DATA;
pkg.setSigningDetails(ParsingPackageUtils.collectCertificates(pkg, false));
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 414449d..8850f29 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -1035,13 +1035,7 @@
private final AppsFilter mAppsFilter;
- class PackageParserCallback extends PackageParser2.Callback {
- @Override public final boolean hasFeature(String feature) {
- return PackageManagerService.this.hasSystemFeature(feature, 0);
- }
- }
-
- final PackageParser2.Callback mPackageParserCallback = new PackageParserCallback();
+ final PackageParser2.Callback mPackageParserCallback;
// Currently known shared libraries.
final ArrayMap<String, LongSparseArray<SharedLibraryInfo>> mSharedLibraries = new ArrayMap<>();
@@ -2720,6 +2714,18 @@
mPermissionManagerService = (IPermissionManager) ServiceManager.getService("permissionmgr");
mIncrementalManager =
(IncrementalManager) mContext.getSystemService(Context.INCREMENTAL_SERVICE);
+ PlatformCompat platformCompat = mInjector.getCompatibility();
+ mPackageParserCallback = new PackageParser2.Callback() {
+ @Override
+ public boolean isChangeEnabled(long changeId, @NonNull ApplicationInfo appInfo) {
+ return platformCompat.isChangeEnabled(changeId, appInfo);
+ }
+
+ @Override
+ public boolean hasFeature(String feature) {
+ return PackageManagerService.this.hasSystemFeature(feature, 0);
+ }
+ };
// CHECKSTYLE:ON IndentationCheck
t.traceEnd();
@@ -3075,6 +3081,8 @@
}
+ packageParser.close();
+
List<Runnable> unfinishedTasks = executorService.shutdownNow();
if (!unfinishedTasks.isEmpty()) {
throw new IllegalStateException("Not all tasks finished before calling close: "
@@ -8901,12 +8909,11 @@
private AndroidPackage scanPackageLI(File scanFile, int parseFlags, int scanFlags,
long currentTime, UserHandle user) throws PackageManagerException {
if (DEBUG_INSTALL) Slog.d(TAG, "Parsing: " + scanFile);
- PackageParser2 pp = new PackageParser2(mSeparateProcesses, mOnlyCore, mMetrics, null,
- mPackageParserCallback);
Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "parsePackage");
final ParsedPackage parsedPackage;
- try {
+ try (PackageParser2 pp = new PackageParser2(mSeparateProcesses, mOnlyCore, mMetrics, null,
+ mPackageParserCallback)) {
parsedPackage = pp.parsePackage(scanFile, parseFlags, false);
} catch (PackageParserException e) {
throw PackageManagerException.from(e);
@@ -16676,12 +16683,10 @@
| PackageParser.PARSE_ENFORCE_CODE
| (onExternal ? PackageParser.PARSE_EXTERNAL_STORAGE : 0);
- PackageParser2 pp = new PackageParser2(mSeparateProcesses, false, mMetrics, null,
- mPackageParserCallback);
-
Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "parsePackage");
ParsedPackage parsedPackage;
- try {
+ try (PackageParser2 pp = new PackageParser2(mSeparateProcesses, false, mMetrics, null,
+ mPackageParserCallback)) {
parsedPackage = pp.parsePackage(tmpPackageFile, parseFlags, false);
AndroidPackageUtils.validatePackageDexMetadata(parsedPackage);
} catch (PackageParserException e) {
@@ -16746,8 +16751,6 @@
"Instant app package must be signed with APK Signature Scheme v2 or greater");
}
- // Get rid of all references to package scan path via parser.
- pp = null;
boolean systemApp = false;
boolean replace = false;
synchronized (mLock) {
diff --git a/services/core/java/com/android/server/pm/parsing/PackageParser2.java b/services/core/java/com/android/server/pm/parsing/PackageParser2.java
index d561b9c..e860c48 100644
--- a/services/core/java/com/android/server/pm/parsing/PackageParser2.java
+++ b/services/core/java/com/android/server/pm/parsing/PackageParser2.java
@@ -17,7 +17,10 @@
package com.android.server.pm.parsing;
import android.annotation.AnyThread;
+import android.annotation.NonNull;
import android.annotation.Nullable;
+import android.content.Context;
+import android.content.pm.ApplicationInfo;
import android.content.pm.PackageParser;
import android.content.pm.PackageParser.PackageParserException;
import android.content.pm.parsing.ParsingPackage;
@@ -27,10 +30,13 @@
import android.content.pm.parsing.result.ParseTypeImpl;
import android.content.res.TypedArray;
import android.os.Build;
+import android.os.ServiceManager;
import android.os.SystemClock;
import android.util.DisplayMetrics;
import android.util.Slog;
+import com.android.server.compat.PlatformCompat;
+import com.android.server.pm.PackageManagerService;
import com.android.server.pm.parsing.pkg.PackageImpl;
import com.android.server.pm.parsing.pkg.ParsedPackage;
@@ -39,19 +45,53 @@
/**
* The v2 of {@link PackageParser} for use when parsing is initiated in the server and must
* contain state contained by the server.
+ *
+ * The {@link AutoCloseable} helps signal that this class contains resources that must be freed.
+ * Although it is sufficient to release references to an instance of this class and let it get
+ * collected automatically.
*/
-public class PackageParser2 {
+public class PackageParser2 implements AutoCloseable {
+
+ /**
+ * For parsing inside the system server but outside of {@link PackageManagerService}.
+ * Generally used for parsing information in an APK that hasn't been installed yet.
+ *
+ * This must be called inside the system process as it relies on {@link ServiceManager}.
+ */
+ public static PackageParser2 forParsingFileWithDefaults() {
+ PlatformCompat platformCompat =
+ (PlatformCompat) ServiceManager.getService(Context.PLATFORM_COMPAT_SERVICE);
+ return new PackageParser2(null /* separateProcesses */, false /* onlyCoreApps */,
+ null /* displayMetrics */, null /* cacheDir */, new Callback() {
+ @Override
+ public boolean isChangeEnabled(long changeId, @NonNull ApplicationInfo appInfo) {
+ return platformCompat.isChangeEnabled(changeId, appInfo);
+ }
+
+ @Override
+ public boolean hasFeature(String feature) {
+ // 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.
+ return false;
+ }
+ });
+ }
static final String TAG = "PackageParser2";
private static final boolean LOG_PARSE_TIMINGS = Build.IS_DEBUGGABLE;
private static final int LOG_PARSE_TIMINGS_THRESHOLD_MS = 100;
- private ThreadLocal<ParseTypeImpl> mSharedResult = ThreadLocal.withInitial(ParseTypeImpl::new);
+ private ThreadLocal<ApplicationInfo> mSharedAppInfo =
+ ThreadLocal.withInitial(() -> {
+ ApplicationInfo appInfo = new ApplicationInfo();
+ appInfo.uid = -1; // Not a valid UID since the app will not be installed yet
+ return appInfo;
+ });
- private final String[] mSeparateProcesses;
- private final boolean mOnlyCoreApps;
- private final DisplayMetrics mDisplayMetrics;
+ private ThreadLocal<ParseTypeImpl> mSharedResult;
@Nullable
protected PackageCacher mCacher;
@@ -64,27 +104,26 @@
* creating a minimalist boot environment.
*/
public PackageParser2(String[] separateProcesses, boolean onlyCoreApps,
- DisplayMetrics displayMetrics, @Nullable File cacheDir, Callback callback) {
- mSeparateProcesses = separateProcesses;
- mOnlyCoreApps = onlyCoreApps;
-
+ DisplayMetrics displayMetrics, @Nullable File cacheDir, @NonNull Callback callback) {
if (displayMetrics == null) {
- mDisplayMetrics = new DisplayMetrics();
- mDisplayMetrics.setToDefaults();
- } else {
- mDisplayMetrics = displayMetrics;
+ displayMetrics = new DisplayMetrics();
+ displayMetrics.setToDefaults();
}
mCacher = cacheDir == null ? null : new PackageCacher(cacheDir);
- // TODO(b/135203078): Remove nullability of callback
- callback = callback != null ? callback : new Callback() {
- @Override
- public boolean hasFeature(String feature) {
- return false;
- }
+
+ parsingUtils = new ParsingPackageUtils(onlyCoreApps, separateProcesses, displayMetrics,
+ callback);
+
+ ParseInput.Callback enforcementCallback = (changeId, packageName, targetSdkVersion) -> {
+ ApplicationInfo appInfo = mSharedAppInfo.get();
+ //noinspection ConstantConditions
+ appInfo.packageName = packageName;
+ appInfo.targetSdkVersion = targetSdkVersion;
+ return callback.isChangeEnabled(changeId, appInfo);
};
- parsingUtils = new ParsingPackageUtils(onlyCoreApps, separateProcesses, displayMetrics, callback);
+ mSharedResult = ThreadLocal.withInitial(() -> new ParseTypeImpl(enforcementCallback));
}
/**
@@ -126,13 +165,38 @@
return parsed;
}
+ /**
+ * Removes the cached value for the thread the parser was created on. It is assumed that
+ * any threads created for parallel parsing will be created and released, so they don't
+ * need an explicit close call.
+ *
+ * Realistically an instance should never be retained, so when the enclosing class is released,
+ * the values will also be released, making this method unnecessary.
+ */
+ @Override
+ public void close() {
+ mSharedResult.remove();
+ mSharedAppInfo.remove();
+ }
+
public static abstract class Callback implements ParsingPackageUtils.Callback {
@Override
- public final ParsingPackage startParsingPackage(String packageName, String baseCodePath,
- String codePath, TypedArray manifestArray, boolean isCoreApp) {
+ public final ParsingPackage startParsingPackage(@NonNull String packageName,
+ @NonNull String baseCodePath, @NonNull String codePath,
+ @NonNull TypedArray manifestArray, boolean isCoreApp) {
return PackageImpl.forParsing(packageName, baseCodePath, codePath, manifestArray,
isCoreApp);
}
+
+ /**
+ * An indirection from {@link ParseInput.Callback#isChangeEnabled(long, String, int)},
+ * allowing the {@link ApplicationInfo} objects to be cached in {@link #mSharedAppInfo}
+ * and cleaned up with the parser instance, not the callback instance.
+ *
+ * @param appInfo will only have 3 fields filled in, {@link ApplicationInfo#packageName},
+ * {@link ApplicationInfo#targetSdkVersion}, and {@link ApplicationInfo#uid}
+ */
+ public abstract boolean isChangeEnabled(long changeId, @NonNull ApplicationInfo appInfo);
}
}
diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkQ.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkQ.apk
new file mode 100644
index 0000000..841fceb
--- /dev/null
+++ b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkQ.apk
Binary files differ
diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkR.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkR.apk
new file mode 100644
index 0000000..3520650
--- /dev/null
+++ b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkR.apk
Binary files differ
diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkQ.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkQ.apk
new file mode 100644
index 0000000..6774d5f
--- /dev/null
+++ b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkQ.apk
Binary files differ
diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkR.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkR.apk
new file mode 100644
index 0000000..23efb19
--- /dev/null
+++ b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkR.apk
Binary files differ
diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkQ.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkQ.apk
new file mode 100644
index 0000000..f684f86
--- /dev/null
+++ b/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkQ.apk
Binary files differ
diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkR.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkR.apk
new file mode 100644
index 0000000..491c56c
--- /dev/null
+++ b/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkR.apk
Binary files differ
diff --git a/services/tests/servicestests/src/com/android/server/integrity/AppIntegrityManagerServiceImplTest.java b/services/tests/servicestests/src/com/android/server/integrity/AppIntegrityManagerServiceImplTest.java
index d3a3e7e..e5741ee 100644
--- a/services/tests/servicestests/src/com/android/server/integrity/AppIntegrityManagerServiceImplTest.java
+++ b/services/tests/servicestests/src/com/android/server/integrity/AppIntegrityManagerServiceImplTest.java
@@ -67,8 +67,11 @@
import androidx.test.InstrumentationRegistry;
import com.android.internal.R;
+import com.android.server.compat.PlatformCompat;
import com.android.server.integrity.engine.RuleEvaluationEngine;
import com.android.server.integrity.model.IntegrityCheckResult;
+import com.android.server.pm.parsing.PackageParser2;
+import com.android.server.pm.parsing.TestPackageParser2;
import com.android.server.testutils.TestUtils;
import org.junit.After;
@@ -88,6 +91,7 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
+import java.util.function.Supplier;
/** Unit test for {@link com.android.server.integrity.AppIntegrityManagerServiceImpl} */
@RunWith(JUnit4.class)
@@ -131,12 +135,15 @@
@org.junit.Rule public MockitoRule mMockitoRule = MockitoJUnit.rule();
@Mock PackageManagerInternal mPackageManagerInternal;
+ @Mock PlatformCompat mPlatformCompat;
@Mock Context mMockContext;
@Mock Resources mMockResources;
@Mock RuleEvaluationEngine mRuleEvaluationEngine;
@Mock IntegrityFileManager mIntegrityFileManager;
@Mock Handler mHandler;
+ private Supplier<PackageParser2> mParserSupplier = TestPackageParser2::new;
+
private final Context mRealContext = InstrumentationRegistry.getTargetContext();
private PackageManager mSpyPackageManager;
@@ -168,6 +175,7 @@
new AppIntegrityManagerServiceImpl(
mMockContext,
mPackageManagerInternal,
+ mParserSupplier,
mRuleEvaluationEngine,
mIntegrityFileManager,
mHandler);
diff --git a/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java
index 73e5b6d..3718c5c 100644
--- a/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java
@@ -46,6 +46,7 @@
import com.android.frameworks.servicestests.R;
import com.android.server.pm.parsing.PackageParser2;
+import com.android.server.pm.parsing.TestPackageParser2;
import org.junit.Before;
import org.junit.Test;
@@ -77,7 +78,7 @@
ApexManager.ApexManagerImpl managerImpl = spy(new ApexManager.ApexManagerImpl());
doReturn(mApexService).when(managerImpl).waitForApexService();
mApexManager = managerImpl;
- mPackageParser2 = new PackageParser2(null, false, null, null, null);
+ mPackageParser2 = new TestPackageParser2();
}
@Test
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 d760629..daaf870 100644
--- a/services/tests/servicestests/src/com/android/server/pm/PackageParserTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/PackageParserTest.java
@@ -49,7 +49,6 @@
import android.os.Parcel;
import android.platform.test.annotations.Presubmit;
import android.util.ArraySet;
-import android.util.DisplayMetrics;
import androidx.annotation.Nullable;
import androidx.test.InstrumentationRegistry;
@@ -61,6 +60,7 @@
import com.android.server.pm.parsing.PackageCacher;
import com.android.server.pm.parsing.PackageInfoUtils;
import com.android.server.pm.parsing.PackageParser2;
+import com.android.server.pm.parsing.TestPackageParser2;
import com.android.server.pm.parsing.pkg.AndroidPackage;
import com.android.server.pm.parsing.pkg.PackageImpl;
import com.android.server.pm.parsing.pkg.ParsedPackage;
@@ -108,7 +108,7 @@
@Test
public void testParse_noCache() throws Exception {
- CachePackageNameParser pp = new CachePackageNameParser(null, false, null, null, null);
+ CachePackageNameParser pp = new CachePackageNameParser(null);
ParsedPackage pkg = pp.parsePackage(FRAMEWORK, 0 /* parseFlags */,
false /* useCaches */);
assertNotNull(pkg);
@@ -125,7 +125,7 @@
@Test
public void testParse_withCache() throws Exception {
- CachePackageNameParser pp = new CachePackageNameParser(null, false, null, null, null);
+ CachePackageNameParser pp = new CachePackageNameParser(null);
pp.setCacheDir(mTmpDir);
// The first parse will write this package to the cache.
@@ -144,7 +144,7 @@
// We haven't set a cache directory here : the parse should still succeed,
// just not using the cached results.
- pp = new CachePackageNameParser(null, false, null, null, null);
+ pp = new CachePackageNameParser(null);
pkg = pp.parsePackage(FRAMEWORK, 0 /* parseFlags */, true /* useCaches */);
assertEquals("android", pkg.getPackageName());
@@ -154,17 +154,29 @@
@Test
public void test_serializePackage() throws Exception {
- PackageParser2 pp = new PackageParser2(null, false, null, mTmpDir, null);
- ParsedPackage pkg = pp.parsePackage(FRAMEWORK, 0 /* parseFlags */,
- true /* useCaches */);
+ try (PackageParser2 pp = new PackageParser2(null, false, null, mTmpDir,
+ new PackageParser2.Callback() {
+ @Override
+ public boolean isChangeEnabled(long changeId, @NonNull ApplicationInfo appInfo) {
+ return true;
+ }
- Parcel p = Parcel.obtain();
- pkg.writeToParcel(p, 0 /* flags */);
+ @Override
+ public boolean hasFeature(String feature) {
+ return false;
+ }
+ })) {
+ ParsedPackage pkg = pp.parsePackage(FRAMEWORK, 0 /* parseFlags */,
+ true /* useCaches */);
- p.setDataPosition(0);
- ParsedPackage deserialized = new PackageImpl(p);
+ Parcel p = Parcel.obtain();
+ pkg.writeToParcel(p, 0 /* flags */);
- assertPackagesEqual(pkg, deserialized);
+ p.setDataPosition(0);
+ ParsedPackage deserialized = new PackageImpl(p);
+
+ assertPackagesEqual(pkg, deserialized);
+ }
}
@Test
@@ -218,10 +230,6 @@
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");
@@ -238,7 +246,7 @@
public void testParseIsolatedSplitsDefault() throws Exception {
final File testFile = extractFile(TEST_APP1_APK);
try {
- final ParsedPackage pkg = makeParser().parsePackage(testFile, 0, false);
+ final ParsedPackage pkg = new TestPackageParser2().parsePackage(testFile, 0, false);
assertFalse("isolatedSplits", pkg.isIsolatedSplitLoading());
} finally {
testFile.delete();
@@ -252,7 +260,7 @@
public void testParseIsolatedSplitsConstant() throws Exception {
final File testFile = extractFile(TEST_APP2_APK);
try {
- final ParsedPackage pkg = makeParser().parsePackage(testFile, 0, false);
+ final ParsedPackage pkg = new TestPackageParser2().parsePackage(testFile, 0, false);
assertTrue("isolatedSplits", pkg.isIsolatedSplitLoading());
} finally {
testFile.delete();
@@ -266,7 +274,7 @@
public void testParseIsolatedSplitsResource() throws Exception {
final File testFile = extractFile(TEST_APP3_APK);
try {
- final ParsedPackage pkg = makeParser().parsePackage(testFile, 0, false);
+ final ParsedPackage pkg = new TestPackageParser2().parsePackage(testFile, 0, false);
assertTrue("isolatedSplits", pkg.isIsolatedSplitLoading());
} finally {
testFile.delete();
@@ -279,10 +287,18 @@
*/
public static class CachePackageNameParser extends PackageParser2 {
- CachePackageNameParser(String[] separateProcesses, boolean onlyCoreApps,
- DisplayMetrics displayMetrics, @Nullable File cacheDir,
- PackageParser2.Callback callback) {
- super(separateProcesses, onlyCoreApps, displayMetrics, cacheDir, callback);
+ CachePackageNameParser(@Nullable File cacheDir) {
+ super(null, false, null, null, new Callback() {
+ @Override
+ public boolean isChangeEnabled(long changeId, @NonNull ApplicationInfo appInfo) {
+ return true;
+ }
+
+ @Override
+ public boolean hasFeature(String feature) {
+ return false;
+ }
+ });
if (cacheDir != null) {
setCacheDir(cacheDir);
}
diff --git a/services/tests/servicestests/src/com/android/server/pm/ParallelPackageParserTest.java b/services/tests/servicestests/src/com/android/server/pm/ParallelPackageParserTest.java
index 9d3ac17..38d01d0 100644
--- a/services/tests/servicestests/src/com/android/server/pm/ParallelPackageParserTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/ParallelPackageParserTest.java
@@ -16,13 +16,13 @@
package com.android.server.pm;
-import android.content.pm.PackageParser;
import android.platform.test.annotations.Presubmit;
import android.util.Log;
import androidx.test.runner.AndroidJUnit4;
import com.android.server.pm.parsing.PackageParser2;
+import com.android.server.pm.parsing.TestPackageParser2;
import com.android.server.pm.parsing.pkg.ParsedPackage;
import junit.framework.Assert;
@@ -48,7 +48,7 @@
@Before
public void setUp() {
- mParser = new TestParallelPackageParser(new PackageParser2(null, false, null, null, null),
+ mParser = new TestParallelPackageParser(new TestPackageParser2(),
ParallelPackageParser.makeExecutorService());
}
diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/DexMetadataHelperTest.java b/services/tests/servicestests/src/com/android/server/pm/dex/DexMetadataHelperTest.java
index f87f68d..3888ff3 100644
--- a/services/tests/servicestests/src/com/android/server/pm/dex/DexMetadataHelperTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/dex/DexMetadataHelperTest.java
@@ -35,7 +35,7 @@
import androidx.test.runner.AndroidJUnit4;
import com.android.frameworks.servicestests.R;
-import com.android.server.pm.parsing.PackageParser2;
+import com.android.server.pm.parsing.TestPackageParser2;
import com.android.server.pm.parsing.pkg.AndroidPackageUtils;
import com.android.server.pm.parsing.pkg.ParsedPackage;
@@ -93,15 +93,11 @@
return outFile;
}
- private PackageParser2 makeParser() {
- return new PackageParser2(null, false, null, null, null);
- }
-
@Test
public void testParsePackageWithDmFileValid() throws IOException, PackageParserException {
copyApkToToTmpDir("install_split_base.apk", R.raw.install_split_base);
createDexMetadataFile("install_split_base.apk");
- ParsedPackage pkg = makeParser().parsePackage(mTmpDir, 0 /* flags */, false);
+ ParsedPackage pkg = new TestPackageParser2().parsePackage(mTmpDir, 0 /* flags */, false);
Map<String, String> packageDexMetadata = AndroidPackageUtils.getPackageDexMetadata(pkg);
assertEquals(1, packageDexMetadata.size());
@@ -117,7 +113,7 @@
copyApkToToTmpDir("install_split_feature_a.apk", R.raw.install_split_feature_a);
createDexMetadataFile("install_split_base.apk");
createDexMetadataFile("install_split_feature_a.apk");
- ParsedPackage pkg = makeParser().parsePackage(mTmpDir, 0 /* flags */, false);
+ ParsedPackage pkg = new TestPackageParser2().parsePackage(mTmpDir, 0 /* flags */, false);
Map<String, String> packageDexMetadata = AndroidPackageUtils.getPackageDexMetadata(pkg);
assertEquals(2, packageDexMetadata.size());
@@ -136,7 +132,7 @@
copyApkToToTmpDir("install_split_base.apk", R.raw.install_split_base);
copyApkToToTmpDir("install_split_feature_a.apk", R.raw.install_split_feature_a);
createDexMetadataFile("install_split_feature_a.apk");
- ParsedPackage pkg = makeParser().parsePackage(mTmpDir, 0 /* flags */, false);
+ ParsedPackage pkg = new TestPackageParser2().parsePackage(mTmpDir, 0 /* flags */, false);
Map<String, String> packageDexMetadata = AndroidPackageUtils.getPackageDexMetadata(pkg);
assertEquals(1, packageDexMetadata.size());
@@ -152,7 +148,8 @@
File invalidDmFile = new File(mTmpDir, "install_split_base.dm");
Files.createFile(invalidDmFile.toPath());
try {
- ParsedPackage pkg = makeParser().parsePackage(mTmpDir, 0 /* flags */, false);
+ ParsedPackage pkg = new TestPackageParser2()
+ .parsePackage(mTmpDir, 0 /* flags */, false);
AndroidPackageUtils.validatePackageDexMetadata(pkg);
} catch (PackageParserException e) {
assertEquals(e.error, PackageManager.INSTALL_FAILED_BAD_DEX_METADATA);
@@ -169,7 +166,8 @@
Files.createFile(invalidDmFile.toPath());
try {
- ParsedPackage pkg = makeParser().parsePackage(mTmpDir, 0 /* flags */, false);
+ ParsedPackage pkg = new TestPackageParser2()
+ .parsePackage(mTmpDir, 0 /* flags */, false);
AndroidPackageUtils.validatePackageDexMetadata(pkg);
} catch (PackageParserException e) {
assertEquals(e.error, PackageManager.INSTALL_FAILED_BAD_DEX_METADATA);
diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt b/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt
index ca6b296..f532dd8 100644
--- a/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt
+++ b/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt
@@ -47,8 +47,7 @@
companion object {
- // TODO(chiuwinson): Enable in separate change to fail all presubmit builds and fix errors
- private const val VERIFY_ALL_APKS = false
+ private const val VERIFY_ALL_APKS = true
/** For auditing memory usage differences */
private const val DUMP_HPROF_TO_EXTERNAL = false
@@ -57,13 +56,10 @@
protected val packageParser = PackageParser().apply {
setOnlyCoreApps(false)
setDisplayMetrics(context.resources.displayMetrics)
- setCallback { true }
+ setCallback { false /* hasFeature */ }
}
- protected val packageParser2 = PackageParser2(null, false, context.resources.displayMetrics,
- null, object : PackageParser2.Callback() {
- override fun hasFeature(feature: String?) = true
- })
+ protected val packageParser2 = TestPackageParser2()
/**
* It would be difficult to mock all possibilities, so just use the APKs on device.
diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/PackageParserLegacyCoreTest.java b/services/tests/servicestests/src/com/android/server/pm/parsing/PackageParserLegacyCoreTest.java
index 66cd466..af89761 100644
--- a/services/tests/servicestests/src/com/android/server/pm/parsing/PackageParserLegacyCoreTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/parsing/PackageParserLegacyCoreTest.java
@@ -102,10 +102,6 @@
}
}
- private PackageParser2 makeParser() {
- return new PackageParser2(null, false, null, null, null);
- }
-
@Test
public void testComputeMinSdkVersion_preReleasePlatform() {
// Do allow older release minSdkVersion on pre-release platform.
@@ -357,8 +353,8 @@
File outFile = null;
try {
outFile = copyRawResourceToFile(apkFileName, apkResourceId);
- return converter.apply(
- makeParser().parsePackage(outFile, 0 /* flags */, false));
+ return converter.apply(new TestPackageParser2()
+ .parsePackage(outFile, 0 /* flags */, false));
} finally {
if (outFile != null) {
outFile.delete();
diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/PackageParsingDeferErrorTest.kt b/services/tests/servicestests/src/com/android/server/pm/parsing/PackageParsingDeferErrorTest.kt
new file mode 100644
index 0000000..2248707
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/pm/parsing/PackageParsingDeferErrorTest.kt
@@ -0,0 +1,149 @@
+/*
+ * 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.server.pm.parsing
+
+import android.annotation.RawRes
+import android.content.Context
+import android.content.pm.parsing.ParsingPackage
+import android.content.pm.parsing.ParsingPackageImpl
+import android.content.pm.parsing.ParsingPackageUtils
+import android.content.pm.parsing.result.ParseInput
+import android.content.pm.parsing.result.ParseInput.DeferredError
+import android.content.pm.parsing.result.ParseResult
+import android.content.res.TypedArray
+import android.os.Build
+import androidx.test.InstrumentationRegistry
+import com.android.frameworks.servicestests.R
+import com.google.common.truth.Truth.assertThat
+import com.google.common.truth.Truth.assertWithMessage
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TemporaryFolder
+
+/**
+ * There are 2 known errors when parsing a manifest that were promoted to true failures in R:
+ * 1. Missing an <application> or <instrumentation> tag
+ * 2. An empty string action/category in an intent-filter
+ *
+ * This verifies these failures when the APK targets R.
+ */
+class PackageParsingDeferErrorTest {
+
+ companion object {
+ private const val TEST_ACTIVITY =
+ "com.android.servicestests.pm.parsing.test.TestActivity"
+ private const val TEST_ACTION =
+ "com.android.servicestests.pm.parsing.test.TEST_ACTION"
+ private const val TEST_CATEGORY =
+ "com.android.servicestests.pm.parsing.test.TEST_CATEGORY"
+ private const val TEST_PERMISSION =
+ "com.android.servicestests.pm.parsing.missingapp.TEST_PERMISSION"
+ }
+
+ private val context: Context = InstrumentationRegistry.getContext()
+
+ private val inputCallback = ParseInput.Callback { changeId, _, targetSdk ->
+ when (changeId) {
+ DeferredError.MISSING_APP_TAG -> targetSdk > Build.VERSION_CODES.Q
+ DeferredError.EMPTY_INTENT_ACTION_CATEGORY -> targetSdk > Build.VERSION_CODES.Q
+ else -> throw IllegalStateException("changeId $changeId is not mocked for test")
+ }
+ }
+
+ private val parsingCallback = object : ParsingPackageUtils.Callback {
+ override fun hasFeature(feature: String?) = true
+
+ override fun startParsingPackage(
+ packageName: String,
+ baseCodePath: String,
+ codePath: String,
+ manifestArray: TypedArray,
+ isCoreApp: Boolean
+ ): ParsingPackage {
+ return ParsingPackageImpl(packageName, baseCodePath, codePath, manifestArray)
+ }
+ }
+
+ @get:Rule
+ val tempFolder = TemporaryFolder(context.filesDir)
+
+ @Test
+ fun emptyIntentFilterActionSdkQ() {
+ val result = parseFile(R.raw.PackageParsingTestAppEmptyActionSdkQ)
+ assertWithMessage(result.errorMessage).that(result.isError).isFalse()
+ val activities = result.result.activities
+ // 2 because of AppDetailsActivity
+ assertThat(activities).hasSize(2)
+ val first = activities.first()
+ assertThat(first.name).isEqualTo(TEST_ACTIVITY)
+ val intents = first.intents
+ assertThat(intents).hasSize(1)
+ assertThat(intents.first().hasCategory(TEST_CATEGORY)).isTrue()
+ assertThat(intents.first().hasAction(TEST_ACTION)).isTrue()
+ }
+
+ @Test
+ fun emptyIntentFilterActionSdkR() {
+ val result = parseFile(R.raw.PackageParsingTestAppEmptyActionSdkR)
+ assertThat(result.isError).isTrue()
+ }
+
+ @Test
+ fun emptyIntentFilterCategorySdkQ() {
+ val result = parseFile(R.raw.PackageParsingTestAppEmptyCategorySdkQ)
+ assertWithMessage(result.errorMessage).that(result.isError).isFalse()
+ val activities = result.result.activities
+ // 2 because of AppDetailsActivity
+ assertThat(activities).hasSize(2)
+ val first = activities.first()
+ assertThat(first.name).isEqualTo(TEST_ACTIVITY)
+ val intents = first.intents
+ assertThat(intents).hasSize(1)
+ assertThat(intents.first().hasAction(TEST_ACTION)).isTrue()
+ }
+
+ @Test
+ fun emptyIntentFilterCategorySdkR() {
+ val result = parseFile(R.raw.PackageParsingTestAppEmptyCategorySdkR)
+ assertThat(result.isError).isTrue()
+ }
+
+ @Test
+ fun missingAppTagSdkQ() {
+ val result = parseFile(R.raw.PackageParsingTestAppMissingAppSdkQ)
+ assertWithMessage(result.errorMessage).that(result.isError).isFalse()
+ val permissions = result.result.permissions
+ assertThat(permissions).hasSize(1)
+ assertThat(permissions.first().name).isEqualTo(TEST_PERMISSION)
+ }
+
+ @Test
+ fun missingAppTagSdkR() {
+ val result = parseFile(R.raw.PackageParsingTestAppMissingAppSdkR)
+ assertThat(result.isError).isTrue()
+ }
+
+ private fun parseFile(@RawRes id: Int): ParseResult<ParsingPackage> {
+ val file = tempFolder.newFile()
+ context.resources.openRawResource(id).use { input ->
+ file.outputStream().use { output ->
+ input.copyTo(output)
+ }
+ }
+ return ParsingPackageUtils.parseDefaultOneTime(file, 0, inputCallback, parsingCallback)
+ }
+}
diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/TestPackageParser2.kt b/services/tests/servicestests/src/com/android/server/pm/parsing/TestPackageParser2.kt
new file mode 100644
index 0000000..7ca7682
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/pm/parsing/TestPackageParser2.kt
@@ -0,0 +1,34 @@
+/*
+ * 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.server.pm.parsing
+
+import android.content.pm.ApplicationInfo
+
+class TestPackageParser2 : PackageParser2(null /* separateProcesses */, false /* onlyCoreApps */,
+ null /* displayMetrics */, null /* cacheDir */, object : PackageParser2.Callback() {
+ override fun isChangeEnabled(changeId: Long, appInfo: ApplicationInfo): Boolean {
+ return true
+ }
+
+ override fun hasFeature(feature: String): Boolean {
+ // 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.
+ return false
+ }
+})
diff --git a/services/tests/servicestests/test-apps/PackageParsingTestManifests/Android.bp b/services/tests/servicestests/test-apps/PackageParsingTestManifests/Android.bp
new file mode 100644
index 0000000..8eb56f6
--- /dev/null
+++ b/services/tests/servicestests/test-apps/PackageParsingTestManifests/Android.bp
@@ -0,0 +1,70 @@
+// Copyright (C) 2019 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.
+
+// NOTE: NONE_OF_THESE_TARGETS_ACTUALLY_WORK. AAPT2 doesn't seem to be able to skip
+// errors within tag attributes. This is here as an reference of how to build the test apps, but
+// they will have to built manually and checked into the tree as prebuilts. A modified version of
+// AAPT2 is necessary to build the broken APKs.
+
+// android_test_helper_app {
+// name: "PackageParsingTestAppEmptyActionSdkQ",
+// manifest: "AndroidManifestEmptyAction.xml",
+// srcs: ["**/*.kt"],
+// min_sdk_version: "28",
+// target_sdk_version: "29",
+// aaptflags: ["--warn-manifest-validation"],
+// }
+
+// android_test_helper_app {
+// name: "PackageParsingTestAppEmptyActionSdkR",
+// manifest: "AndroidManifestEmptyAction.xml",
+// srcs: ["**/*.kt"],
+// min_sdk_version: "28",
+// target_sdk_version: "30",
+// aaptflags: ["--warn-manifest-validation"],
+// }
+
+// android_test_helper_app {
+// name: "PackageParsingTestAppEmptyCategorySdkQ",
+// manifest: "AndroidManifestEmptyCategory.xml",
+// srcs: ["**/*.kt"],
+// min_sdk_version: "28",
+// target_sdk_version: "29",
+// aaptflags: ["--warn-manifest-validation"],
+// }
+
+// android_test_helper_app {
+// name: "PackageParsingTestAppEmptyCategorySdkR",
+// manifest: "AndroidManifestEmptyCategory.xml",
+// srcs: ["**/*.kt"],
+// min_sdk_version: "28",
+// target_sdk_version: "30",
+// aaptflags: ["--warn-manifest-validation"],
+// }
+
+// android_test_helper_app {
+// name: "PackageParsingTestAppMissingAppSdkQ",
+// manifest: "AndroidManifestMissingApp.xml",
+// min_sdk_version: "28",
+// target_sdk_version: "29",
+// aaptflags: ["--warn-manifest-validation"],
+// }
+
+// android_test_helper_app {
+// name: "PackageParsingTestAppMissingAppSdkR",
+// manifest: "AndroidManifestMissingApp.xml",
+// min_sdk_version: "28",
+// target_sdk_version: "30",
+// aaptflags: ["--warn-manifest-validation"],
+// }
diff --git a/services/tests/servicestests/test-apps/PackageParsingTestManifests/AndroidManifestEmptyAction.xml b/services/tests/servicestests/test-apps/PackageParsingTestManifests/AndroidManifestEmptyAction.xml
new file mode 100644
index 0000000..cb294f6
--- /dev/null
+++ b/services/tests/servicestests/test-apps/PackageParsingTestManifests/AndroidManifestEmptyAction.xml
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- 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.
+-->
+
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="com.android.servicestests.pm.parsing.emptyaction">
+
+ <application>
+ <activity android:name="com.android.servicestests.pm.parsing.test.TestActivity">
+ <intent-filter>
+ <action android:name="" />
+ <!-- Non-empty action use to verify filter, since 0 action filters are stripped -->
+ <action android:name="com.android.servicestests.pm.parsing.test.TEST_ACTION" />
+ <category android:name="com.android.servicestests.pm.parsing.test.TEST_CATEGORY"/>
+ </intent-filter>
+ </activity>
+ </application>
+
+</manifest>
diff --git a/services/tests/servicestests/test-apps/PackageParsingTestManifests/AndroidManifestEmptyCategory.xml b/services/tests/servicestests/test-apps/PackageParsingTestManifests/AndroidManifestEmptyCategory.xml
new file mode 100644
index 0000000..5b0f80a
--- /dev/null
+++ b/services/tests/servicestests/test-apps/PackageParsingTestManifests/AndroidManifestEmptyCategory.xml
@@ -0,0 +1,29 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- 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.
+-->
+
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="com.android.servicestests.pm.parsing.emptycategory">
+
+ <application>
+ <activity android:name="com.android.servicestests.pm.parsing.test.TestActivity">
+ <intent-filter>
+ <action android:name="com.android.servicestests.pm.parsing.test.TEST_ACTION"/>
+ <category android:name="" />
+ </intent-filter>
+ </activity>
+ </application>
+
+</manifest>
diff --git a/services/tests/servicestests/test-apps/PackageParsingTestManifests/AndroidManifestMissingApp.xml b/services/tests/servicestests/test-apps/PackageParsingTestManifests/AndroidManifestMissingApp.xml
new file mode 100644
index 0000000..3824947
--- /dev/null
+++ b/services/tests/servicestests/test-apps/PackageParsingTestManifests/AndroidManifestMissingApp.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- 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.
+-->
+
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="com.android.servicestests.pm.parsing.missingapp">
+
+ <!-- Only to assert that the manifest parsed correctly -->
+ <permission android:name="com.android.servicestests.pm.parsing.missingapp.TEST_PERMISSION"
+ android:protectionLevel="normal" />
+
+</manifest>
diff --git a/services/tests/servicestests/test-apps/PackageParsingTestManifests/src/com/android/servicestests/pm/parsing/test/TestActivity.kt b/services/tests/servicestests/test-apps/PackageParsingTestManifests/src/com/android/servicestests/pm/parsing/test/TestActivity.kt
new file mode 100644
index 0000000..3b8b29d
--- /dev/null
+++ b/services/tests/servicestests/test-apps/PackageParsingTestManifests/src/com/android/servicestests/pm/parsing/test/TestActivity.kt
@@ -0,0 +1,19 @@
+/*
+ * 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.pm.parsing.test
+
+class TestActivity