Merge "Deliver LinkProperties(with mParcelSensitiveFields=false) to wifi." am: 2c6d12623e

Original change: https://android-review.googlesource.com/c/platform/packages/modules/NetworkStack/+/1932660

Change-Id: I84114748c2fbc7f4bea46572449573284a12838b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/common/networkstackclient/src/android/net/ip/IpClientUtil.java b/common/networkstackclient/src/android/net/ip/IpClientUtil.java
index d488578..7636abd 100644
--- a/common/networkstackclient/src/android/net/ip/IpClientUtil.java
+++ b/common/networkstackclient/src/android/net/ip/IpClientUtil.java
@@ -120,19 +120,27 @@
             mCb.onNewDhcpResults(dhcpResults);
         }
 
+        // LinkProperties always have parcelSensitiveFields=false after parceling
+        // and unparceling, so the IIpClientCallback caller's side will always receive
+        // LinkProperties that do not have the flag set, except on Go devices where
+        // IpClient is running in the system_server, it's possible that no parceling
+        // happens and the object is sent as true, which results in wifi throws the
+        // UnsupportedOperationException when calling LinkProperties.clear() and then
+        // reboot. To keep the consistent behavior, deliver LinkProperties with
+        // mParcelSensitiveFields=false to wifi upon callback is triggered.
         @Override
         public void onProvisioningSuccess(LinkProperties newLp) {
-            mCb.onProvisioningSuccess(newLp);
+            mCb.onProvisioningSuccess(new LinkProperties(newLp));
         }
         @Override
         public void onProvisioningFailure(LinkProperties newLp) {
-            mCb.onProvisioningFailure(newLp);
+            mCb.onProvisioningFailure(new LinkProperties(newLp));
         }
 
         // Invoked on LinkProperties changes.
         @Override
         public void onLinkPropertiesChange(LinkProperties newLp) {
-            mCb.onLinkPropertiesChange(newLp);
+            mCb.onLinkPropertiesChange(new LinkProperties(newLp));
         }
 
         // Called when the internal IpReachabilityMonitor (if enabled) has
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
index da4fb5e..f423862 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java
@@ -59,6 +59,7 @@
 import static com.android.net.module.util.NetworkStackConstants.PIO_FLAG_AUTONOMOUS;
 import static com.android.net.module.util.NetworkStackConstants.PIO_FLAG_ON_LINK;
 import static com.android.testutils.MiscAsserts.assertThrows;
+import static com.android.testutils.ParcelUtils.parcelingRoundTrip;
 import static com.android.testutils.TestPermissionUtil.runAsShell;
 
 import static junit.framework.Assert.fail;
@@ -2320,7 +2321,7 @@
         assertIpMemoryStoreNetworkAttributes(TEST_LEASE_DURATION_S, currentTime, TEST_DEFAULT_MTU);
     }
 
-    private void runDhcpClientCaptivePortalApiTest(boolean featureEnabled,
+    private LinkProperties runDhcpClientCaptivePortalApiTest(boolean featureEnabled,
             boolean serverSendsOption) throws Exception {
         startIpClientProvisioning(false /* isDhcpLeaseCacheEnabled */,
                 false /* shouldReplyRapidCommitAck */, false /* isPreConnectionEnabled */,
@@ -2347,10 +2348,12 @@
 
         // Ensure that the URL was set as expected in the callbacks.
         // Can't verify the URL up to Q as there is no such attribute in LinkProperties.
-        if (!ShimUtils.isAtLeastR()) return;
+        if (!ShimUtils.isAtLeastR()) return null;
         verify(mCb, atLeastOnce()).onLinkPropertiesChange(captor.capture());
-        assertTrue(captor.getAllValues().stream().anyMatch(
-                lp -> Objects.equals(expectedUrl, lp.getCaptivePortalApiUrl())));
+        final LinkProperties expectedLp = captor.getAllValues().stream().findFirst().get();
+        assertNotNull(expectedLp);
+        assertEquals(expectedUrl, expectedLp.getCaptivePortalApiUrl());
+        return expectedLp;
     }
 
     @Test
@@ -2368,6 +2371,30 @@
     }
 
     @Test
+    public void testDhcpClientCaptivePortalApiEnabled_ParcelSensitiveFields() throws Exception {
+        // Only run the test on platforms / builds where the API is enabled
+        assumeTrue(CaptivePortalDataShimImpl.isSupported());
+        LinkProperties lp = runDhcpClientCaptivePortalApiTest(true /* featureEnabled */,
+                true /* serverSendsOption */);
+
+        // Integration test process runs in the same process with network stack module, there
+        // won't be any IPC call happened on IpClientCallbacks, manually run parcelingRoundTrip
+        // to parcel and unparcel the LinkProperties to simulate what happens during the binder
+        // call. In this case lp should contain the senstive data but mParcelSensitiveFields is
+        // false after round trip.
+        if (useNetworkStackSignature()) {
+            lp = parcelingRoundTrip(lp);
+        }
+        final Uri expectedUrl = Uri.parse(TEST_CAPTIVE_PORTAL_URL);
+        assertEquals(expectedUrl, lp.getCaptivePortalApiUrl());
+
+        // Parcel and unparcel the captured LinkProperties, mParcelSensitiveFields is false,
+        // CaptivePortalApiUrl should be null after parceling round trip.
+        final LinkProperties unparceled = parcelingRoundTrip(lp);
+        assertNull(unparceled.getCaptivePortalApiUrl());
+    }
+
+    @Test
     public void testDhcpClientCaptivePortalApiDisabled() throws Exception {
         // Only run the test on platforms / builds where the API is disabled
         assumeFalse(CaptivePortalDataShimImpl.isSupported());