Merge "Fix NetworkStackTests compatibility with Q"
diff --git a/Android.bp b/Android.bp
index 54bea17..13eec44 100644
--- a/Android.bp
+++ b/Android.bp
@@ -21,7 +21,7 @@
// NetworkStackAndroidLibraryDefaults <-- common defaults for android libs
// / \
// +NetworkStackApiStableShims --> / \ <-- +NetworkStackApiCurrentShims
-// +NetworkStackApiStableLevel / \ +NetworkStackApiCurrentLevel
+// +NetworkStackReleaseApiLevel / \ +NetworkStackDevApiLevel
// +jarjar apistub.api[latest].* / \ +module src/
// to apistub.* / \
// / \
@@ -34,7 +34,7 @@
// | <-- +NetworkStackAppDefaults --> |
// | (APK build params) |
// | |
-// | <-- +NetworkStackApiStableLevel | <-- +NetworkStackApiCurrentLevel
+// | <-- +NetworkStackReleaseApiLevel | <-- +NetworkStackDevApiLevel
// | |
// | |
// NetworkStackApiStable NetworkStack, InProcessNetworkStack, <-- APKs
@@ -42,15 +42,15 @@
// Common defaults to define SDK level
java_defaults {
- name: "NetworkStackApiCurrentLevel",
+ name: "NetworkStackDevApiLevel",
sdk_version: "system_current",
- min_sdk_version: "28",
}
java_defaults {
- name: "NetworkStackApiStableLevel",
+ name: "NetworkStackReleaseApiLevel",
sdk_version: "system_29",
- min_sdk_version: "28",
+ min_sdk_version: "29",
+ target_sdk_version: "29",
}
// Filegroups for the API shims
@@ -101,7 +101,7 @@
// there eventually), and to use the compat shim as fallback on older devices.
android_library {
name: "NetworkStackApiCurrentLib",
- defaults: ["NetworkStackApiCurrentLevel", "NetworkStackAndroidLibraryDefaults"],
+ defaults: ["NetworkStackDevApiLevel", "NetworkStackAndroidLibraryDefaults"],
srcs: [
":NetworkStackApiCurrentShims",
"src/**/*.java",
@@ -114,14 +114,14 @@
// linking with the dependencies.
java_library {
name: "NetworkStackApiStableDependencies",
- defaults: ["NetworkStackApiStableLevel", "NetworkStackAndroidLibraryDefaults"],
+ defaults: ["NetworkStackReleaseApiLevel", "NetworkStackAndroidLibraryDefaults"],
srcs: [":NetworkStackApiStableShims"],
jarjar_rules: "apishim/jarjar-rules-compat.txt",
}
android_library {
name: "NetworkStackApiStableLib",
- defaults: ["NetworkStackApiStableLevel"],
+ defaults: ["NetworkStackReleaseApiLevel"],
srcs: [
"src/**/*.java",
":statslog-networkstack-java-gen-q",
@@ -153,7 +153,7 @@
// Non-updatable network stack running in the system server process for devices not using the module
android_app {
name: "InProcessNetworkStack",
- defaults: [ "NetworkStackAppDefaults", "NetworkStackApiCurrentLevel"],
+ defaults: [ "NetworkStackAppDefaults", "NetworkStackDevApiLevel"],
static_libs: ["NetworkStackApiCurrentLib"],
certificate: "platform",
manifest: "AndroidManifest_InProcess.xml",
@@ -165,13 +165,13 @@
required: ["PlatformNetworkPermissionConfig", "PlatformCaptivePortalLogin"],
}
-// Updatable network stack packaged as an application
+// NetworkStack build targeting the current API release, for testing on in-development SDK
android_app {
name: "NetworkStackNext",
- defaults: ["NetworkStackAppDefaults", "NetworkStackApiCurrentLevel"],
+ defaults: ["NetworkStackAppDefaults", "NetworkStackDevApiLevel"],
static_libs: ["NetworkStackApiCurrentLib"],
certificate: "networkstack",
- manifest: "AndroidManifest.xml",
+ manifest: ":NetworkStackNextAndroidManifest",
// The permission configuration *must* be included to ensure security of the device
required: ["NetworkPermissionConfig"],
}
@@ -179,7 +179,7 @@
// Updatable network stack for finalized API
android_app {
name: "NetworkStack",
- defaults: ["NetworkStackAppDefaults", "NetworkStackApiStableLevel"],
+ defaults: ["NetworkStackAppDefaults", "NetworkStackReleaseApiLevel"],
static_libs: ["NetworkStackApiStableLib"],
certificate: "networkstack",
manifest: "AndroidManifest.xml",
@@ -190,8 +190,8 @@
// Android library to derive test APKs for integration tests
android_library {
name: "TestNetworkStackLib",
- defaults: ["NetworkStackAppDefaults", "NetworkStackApiCurrentLevel"],
- static_libs: ["NetworkStackApiCurrentLib"],
+ defaults: ["NetworkStackAppDefaults", "NetworkStackReleaseApiLevel"],
+ static_libs: ["NetworkStackApiStableLib"],
manifest: "AndroidManifestBase.xml",
}
@@ -243,16 +243,37 @@
out: ["com/android/networkstack/metrics/NetworkStackStatsLog.java"],
}
+version_code_networkstack_next = "300000000"
+version_code_networkstack_test = "999999999"
+
genrule {
name: "NetworkStackTestAndroidManifest",
srcs: ["AndroidManifest.xml"],
out: ["TestAndroidManifest.xml"],
- cmd: "sed 's/versionCode=\".*\"/versionCode=\"300000000\"/' $(in) > $(out)",
+ cmd: "sed -E 's/versionCode=\"[0-9]+\"/versionCode=\""
+ + version_code_networkstack_test
+ + "\"/' $(in) > $(out)",
+ visibility: ["//visibility:private"],
+}
+
+// genrule to modify the NetworkStack manifest for NetworkStackNext, which is the "next" version
+// that builds against the "next", non-stable APIs.
+// A genrule seems simpler than having yet another manifest to merge. The only elements that would
+// change in the manifest are the version code, and the min/target SDK which are populated
+// automatically on build with the current SDK.
+genrule {
+ name: "NetworkStackNextAndroidManifest",
+ srcs: ["AndroidManifest.xml"],
+ out: ["NetworkStackNextAndroidManifest.xml"],
+ cmd: "sed -E 's/versionCode=\"[0-9]+\"/versionCode=\""
+ + version_code_networkstack_next
+ + "\"/' $(in) > $(out)",
+ visibility: ["//visibility:private"],
}
android_app {
name: "TestNetworkStack",
- defaults: ["NetworkStackAppDefaults", "NetworkStackApiCurrentLevel"],
+ defaults: ["NetworkStackAppDefaults", "NetworkStackDevApiLevel"],
static_libs: ["NetworkStackApiCurrentLib"],
certificate: "networkstack",
manifest: ":NetworkStackTestAndroidManifest",
diff --git a/AndroidManifest.xml b/AndroidManifest.xml
index b1603d8..f68cdf9 100644
--- a/AndroidManifest.xml
+++ b/AndroidManifest.xml
@@ -22,9 +22,6 @@
android:versionCode="299900000"
android:versionName="2019-09"
>
-
- <uses-sdk android:minSdkVersion="29" android:targetSdkVersion="29" />
-
<!-- Permissions must be defined here, and not in the base manifest, as the network stack
running in the system server process does not need any permission, and having privileged
permissions added would cause crashes on startup unless they are also added to the
diff --git a/AndroidManifest_InProcess.xml b/AndroidManifest_InProcess.xml
index 723df09..41b2e86 100644
--- a/AndroidManifest_InProcess.xml
+++ b/AndroidManifest_InProcess.xml
@@ -20,7 +20,6 @@
package="com.android.networkstack.inprocess"
android:sharedUserId="android.uid.system"
android:process="system">
- <uses-sdk android:minSdkVersion="28" android:targetSdkVersion="28" />
<application>
<service android:name="com.android.server.NetworkStackService"
android:process="system"
diff --git a/src/android/net/dhcp/DhcpClient.java b/src/android/net/dhcp/DhcpClient.java
index b429923..fbcd0f6 100644
--- a/src/android/net/dhcp/DhcpClient.java
+++ b/src/android/net/dhcp/DhcpClient.java
@@ -1348,6 +1348,11 @@
@Override
public void enter() {
super.enter();
+ // We must call notifySuccess to apply the rest of the DHCP configuration (e.g., DNS
+ // servers) before adding the IP address to the interface. Otherwise, as soon as
+ // IpClient sees the IP address appear, it will enter provisioned state without any
+ // configuration information from DHCP. http://b/146850745.
+ notifySuccess();
mController.sendMessage(CMD_CONFIGURE_LINKADDRESS, mDhcpLease.ipAddress);
}
@@ -1632,7 +1637,6 @@
transitionTo(mStoppedState);
}
- notifySuccess();
scheduleLeaseTimers();
logTimeToBoundState();
}
@@ -1701,6 +1705,7 @@
// the registered IpManager.Callback. IP address changes
// are not supported here.
acceptDhcpResults(results, mLeaseMsg);
+ notifySuccess();
transitionTo(mDhcpBoundState);
}
} else if (packet instanceof DhcpNakPacket) {
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
index d30b4e9..30b1b19 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
@@ -53,6 +53,7 @@
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -139,6 +140,7 @@
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -636,8 +638,19 @@
} else {
fail("invalid DHCP packet");
}
+
// wait for reply to DHCPOFFER packet if disabling rapid commit option
if (shouldReplyRapidCommitAck || !(packet instanceof DhcpDiscoverPacket)) {
+ if (!isDhcpIpConflictDetectEnabled && isSuccessLease) {
+ // verify IPv4-only provisioning success before exiting loop.
+ // 1. if it's a failure lease, onProvisioningSuccess() won't be called;
+ // 2. if duplicated IPv4 address detection is enabled, verify TIMEOUT
+ // will affect ARP packet capture running in other test cases.
+ ArgumentCaptor<LinkProperties> captor =
+ ArgumentCaptor.forClass(LinkProperties.class);
+ verifyProvisioningSuccess(captor, Collections.singletonList(CLIENT_ADDR));
+ }
+
return packetList;
}
}
@@ -692,6 +705,16 @@
verify(mCb, timeout(TEST_TIMEOUT_MS)).onLinkPropertiesChange(emptyLp);
}
+ private void verifyProvisioningSuccess(ArgumentCaptor<LinkProperties> captor,
+ final Collection<InetAddress> addresses) throws Exception {
+ verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningSuccess(captor.capture());
+ LinkProperties lp = captor.getValue();
+ assertNotNull(lp);
+ assertNotEquals(0, lp.getDnsServers().size());
+ assertEquals(addresses.size(), lp.getAddresses().size());
+ assertTrue(lp.getAddresses().containsAll(addresses));
+ }
+
private void doRestoreInitialMtuTest(final boolean shouldChangeMtu,
final boolean shouldRemoveTapInterface) throws Exception {
final long currentTime = System.currentTimeMillis();
@@ -862,6 +885,8 @@
assertTrue(packet instanceof DhcpDeclinePacket);
assertEquals(packet.mServerIdentifier, SERVER_ADDR);
assertEquals(packet.mRequestedIp, CLIENT_ADDR);
+
+ verify(mCb, never()).onProvisioningFailure(any());
assertIpMemoryNeverStoreNetworkAttributes();
} else if (isDhcpIpConflictDetectEnabled) {
int arpPacketCount = 0;
@@ -875,6 +900,8 @@
assertArpProbe(packetList.get(0));
assertArpAnnounce(packetList.get(3));
+ ArgumentCaptor<LinkProperties> captor = ArgumentCaptor.forClass(LinkProperties.class);
+ verifyProvisioningSuccess(captor, Collections.singletonList(CLIENT_ADDR));
assertIpMemoryStoreNetworkAttributes(TEST_LEASE_DURATION_S, currentTime,
TEST_DEFAULT_MTU);
}
@@ -903,6 +930,8 @@
performDhcpHandshake(false /* isSuccessLease */, TEST_LEASE_DURATION_S,
true /* isDhcpLeaseCacheEnabled */, false /* shouldReplyRapidCommitAck */,
TEST_DEFAULT_MTU, false /* isDhcpIpConflictDetectEnabled */);
+
+ verify(mCb, never()).onProvisioningSuccess(any());
assertIpMemoryNeverStoreNetworkAttributes();
}
@@ -1232,13 +1261,6 @@
TEST_DEFAULT_MTU, false /* isDhcpIpConflictDetectEnabled */);
assertIpMemoryStoreNetworkAttributes(TEST_LEASE_DURATION_S, currentTime, TEST_DEFAULT_MTU);
- ArgumentCaptor<LinkProperties> captor = ArgumentCaptor.forClass(LinkProperties.class);
- verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningSuccess(captor.capture());
- LinkProperties lp = captor.getValue();
- assertNotNull(lp);
- assertEquals(1, lp.getAddresses().size());
- assertTrue(lp.getAddresses().contains(InetAddress.getByName(CLIENT_ADDR.getHostAddress())));
-
// Stop IpClient and expect a final LinkProperties callback with an empty LP.
mIpc.stop();
verify(mCb, timeout(TEST_TIMEOUT_MS)).onLinkPropertiesChange(argThat(
@@ -1257,12 +1279,6 @@
performDhcpHandshake(true /* isSuccessLease */, TEST_LEASE_DURATION_S,
true /* isDhcpLeaseCacheEnabled */, false /* shouldReplyRapidCommitAck */,
TEST_DEFAULT_MTU, false /* isDhcpIpConflictDetectEnabled */);
-
- verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningSuccess(captor.capture());
- lp = captor.getValue();
- assertNotNull(lp);
- assertEquals(1, lp.getAddresses().size());
- assertTrue(lp.getAddresses().contains(InetAddress.getByName(CLIENT_ADDR.getHostAddress())));
}
@Test