Improve the net-capabilities scheme for backward compatibility

- Create a new XML tag for the new format. The new format should
  persist the arrays of values without assuming they necessarily
  have different bits.

- Upon reading, if the new tag is present, use that. If the new
  tag is not present but the old tag is present, then limit the
  contents to the capabilities/transports that existed in R.

- Catch all exceptions and ignore the requests if they can't be
  re-read from disk. This is a measure to avoid any mistake where
  the device couldn't boot when JS tries to restore state.

Bug: 183071974
Test: atest JobStoreTest
Original-Change: https://android-review.googlesource.com/1679762
Merged-In: I56cde50d2adab81134c8be4f6996d68018f4212a
Change-Id: I56cde50d2adab81134c8be4f6996d68018f4212a
This commit is contained in:
Frank Li 2021-05-21 01:49:50 +00:00
parent d77459a21f
commit 2fcda2f9f4
2 changed files with 126 additions and 23 deletions

View File

@ -16,6 +16,9 @@
package com.android.server.job;
import static android.net.NetworkCapabilities.NET_CAPABILITY_TEMPORARILY_NOT_METERED;
import static android.net.NetworkCapabilities.TRANSPORT_TEST;
import static com.android.server.job.JobSchedulerService.sElapsedRealtimeClock;
import static com.android.server.job.JobSchedulerService.sSystemClock;
@ -30,6 +33,7 @@ import android.os.PersistableBundle;
import android.os.Process;
import android.os.SystemClock;
import android.os.UserHandle;
import android.text.TextUtils;
import android.text.format.DateUtils;
import android.util.ArraySet;
import android.util.AtomicFile;
@ -63,6 +67,7 @@ import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.StringJoiner;
import java.util.function.Consumer;
import java.util.function.Predicate;
@ -386,6 +391,36 @@ public final class JobStore {
return true;
}
/**
* Returns a single string representation of the contents of the specified intArray.
* If the intArray is [1, 2, 4] as the input, the return result will be the string "1,2,4".
*/
@VisibleForTesting
static String intArrayToString(int[] values) {
final StringJoiner sj = new StringJoiner(",");
for (final int value : values) {
sj.add(String.valueOf(value));
}
return sj.toString();
}
/**
* Converts a string containing a comma-separated list of decimal representations
* of ints into an array of int. If the string is not correctly formatted,
* or if any value doesn't fit into an int, NumberFormatException is thrown.
*/
@VisibleForTesting
static int[] stringToIntArray(String str) {
if (TextUtils.isEmpty(str)) return new int[0];
final String[] arr = str.split(",");
final int[] values = new int[arr.length];
for (int i = 0; i < arr.length; i++) {
values[i] = Integer.parseInt(arr[i]);
}
return values;
}
/**
* Runnable that writes {@link #mJobSet} out to xml.
* NOTE: This Runnable locks on mLock
@ -549,15 +584,12 @@ public final class JobStore {
out.startTag(null, XML_TAG_PARAMS_CONSTRAINTS);
if (jobStatus.hasConnectivityConstraint()) {
final NetworkRequest network = jobStatus.getJob().getRequiredNetwork();
// STOPSHIP b/183071974: improve the scheme for backward compatibility and
// mainline cleanliness.
out.attribute(null, "net-capabilities", Long.toString(
BitUtils.packBits(network.getCapabilities())));
out.attribute(null, "net-unwanted-capabilities", Long.toString(
BitUtils.packBits(network.getForbiddenCapabilities())));
out.attribute(null, "net-transport-types", Long.toString(
BitUtils.packBits(network.getTransportTypes())));
out.attribute(null, "net-capabilities-csv", intArrayToString(
network.getCapabilities()));
out.attribute(null, "net-forbidden-capabilities-csv", intArrayToString(
network.getForbiddenCapabilities()));
out.attribute(null, "net-transport-types-csv", intArrayToString(
network.getTransportTypes()));
}
if (jobStatus.hasIdleConstraint()) {
out.attribute(null, "idle", Boolean.toString(true));
@ -831,7 +863,14 @@ public final class JobStore {
} catch (NumberFormatException e) {
Slog.d(TAG, "Error reading constraints, skipping.");
return null;
} catch (XmlPullParserException e) {
Slog.d(TAG, "Error Parser Exception.", e);
return null;
} catch (IOException e) {
Slog.d(TAG, "Error I/O Exception.", e);
return null;
}
parser.next(); // Consume </constraints>
// Read out execution parameters tag.
@ -973,31 +1012,79 @@ public final class JobStore {
return new JobInfo.Builder(jobId, cname);
}
private void buildConstraintsFromXml(JobInfo.Builder jobBuilder, XmlPullParser parser) {
/**
* In S, there has been a change in format to make the code more robust and more
* maintainable.
* If the capabities are bits 4, 14, 15, the format in R, it is a long string as
* netCapabilitiesLong = '49168' from the old XML file attribute "net-capabilities".
* The format in S is the int array string as netCapabilitiesIntArray = '4,14,15'
* from the new XML file attribute "net-capabilities-array".
* For backward compatibility, when reading old XML the old format is still supported in
* reading, but in order to avoid issues with OEM-defined flags, the accepted capabilities
* are limited to that(maxNetCapabilityInR & maxTransportInR) defined in R.
*/
private void buildConstraintsFromXml(JobInfo.Builder jobBuilder, XmlPullParser parser)
throws XmlPullParserException, IOException {
String val;
String netCapabilitiesLong = null;
String netForbiddenCapabilitiesLong = null;
String netTransportTypesLong = null;
final String netCapabilities = parser.getAttributeValue(null, "net-capabilities");
final String netforbiddenCapabilities = parser.getAttributeValue(
null, "net-unwanted-capabilities");
final String netTransportTypes = parser.getAttributeValue(null, "net-transport-types");
if (netCapabilities != null && netTransportTypes != null) {
final String netCapabilitiesIntArray = parser.getAttributeValue(
null, "net-capabilities-csv");
final String netForbiddenCapabilitiesIntArray = parser.getAttributeValue(
null, "net-forbidden-capabilities-csv");
final String netTransportTypesIntArray = parser.getAttributeValue(
null, "net-transport-types-csv");
if (netCapabilitiesIntArray == null || netTransportTypesIntArray == null) {
netCapabilitiesLong = parser.getAttributeValue(null, "net-capabilities");
netForbiddenCapabilitiesLong = parser.getAttributeValue(
null, "net-unwanted-capabilities");
netTransportTypesLong = parser.getAttributeValue(null, "net-transport-types");
}
if ((netCapabilitiesIntArray != null) && (netTransportTypesIntArray != null)) {
final NetworkRequest.Builder builder = new NetworkRequest.Builder()
.clearCapabilities();
final long forbiddenCapabilities = netforbiddenCapabilities != null
? Long.parseLong(netforbiddenCapabilities)
: BitUtils.packBits(builder.build().getForbiddenCapabilities());
// We're okay throwing NFE here; caught by caller
for (int capability : BitUtils.unpackBits(Long.parseLong(netCapabilities))) {
for (int capability : stringToIntArray(netCapabilitiesIntArray)) {
builder.addCapability(capability);
}
for (int forbiddenCapability : BitUtils.unpackBits(
Long.parseLong(netforbiddenCapabilities))) {
for (int forbiddenCapability : stringToIntArray(netForbiddenCapabilitiesIntArray)) {
builder.addForbiddenCapability(forbiddenCapability);
}
for (int transport : BitUtils.unpackBits(Long.parseLong(netTransportTypes))) {
for (int transport : stringToIntArray(netTransportTypesIntArray)) {
builder.addTransportType(transport);
}
jobBuilder.setRequiredNetwork(builder.build());
} else if (netCapabilitiesLong != null && netTransportTypesLong != null) {
final NetworkRequest.Builder builder = new NetworkRequest.Builder()
.clearCapabilities();
final int maxNetCapabilityInR = NET_CAPABILITY_TEMPORARILY_NOT_METERED;
// We're okay throwing NFE here; caught by caller
for (int capability : BitUtils.unpackBits(Long.parseLong(
netCapabilitiesLong))) {
if (capability <= maxNetCapabilityInR) {
builder.addCapability(capability);
}
}
for (int forbiddenCapability : BitUtils.unpackBits(Long.parseLong(
netForbiddenCapabilitiesLong))) {
if (forbiddenCapability <= maxNetCapabilityInR) {
builder.addForbiddenCapability(forbiddenCapability);
}
}
final int maxTransportInR = TRANSPORT_TEST;
for (int transport : BitUtils.unpackBits(Long.parseLong(
netTransportTypesLong))) {
if (transport <= maxTransportInR) {
builder.addTransportType(transport);
}
}
jobBuilder.setRequiredNetwork(builder.build());
} else {
// Read legacy values
val = parser.getAttributeValue(null, "connectivity");

View File

@ -4,7 +4,9 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_IMS;
import static android.net.NetworkCapabilities.NET_CAPABILITY_OEM_PAID;
import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.anyString;
@ -101,6 +103,20 @@ public class JobStoreTest {
mTaskStoreUnderTest.waitForWriteToCompleteForTesting(5_000L));
}
@Test
public void testStringToIntArrayAndIntArrayToString() {
final int[] netCapabilitiesIntArray = { 1, 3, 5, 7, 9 };
final String netCapabilitiesStr = "1,3,5,7,9";
final String netCapabilitiesStrWithErrorInt = "1,3,a,7,9";
final String emptyString = "";
final String str1 = JobStore.intArrayToString(netCapabilitiesIntArray);
assertArrayEquals(netCapabilitiesIntArray, JobStore.stringToIntArray(str1));
assertEquals(0, JobStore.stringToIntArray(emptyString).length);
assertThrows(NumberFormatException.class,
() -> JobStore.stringToIntArray(netCapabilitiesStrWithErrorInt));
assertEquals(netCapabilitiesStr, JobStore.intArrayToString(netCapabilitiesIntArray));
}
@Test
public void testMaybeWriteStatusToDisk() throws Exception {
int taskId = 5;