diff options
| author | Winson <chiuwinson@google.com> | 2020-03-10 15:25:32 -0700 |
|---|---|---|
| committer | Winson Chiu <chiuwinson@google.com> | 2020-04-01 05:42:52 +0000 |
| commit | 727da64be5c2553753c768d4b309174016968010 (patch) | |
| tree | f17ec1feb405686348347a919ea3f1c669bc8b3c | |
| parent | 63ff5bb6f13f567c7b5eaa24d52b7d78fbf8e7ed (diff) | |
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
32 files changed, 1106 insertions, 193 deletions
diff --git a/apct-tests/perftests/core/src/android/os/PackageParsingPerfTest.kt b/apct-tests/perftests/core/src/android/os/PackageParsingPerfTest.kt index 9e463652d0b6..29721c593646 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.PackageParserCacheHelper.WriteHelper 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 PackageParsingPerfTest { 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 e90ccdffa8a8..12328cf32fb3 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.ParsedProviderUtils; 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 class ParsingPackageUtils { 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 @@ public class ParsingPackageUtils { } 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 @@ public class ParsingPackageUtils { } 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 @@ public class ParsingPackageUtils { 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 @@ public class ParsingPackageUtils { 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 @@ public class ParsingPackageUtils { 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 @@ public class ParsingPackageUtils { // 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 @@ public class ParsingPackageUtils { 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 class ParsingPackageUtils { 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 6e5c51a22025..f64560a14832 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 @@ public class ParsedActivityUtils { 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 a7b950b194d2..390f76968e7c 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.Resources; 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 @@ public class ParsedIntentInfoUtils { 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 @@ public class ParsedIntentInfoUtils { 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 6188f8933ab2..f4c9914cb69f 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.NonNull; 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.os.Build; 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 @@ class ParsedMainComponentUtils { 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 c46850609d9e..538510049e04 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 @@ import android.content.pm.PackageManager; */ 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 @@ public interface ParseInput { * 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 @@ public interface ParseInput { * 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 338048c3c1ec..518395d7d392 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 9b22f09b2978..b26bf71a61c5 100644 --- a/core/java/android/content/pm/parsing/result/ParseTypeImpl.java +++ b/core/java/android/content/pm/parsing/result/ParseTypeImpl.java @@ -20,54 +20,133 @@ import android.annotation.NonNull; 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; - public ParseInput reset() { - this.result = null; - this.errorCode = PackageManager.INSTALL_SUCCEEDED; - this.errorMessage = null; - this.exception = null; - return this; + /** + * 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; } - @Override - public void ignoreError() { - reset(); + public ParseInput reset() { + 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 <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); + if (mErrorCode != PackageManager.INSTALL_SUCCEEDED) { + Slog.wtf(ParsingUtils.TAG, "Cannot set to success after set to error, was " + + mErrorMessage, mException); } - this.result = result; + mResult = result; //noinspection unchecked return (ParseResult<ResultType>) this; } @Override + public ParseResult<?> deferError(@NonNull String parseError, long deferredError) { + if (DEBUG_THROW_ALL_ERRORS) { + return error(parseError); + } + 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 public <ResultType> ParseResult<ResultType> error(int parseError) { return error(parseError, null); } @@ -84,25 +163,26 @@ public class ParseTypeImpl implements ParseInput, ParseResult<Object> { } @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 @@ public class ParseTypeImpl implements ParseInput, ParseResult<Object> { @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 @@ public class ParseTypeImpl implements ParseInput, ParseResult<Object> { @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 000000000000..d45fee97950f --- /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 44ab43828c2d..273fd48b0d96 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.Map; 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 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub { 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 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub { return new AppIntegrityManagerServiceImpl( context, LocalServices.getService(PackageManagerInternal.class), + PackageParser2::forParsingFileWithDefaults, RuleEvaluationEngine.getRuleEvaluationEngine(), IntegrityFileManager.getInstance(), handlerThread.getThreadHandler()); @@ -145,11 +148,13 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub { 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 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub { 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 414449d42d78..8850f29e9bc2 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 @@ public class PackageManagerService extends IPackageManager.Stub 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 @@ public class PackageManagerService extends IPackageManager.Stub 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 @@ public class PackageManagerService extends IPackageManager.Stub } + packageParser.close(); + List<Runnable> unfinishedTasks = executorService.shutdownNow(); if (!unfinishedTasks.isEmpty()) { throw new IllegalStateException("Not all tasks finished before calling close: " @@ -8901,12 +8909,11 @@ public class PackageManagerService extends IPackageManager.Stub 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 @@ public class PackageManagerService extends IPackageManager.Stub | 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 @@ public class PackageManagerService extends IPackageManager.Stub "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 d561b9c1ffdd..e860c4857bf4 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.ParseResult; 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 @@ import java.io.File; /** * 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 @@ public class PackageParser2 { * 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 @@ public class PackageParser2 { 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 Binary files differnew file mode 100644 index 000000000000..841fcebd6c1f --- /dev/null +++ b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkQ.apk diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkR.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkR.apk Binary files differnew file mode 100644 index 000000000000..3520650215ef --- /dev/null +++ b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyActionSdkR.apk diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkQ.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkQ.apk Binary files differnew file mode 100644 index 000000000000..6774d5f0a452 --- /dev/null +++ b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkQ.apk diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkR.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkR.apk Binary files differnew file mode 100644 index 000000000000..23efb1977bbc --- /dev/null +++ b/services/tests/servicestests/res/raw/PackageParsingTestAppEmptyCategorySdkR.apk diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkQ.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkQ.apk Binary files differnew file mode 100644 index 000000000000..f684f8691151 --- /dev/null +++ b/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkQ.apk diff --git a/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkR.apk b/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkR.apk Binary files differnew file mode 100644 index 000000000000..491c56cef86b --- /dev/null +++ b/services/tests/servicestests/res/raw/PackageParsingTestAppMissingAppSdkR.apk 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 d3a3e7e32ece..e5741ee1a384 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 android.provider.Settings; 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.nio.file.Files; 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 @@ public class AppIntegrityManagerServiceImplTest { @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 @@ public class AppIntegrityManagerServiceImplTest { 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 73e5b6ddfb1d..3718c5c5a5e1 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 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 org.junit.Before; import org.junit.Test; @@ -77,7 +78,7 @@ public class ApexManagerTest { 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 d760629552b8..daaf870fa695 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.Bundle; 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.internal.util.ArrayUtils; 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 @@ public class PackageParserTest { @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 @@ public class PackageParserTest { @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 @@ public class PackageParserTest { // 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 @@ public class PackageParserTest { @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 */); - - Parcel p = Parcel.obtain(); - pkg.writeToParcel(p, 0 /* flags */); - - p.setDataPosition(0); - ParsedPackage deserialized = new PackageImpl(p); - - assertPackagesEqual(pkg, deserialized); + try (PackageParser2 pp = new PackageParser2(null, false, null, mTmpDir, + new PackageParser2.Callback() { + @Override + public boolean isChangeEnabled(long changeId, @NonNull ApplicationInfo appInfo) { + return true; + } + + @Override + public boolean hasFeature(String feature) { + return false; + } + })) { + ParsedPackage pkg = pp.parsePackage(FRAMEWORK, 0 /* parseFlags */, + true /* useCaches */); + + Parcel p = Parcel.obtain(); + pkg.writeToParcel(p, 0 /* flags */); + + p.setDataPosition(0); + ParsedPackage deserialized = new PackageImpl(p); + + assertPackagesEqual(pkg, deserialized); + } } @Test @@ -218,10 +230,6 @@ 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"); @@ -238,7 +246,7 @@ public class PackageParserTest { 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 class PackageParserTest { 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 class PackageParserTest { 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 class PackageParserTest { */ 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 9d3ac1720930..38d01d0c4c18 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 @@ public class ParallelPackageParserTest { @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 f87f68d4942f..3888ff3e278a 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.filters.SmallTest; 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 @@ public class DexMetadataHelperTest { 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 @@ public class DexMetadataHelperTest { 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 @@ public class DexMetadataHelperTest { 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 @@ public class DexMetadataHelperTest { 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 @@ public class DexMetadataHelperTest { 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 ca6b296f155c..f532dd87909e 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 @@ open class AndroidPackageParsingTestBase { 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 @@ open class AndroidPackageParsingTestBase { 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 66cd46699334..af89761acf9d 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 @@ public class PackageParserLegacyCoreTest { } } - 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 @@ public class PackageParserLegacyCoreTest { 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 000000000000..22487071cd71 --- /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 000000000000..7ca7682f1c82 --- /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 000000000000..8eb56f676f91 --- /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 000000000000..cb294f6e87e1 --- /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 000000000000..5b0f80a1754c --- /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 000000000000..38249478551b --- /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 000000000000..3b8b29d19a41 --- /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 |