Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit bd6ba76b authored by Kweku Adams's avatar Kweku Adams
Browse files

Revert^2 "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
Test: atest com.android.federatedcompute.services.encryption.BackgroundKeyFetchJobServiceTest#testScheduleJob_notNeeded

Change-Id: I04a3c0ca10cddde3052f0cd757ac5a01ced78c7e
parent fe177f2b

apex/jobscheduler/framework/java/android/app/job/JobInfo.java

+43 −6
Original line number Diff line number Diff line
@@ -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);

    }



    /**
@@ -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) {

apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java

+4 −2
Original line number Diff line number Diff line
@@ -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);

apex/jobscheduler/service/java/com/android/server/job/JobStore.java

+1 −1
Original line number Diff line number Diff line
@@ -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;

apex/jobscheduler/service/java/com/android/server/job/controllers/JobStatus.java

+1 −1
Original line number Diff line number Diff line
@@ -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;