Merge "Convert domain verify dev errors to IllegalArg" into sc-dev

5 files changed, 118 insertions, 70 deletions

diff --git a/core/api/system-current.txt b/core/api/system-current.txt
index cf6499b2474e..edc6ab8767a5 100644
--- a/core/api/system-current.txt
+++ b/core/api/system-current.txt
@@ -2874,14 +2874,11 @@ package android.content.pm.verify.domain {
     method @NonNull @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public java.util.List<android.content.pm.verify.domain.DomainOwner> getOwnersForDomain(@NonNull String);
     method @NonNull @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) public java.util.List<java.lang.String> queryValidVerificationPackageNames();
     method @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public void setDomainVerificationLinkHandlingAllowed(@NonNull String, boolean) throws android.content.pm.PackageManager.NameNotFoundException;
-    method @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) public int setDomainVerificationStatus(@NonNull java.util.UUID, @NonNull java.util.Set<java.lang.String>, int) throws android.content.pm.PackageManager.NameNotFoundException;
-    method @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public int setDomainVerificationUserSelection(@NonNull java.util.UUID, @NonNull java.util.Set<java.lang.String>, boolean) throws android.content.pm.PackageManager.NameNotFoundException;
+    method @CheckResult @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT) public int setDomainVerificationStatus(@NonNull java.util.UUID, @NonNull java.util.Set<java.lang.String>, int) throws android.content.pm.PackageManager.NameNotFoundException;
+    method @CheckResult @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION) public int setDomainVerificationUserSelection(@NonNull java.util.UUID, @NonNull java.util.Set<java.lang.String>, boolean) throws android.content.pm.PackageManager.NameNotFoundException;
     field public static final int ERROR_DOMAIN_SET_ID_INVALID = 1; // 0x1
-    field public static final int ERROR_DOMAIN_SET_ID_NULL = 2; // 0x2
-    field public static final int ERROR_DOMAIN_SET_NULL_OR_EMPTY = 3; // 0x3
-    field public static final int ERROR_INVALID_STATE_CODE = 6; // 0x6
-    field public static final int ERROR_UNABLE_TO_APPROVE = 5; // 0x5
-    field public static final int ERROR_UNKNOWN_DOMAIN = 4; // 0x4
+    field public static final int ERROR_UNABLE_TO_APPROVE = 3; // 0x3
+    field public static final int ERROR_UNKNOWN_DOMAIN = 2; // 0x2
     field public static final String EXTRA_VERIFICATION_REQUEST = "android.content.pm.verify.domain.extra.VERIFICATION_REQUEST";
     field public static final int STATUS_OK = 0; // 0x0
   }
diff --git a/core/java/android/content/pm/verify/domain/DomainVerificationManager.java b/core/java/android/content/pm/verify/domain/DomainVerificationManager.java
index d187f60ecd44..d2d14410ff3c 100644
--- a/core/java/android/content/pm/verify/domain/DomainVerificationManager.java
+++ b/core/java/android/content/pm/verify/domain/DomainVerificationManager.java
@@ -16,6 +16,7 @@
 
 package android.content.pm.verify.domain;
 
+import android.annotation.CheckResult;
 import android.annotation.IntDef;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
@@ -29,6 +30,8 @@ import android.os.RemoteException;
 import android.os.ServiceSpecificException;
 import android.os.UserHandle;
 
