Handle negative delays and deadlines.

Some apps are scheduling jobs with negative delays and deadlines. This
is invalid input that can lead to incorrect behavior when used in
calculations. Set a floor for delays and deadlines so that negatives are
no longer propagated and throw a target SDK gated exception for the
invalid input.

Bug: 323349338
Test: atest CtsJobSchedulerTestCases:JobInfoTest
Change-Id: I524cf83d4f60e72a874c6272c30c348a04de00ba

4 files changed, 51 insertions, 12 deletions

diff --git a/apex/jobscheduler/framework/java/android/app/job/JobInfo.java b/apex/jobscheduler/framework/java/android/app/job/JobInfo.java
index 7284f479df35..817726834c2c 100644
--- a/apex/jobscheduler/framework/java/android/app/job/JobInfo.java
+++ b/apex/jobscheduler/framework/java/android/app/job/JobInfo.java
@@ -124,6 +124,15 @@ public class JobInfo implements Parcelable {
     @Overridable // Aid in testing
     public static final long ENFORCE_MINIMUM_TIME_WINDOWS = 311402873L;
 
+    /**
+     * Require that minimum latencies and override deadlines are nonnegative.
+     *
+     * @hide
+     */
+    @ChangeId
+    @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
+    public static final long REJECT_NEGATIVE_DELAYS_AND_DEADLINES = 323349338L;
+
     /** @hide */
     @IntDef(prefix = { "NETWORK_TYPE_" }, value = {
             NETWORK_TYPE_NONE,
@@ -692,14 +701,14 @@ public class JobInfo implements Parcelable {
      * @see JobInfo.Builder#setMinimumLatency(long)
      */
     public long getMinLatencyMillis() {
-        return minLatencyMillis;
+        return Math.max(0, minLatencyMillis);
     }
 
     /**
      * @see JobInfo.Builder#setOverrideDeadline(long)
      */
     public long getMaxExecutionDelayMillis() {
-        return maxExecutionDelayMillis;
+        return Math.max(0, maxExecutionDelayMillis);
     }
 
     /**
@@ -818,7 +827,7 @@ public class JobInfo implements Parcelable {
      * @hide
      */
     public boolean hasEarlyConstraint() {
-        return hasEarlyConstraint;
+        return hasEarlyConstraint && minLatencyMillis > 0;
     }
 
     /**
@@ -827,7 +836,7 @@ public class JobInfo implements Parcelable {
      * @hide
      */
     public boolean hasLateConstraint() {
-        return hasLateConstraint;
+        return hasLateConstraint && maxExecutionDelayMillis >= 0;
     }
 
     @Override
@@ -1869,6 +1878,13 @@ public class JobInfo implements Parcelable {
          * Because it doesn't make sense setting this property on a periodic job, doing so will
          * throw an {@link java.lang.IllegalArgumentException} when
          * {@link android.app.job.JobInfo.Builder#build()} is called.
+         *
+         * Negative latencies also don't make sense for a job and are indicative of an error,
+         * so starting in Android version {@link android.os.Build.VERSION_CODES#VANILLA_ICE_CREAM},
+         * setting a negative deadline will result in
+         * {@link android.app.job.JobInfo.Builder#build()} throwing an
+         * {@link java.lang.IllegalArgumentException}.
+         *
          * @param minLatencyMillis Milliseconds before which this job will not be considered for
          *                         execution.
          * @see JobInfo#getMinLatencyMillis()
@@ -1892,6 +1908,13 @@ public class JobInfo implements Parcelable {
          * throw an {@link java.lang.IllegalArgumentException} when
          * {@link android.app.job.JobInfo.Builder#build()} is called.
          *
+         * <p>
+         * Negative deadlines also don't make sense for a job and are indicative of an error,
+         * so starting in Android version {@link android.os.Build.VERSION_CODES#VANILLA_ICE_CREAM},
+         * setting a negative deadline will result in
+         * {@link android.app.job.JobInfo.Builder#build()} throwing an
+         * {@link java.lang.IllegalArgumentException}.
+         *
          * <p class="note">
          * Since a job will run once the deadline has passed regardless of the status of other
          * constraints, setting a deadline of 0 (or a {@link #setMinimumLatency(long) delay} equal
@@ -2189,13 +2212,15 @@ public class JobInfo implements Parcelable {
         public JobInfo build() {
             return build(Compatibility.isChangeEnabled(DISALLOW_DEADLINES_FOR_PREFETCH_JOBS),
                     Compatibility.isChangeEnabled(REJECT_NEGATIVE_NETWORK_ESTIMATES),
-                    Compatibility.isChangeEnabled(ENFORCE_MINIMUM_TIME_WINDOWS));
+                    Compatibility.isChangeEnabled(ENFORCE_MINIMUM_TIME_WINDOWS),
+                    Compatibility.isChangeEnabled(REJECT_NEGATIVE_DELAYS_AND_DEADLINES));
         }
 
         /** @hide */
         public JobInfo build(boolean disallowPrefetchDeadlines,
                 boolean rejectNegativeNetworkEstimates,
-                boolean enforceMinimumTimeWindows) {
+                boolean enforceMinimumTimeWindows,
+                boolean rejectNegativeDelaysAndDeadlines) {
             // This check doesn't need to be inside enforceValidity. It's an unnecessary legacy
             // check that would ideally be phased out instead.
             if (mBackoffPolicySet && (mConstraintFlags & CONSTRAINT_FLAG_DEVICE_IDLE) != 0) {
@@ -2205,7 +2230,7 @@ public class JobInfo implements Parcelable {
             }
             JobInfo jobInfo = new JobInfo(this);
             jobInfo.enforceValidity(disallowPrefetchDeadlines, rejectNegativeNetworkEstimates,
-                    enforceMinimumTimeWindows);
+                    enforceMinimumTimeWindows, rejectNegativeDelaysAndDeadlines);
             return jobInfo;
         }
 
@@ -2225,7 +2250,8 @@ public class JobInfo implements Parcelable {
      */
     public final void enforceValidity(boolean disallowPrefetchDeadlines,
             boolean rejectNegativeNetworkEstimates,
-            boolean enforceMinimumTimeWindows) {
+            boolean enforceMinimumTimeWindows,
+            boolean rejectNegativeDelaysAndDeadlines) {
         // Check that network estimates require network type and are reasonable values.
         if ((networkDownloadBytes > 0 || networkUploadBytes > 0 || minimumNetworkChunkBytes > 0)
                 && networkRequest == null) {
@@ -2259,6 +2285,17 @@ public class JobInfo implements Parcelable {
             throw new IllegalArgumentException("Minimum chunk size must be positive");
         }
 
+        if (rejectNegativeDelaysAndDeadlines) {
+            if (minLatencyMillis < 0) {
+                throw new IllegalArgumentException(
+                        "Minimum latency is negative: " + minLatencyMillis);
+            }
+            if (maxExecutionDelayMillis < 0) {
+                throw new IllegalArgumentException(
+                        "Override deadline is negative: " + maxExecutionDelayMillis);
+            }
+        }
+
         final boolean hasDeadline = maxExecutionDelayMillis != 0L;
         // Check that a deadline was not set on a periodic job.
         if (isPeriodic) {
diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
index a83c099b764d..f819f15b430f 100644
--- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
@@ -4850,7 +4850,7 @@ public class JobSchedulerService extends com.android.server.SystemService
                     Slog.w(TAG, "Uid " + uid + " set bias on its job");
                     return new JobInfo.Builder(job)
                             .setBias(JobInfo.BIAS_DEFAULT)
-                            .build(false, false, false);
+                            .build(false, false, false, false);
                 }
             }
 
@@ -4874,7 +4874,9 @@ public class JobSchedulerService extends com.android.server.SystemService
                             JobInfo.DISALLOW_DEADLINES_FOR_PREFETCH_JOBS, callingUid),
                     rejectNegativeNetworkEstimates,
                     CompatChanges.isChangeEnabled(
-                            JobInfo.ENFORCE_MINIMUM_TIME_WINDOWS, callingUid));
+                            JobInfo.ENFORCE_MINIMUM_TIME_WINDOWS, callingUid),
+                    CompatChanges.isChangeEnabled(
+                            JobInfo.REJECT_NEGATIVE_DELAYS_AND_DEADLINES, callingUid));
             if ((job.getFlags() & JobInfo.FLAG_WILL_BE_FOREGROUND) != 0) {
                 getContext().enforceCallingOrSelfPermission(
                         android.Manifest.permission.CONNECTIVITY_INTERNAL, TAG);
diff --git a/apex/jobscheduler/service/java/com/android/server/job/JobStore.java b/apex/jobscheduler/service/java/com/android/server/job/JobStore.java
index 53b14d616ecc..d8934d8f83b8 100644
--- a/apex/jobscheduler/service/java/com/android/server/job/JobStore.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/JobStore.java
@@ -1495,7 +1495,7 @@ public final class JobStore {
                 // return value), the deadline is dropped. Periodic jobs require all constraints
                 // to be met, so there's no issue with their deadlines.
                 // The same logic applies for other target SDK-based validation checks.
-                builtJob = jobBuilder.build(false, false, false);
+                builtJob = jobBuilder.build(false, false, false, false);
             } catch (Exception e) {
                 Slog.w(TAG, "Unable to build job from XML, ignoring: " + jobBuilder.summarize(), e);
                 return null;
diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java
index a3a686fdc5c8..655afbc407f6 100644
--- a/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java
@@ -657,7 +657,7 @@ public final class JobStatus {
                     .build());
             // Don't perform validation checks at this point since we've already passed the
             // initial validation check.
-            job = builder.build(false, false, false);
+            job = builder.build(false, false, false, false);
         }
 
         this.job = job;