+import com.android.internal.util.CollectionUtils;
+
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
@@ -80,22 +83,6 @@ public final class DomainVerificationManager {
     public static final int ERROR_DOMAIN_SET_ID_INVALID = 1;
 
     /**
-     * The provided domain set ID was null. This is a developer error.
-     *
-     * @hide
-     */
-    @SystemApi
-    public static final int ERROR_DOMAIN_SET_ID_NULL = 2;
-
-    /**
-     * The provided set of domains was null or empty. This is a developer error.
-     *
-     * @hide
-     */
-    @SystemApi
-    public static final int ERROR_DOMAIN_SET_NULL_OR_EMPTY = 3;
-
-    /**
      * The provided set of domains contains a domain not declared by the target package. This
      * usually means the work being processed by the verification agent is outdated and a new
      * request should be scheduled, which should already be in progress as part of the
@@ -104,7 +91,7 @@ public final class DomainVerificationManager {
      * @hide
      */
     @SystemApi
-    public static final int ERROR_UNKNOWN_DOMAIN = 4;
+    public static final int ERROR_UNKNOWN_DOMAIN = 2;
 
     /**
      * The system was unable to select the domain for approval. This indicates another application
@@ -114,17 +101,7 @@ public final class DomainVerificationManager {
      * @hide
      */
     @SystemApi
-    public static final int ERROR_UNABLE_TO_APPROVE = 5;
-
-    /**
-     * The provided state code is incorrect. The domain verification agent is only allowed to
-     * assign {@link DomainVerificationInfo#STATE_SUCCESS} or error codes equal to or greater than
-     * {@link DomainVerificationInfo#STATE_FIRST_VERIFIER_DEFINED}.
-     *
-     * @hide
-     */
-    @SystemApi
-    public static final int ERROR_INVALID_STATE_CODE = 6;
+    public static final int ERROR_UNABLE_TO_APPROVE = 3;
 
     /**
      * Used to communicate through {@link ServiceSpecificException}. Should not be exposed as API.
@@ -138,11 +115,8 @@ public final class DomainVerificationManager {
      */
     @IntDef(prefix = {"ERROR_"}, value = {
             ERROR_DOMAIN_SET_ID_INVALID,
-            ERROR_DOMAIN_SET_ID_NULL,
-            ERROR_DOMAIN_SET_NULL_OR_EMPTY,
             ERROR_UNKNOWN_DOMAIN,
             ERROR_UNABLE_TO_APPROVE,
-            ERROR_INVALID_STATE_CODE
     })
     public @interface Error {
     }
@@ -232,19 +206,21 @@ public final class DomainVerificationManager {
      * @param domainSetId See {@link DomainVerificationInfo#getIdentifier()}.
      * @param domains     List of host names to change the state of.
      * @param state       See {@link DomainVerificationInfo#getHostToStateMap()}.
-     * @throws NameNotFoundException    If the ID is known to be good, but the package is
-     *                                  unavailable. This may be because the package is installed on
-     *                                  a volume that is no longer mounted. This error is
-     *                                  unrecoverable until the package is available again, and
-     *                                  should not be re-tried except on a time scheduled basis.
      * @return error code or {@link #STATUS_OK} if successful
-     *
+     * @throws NameNotFoundException If the ID is known to be good, but the package is
+     *                               unavailable. This may be because the package is installed on
+     *                               a volume that is no longer mounted. This error is
+     *                               unrecoverable until the package is available again, and
+     *                               should not be re-tried except on a time scheduled basis.
      * @hide
      */
+    @CheckResult
     @SystemApi
     @RequiresPermission(android.Manifest.permission.DOMAIN_VERIFICATION_AGENT)
     public int setDomainVerificationStatus(@NonNull UUID domainSetId, @NonNull Set<String> domains,
             int state) throws NameNotFoundException {
+        validateInput(domainSetId, domains);
+
         try {
             return mDomainVerificationManager.setDomainVerificationStatus(domainSetId.toString(),
                     new DomainSet(domains), state);
@@ -312,19 +288,21 @@ public final class DomainVerificationManager {
      * @param domainSetId See {@link DomainVerificationInfo#getIdentifier()}.
      * @param domains     The domains to toggle the state of.
      * @param enabled     Whether or not the app should automatically open the domains specified.
-     * @throws NameNotFoundException    If the ID is known to be good, but the package is
-     *                                  unavailable. This may be because the package is installed on
-     *                                  a volume that is no longer mounted. This error is
-     *                                  unrecoverable until the package is available again, and
-     *                                  should not be re-tried except on a time scheduled basis.
      * @return error code or {@link #STATUS_OK} if successful
-     *
+     * @throws NameNotFoundException If the ID is known to be good, but the package is
+     *                               unavailable. This may be because the package is installed on
+     *                               a volume that is no longer mounted. This error is
+     *                               unrecoverable until the package is available again, and
+     *                               should not be re-tried except on a time scheduled basis.
      * @hide
      */
+    @CheckResult
     @SystemApi
     @RequiresPermission(android.Manifest.permission.UPDATE_DOMAIN_VERIFICATION_USER_SELECTION)
     public int setDomainVerificationUserSelection(@NonNull UUID domainSetId,
             @NonNull Set<String> domains, boolean enabled) throws NameNotFoundException {
+        validateInput(domainSetId, domains);
+
         try {
             return mDomainVerificationManager.setDomainVerificationUserSelection(
                     domainSetId.toString(), new DomainSet(domains), enabled, mContext.getUserId());
@@ -405,4 +383,12 @@ public final class DomainVerificationManager {
             return exception;
         }
     }
+
+    private void validateInput(@Nullable UUID domainSetId, @Nullable Set<String> domains) {
+        if (domainSetId == null) {
+            throw new IllegalArgumentException("domainSetId cannot be null");
+        } else if (CollectionUtils.isEmpty(domains)) {
+            throw new IllegalArgumentException("Provided domain set cannot be empty");
+        }
+    }
 }
diff --git a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java
index a0e252a8a28a..8075bdb4f6f2 100644
--- a/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java
+++ b/services/core/java/com/android/server/pm/verify/domain/DomainVerificationService.java
@@ -284,7 +284,8 @@ public class DomainVerificationService extends SystemService
             int state) throws NameNotFoundException {
         if (state < DomainVerificationState.STATE_FIRST_VERIFIER_DEFINED) {
             if (state != DomainVerificationState.STATE_SUCCESS) {
-                return DomainVerificationManager.ERROR_INVALID_STATE_CODE;
+                throw new IllegalArgumentException(
+                        "Caller is not allowed to set state code " + state);
             }
         }
 
@@ -1119,7 +1120,7 @@ public class DomainVerificationService extends SystemService
             @NonNull Set<String> domains, boolean forAutoVerify, int callingUid,
             @Nullable Integer userIdForFilter) throws NameNotFoundException {
         if (domainSetId == null) {
-            return GetAttachedResult.error(DomainVerificationManager.ERROR_DOMAIN_SET_ID_NULL);
+            throw new IllegalArgumentException("domainSetId cannot be null");
         }
 
         DomainVerificationPkgState pkgState = mAttachedPkgStates.get(domainSetId);
@@ -1140,9 +1141,9 @@ public class DomainVerificationService extends SystemService
         }
 
         if (CollectionUtils.isEmpty(domains)) {
-            return GetAttachedResult.error(
-                    DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY);
+            throw new IllegalArgumentException("Provided domain set cannot be empty");
         }
+
         AndroidPackage pkg = pkgSetting.getPkg();
         ArraySet<String> declaredDomains = forAutoVerify
                 ? mCollector.collectValidAutoVerifyDomains(pkg)
diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationJavaUtil.java b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationJavaUtil.java
index 8ae4c5ae96a3..14c02d53efad 100644
--- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationJavaUtil.java
+++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationJavaUtil.java
@@ -20,6 +20,7 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.UserIdInt;
 import android.content.pm.PackageManager;
+import android.content.pm.verify.domain.DomainVerificationManager;
 
 import com.android.server.pm.verify.domain.DomainVerificationService;
 
@@ -45,4 +46,16 @@ class DomainVerificationJavaUtil {
             throws PackageManager.NameNotFoundException {
         return service.setDomainVerificationUserSelection(domainSetId, domains, enabled, userId);
     }
+
+    static int setStatusForceNullable(@NonNull DomainVerificationManager manager,
+            @Nullable UUID domainSetId, @Nullable Set<String> domains, int state)
+            throws PackageManager.NameNotFoundException {
+        return manager.setDomainVerificationStatus(domainSetId, domains, state);
+    }
+
+    static int setUserSelectionForceNullable(@NonNull DomainVerificationManager manager,
+            @Nullable UUID domainSetId, @Nullable Set<String> domains, boolean enabled)
+            throws PackageManager.NameNotFoundException {
+        return manager.setDomainVerificationUserSelection(domainSetId, domains, enabled);
+    }
 }
diff --git a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt
index ef79b08aa1c5..881604ffe6f6 100644
--- a/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt
+++ b/services/tests/PackageManagerServiceTests/unit/src/com/android/server/pm/test/verify/domain/DomainVerificationManagerApiTest.kt
@@ -16,6 +16,7 @@
 
 package com.android.server.pm.test.verify.domain
 
+import android.content.Context
 import android.content.Intent
 import android.content.pm.PackageManager
 import android.content.pm.parsing.component.ParsedActivity
@@ -23,6 +24,7 @@ import android.content.pm.parsing.component.ParsedIntentInfo
 import android.content.pm.verify.domain.DomainVerificationInfo
 import android.content.pm.verify.domain.DomainVerificationManager
 import android.content.pm.verify.domain.DomainVerificationUserState
+import android.content.pm.verify.domain.IDomainVerificationManager
 import android.os.Build
 import android.os.PatternMatcher
 import android.os.Process
@@ -127,15 +129,17 @@ class DomainVerificationManagerApiTest {
         assertThat(service.setStatus(UUID_INVALID, setOf(DOMAIN_1), 1100))
                 .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID)
 
-        assertThat(DomainVerificationJavaUtil.setStatusForceNullable(service, null,
-                setOf(DOMAIN_1), 1100))
-                .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_ID_NULL)
+        assertFailsWith(IllegalArgumentException::class) {
+            DomainVerificationJavaUtil.setStatusForceNullable(service, null, setOf(DOMAIN_1), 1100)
+        }
 
-        assertThat(DomainVerificationJavaUtil.setStatusForceNullable(service, UUID_ONE, null,
-                1100)).isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY)
+        assertFailsWith(IllegalArgumentException::class) {
+            DomainVerificationJavaUtil.setStatusForceNullable(service, UUID_ONE, null, 1100)
+        }
 
-        assertThat(service.setStatus(UUID_ONE, emptySet(), 1100))
-                .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY)
+        assertFailsWith(IllegalArgumentException::class) {
+            service.setStatus(UUID_ONE, emptySet(), 1100)
+        }
 
         assertThat(service.setStatus(UUID_ONE, setOf(DOMAIN_3), 1100))
                 .isEqualTo(DomainVerificationManager.ERROR_UNKNOWN_DOMAIN)
@@ -143,8 +147,9 @@ class DomainVerificationManagerApiTest {
         assertThat(service.setStatus(UUID_ONE, setOf(DOMAIN_1, DOMAIN_2, DOMAIN_3), 1100))
                 .isEqualTo(DomainVerificationManager.ERROR_UNKNOWN_DOMAIN)
 
-        assertThat(service.setStatus(UUID_ONE, setOf(DOMAIN_1), 15))
-                .isEqualTo(DomainVerificationManager.ERROR_INVALID_STATE_CODE)
+        assertFailsWith(IllegalArgumentException::class) {
+            service.setStatus(UUID_ONE, setOf(DOMAIN_1), 15)
+        }
 
         map.clear()
         assertFailsWith(PackageManager.NameNotFoundException::class){
@@ -198,15 +203,19 @@ class DomainVerificationManagerApiTest {
         assertThat(service.setUserSelection(UUID_INVALID, setOf(DOMAIN_1), true, 0))
                 .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_ID_INVALID)
 
-        assertThat(DomainVerificationJavaUtil.setUserSelectionForceNullable(service, null,
-                setOf(DOMAIN_1), true, 0))
-                .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_ID_NULL)
+        assertFailsWith(IllegalArgumentException::class) {
+            DomainVerificationJavaUtil.setUserSelectionForceNullable(service, null,
+                setOf(DOMAIN_1), true, 0)
+        }
 
-        assertThat(DomainVerificationJavaUtil.setUserSelectionForceNullable(service, UUID_ONE, null,
-                true, 0)).isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY)
+        assertFailsWith(IllegalArgumentException::class) {
+            DomainVerificationJavaUtil.setUserSelectionForceNullable(service, UUID_ONE, null,
+                true, 0)
+        }
 
-        assertThat(service.setUserSelection(UUID_ONE, emptySet(), true, 0))
-                .isEqualTo(DomainVerificationManager.ERROR_DOMAIN_SET_NULL_OR_EMPTY)
+        assertFailsWith(IllegalArgumentException::class) {
+            service.setUserSelection(UUID_ONE, emptySet(), true, 0)
+        }
 
         assertThat(service.setUserSelection(UUID_ONE, setOf(DOMAIN_3), true, 0))
                 .isEqualTo(DomainVerificationManager.ERROR_UNKNOWN_DOMAIN)
@@ -297,6 +306,48 @@ class DomainVerificationManagerApiTest {
         assertThat(service.getOwnersForDomain(DOMAIN_2, 1)).isEmpty()
     }
 
+    @Test
+    fun appProcessManager() {
+        // The app side DomainVerificationManager also has to do some argument enforcement since
+        // the input values are transformed before they are sent across Binder. Verify that here.
+
+        // Mock nothing to ensure no calls are made before failing
+        val context = mockThrowOnUnmocked<Context>()
+        val binderInterface = mockThrowOnUnmocked<IDomainVerificationManager>()
+
+        val manager = DomainVerificationManager(context, binderInterface)
+
+        assertFailsWith(IllegalArgumentException::class) {
+            DomainVerificationJavaUtil.setStatusForceNullable(manager, null, setOf(DOMAIN_1), 1100)
+        }
+
+        assertFailsWith(IllegalArgumentException::class) {
+            DomainVerificationJavaUtil.setStatusForceNullable(manager, UUID_ONE, null, 1100)
+        }
+
+        assertFailsWith(IllegalArgumentException::class) {
+            manager.setDomainVerificationStatus(UUID_ONE, emptySet(), 1100)
+        }
+
+        assertFailsWith(IllegalArgumentException::class) {
+            DomainVerificationJavaUtil.setUserSelectionForceNullable(
+                manager, null,
+                setOf(DOMAIN_1), true
+            )
+        }
+
+        assertFailsWith(IllegalArgumentException::class) {
+            DomainVerificationJavaUtil.setUserSelectionForceNullable(
+                manager, UUID_ONE,
+                null, true
+            )
+        }
+
+        assertFailsWith(IllegalArgumentException::class) {
+            manager.setDomainVerificationUserSelection(UUID_ONE, emptySet(), true)
+        }
+    }
+
     private fun makeService(vararg pkgSettings: PackageSetting) =
             makeService { pkgName -> pkgSettings.find { pkgName == it.getName() } }