From 0869af65b492cb87e4c0269c1411c607c4fe6f56 Mon Sep 17 00:00:00 2001
From: Paul Hu <paulhu@google.com>
Date: Wed, 27 Apr 2022 17:20:00 +0800
Subject: [PATCH] Make a defensive copy when sending NetworkInfo change

The issue scenario is:
- Telephony registers a network agent and calls markConnected().
- NetworkAgent set mNetworkInfo to CONNECTED and call
  queueOrSendNetworkInfo() with mNetworkInfo, but NetworkAgent
  don't actually send a message to CS yet because the agent is
  not registered.
- Telephony calls unregister because the cellular network is
  disconnected.
- NetworkAgent set mNetworkInfo to DISCONNECTED, overwriting the
  previous CONNECTED state, and then call queueOrSendNetworkInfo
  again with mNetworkInfo. Again this doesn't send any message
  because the agent is not connected.
- EVENT_AGENT_CONNECTED arrives. NetworkAgent replies all the
  messages in mPreConnectedQueue, but NetworkAgent send two
  DISCONNECTED NetworkInfos eventually.

The CONNECTED state should be sent to CS instead of latest state
DISCONNECTED. Thus, make a defensive copy when sending
NetworkInfo change to prevent state overwriting.

Without the NetworkAgent fix, the test will fail with no
onAvailable callback. Because the Network has never been martk as
CONNECTED.

Bug: 228623362
Test: atest FrameworksNetTests CtsNetTestCases
Change-Id: I11681743d3ff87ff9affd0b7e766894dc5111028
---
 framework/src/android/net/NetworkAgent.java   |  5 +++--
 .../src/android/net/cts/NetworkAgentTest.kt   | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/framework/src/android/net/NetworkAgent.java b/framework/src/android/net/NetworkAgent.java
index 29add1cbfc..2c50c73331 100644
--- a/framework/src/android/net/NetworkAgent.java
+++ b/framework/src/android/net/NetworkAgent.java
@@ -1076,11 +1076,12 @@ public abstract class NetworkAgent {
      */
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
     public final void sendNetworkInfo(NetworkInfo networkInfo) {
-        queueOrSendNetworkInfo(new NetworkInfo(networkInfo));
+        queueOrSendNetworkInfo(networkInfo);
     }
 
     private void queueOrSendNetworkInfo(NetworkInfo networkInfo) {
-        queueOrSendMessage(reg -> reg.sendNetworkInfo(networkInfo));
+        final NetworkInfo ni = new NetworkInfo(networkInfo);
+        queueOrSendMessage(reg -> reg.sendNetworkInfo(ni));
     }
 
     /**
diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt
index 050497392e..d4f3d579a6 100644
--- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt
+++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt
@@ -1275,4 +1275,23 @@ class NetworkAgentTest {
         matchAllCallback.expectCallback<Lost>(wifiNetwork)
         wifiAgent.expectCallback<OnNetworkUnwanted>()
     }
+
+    @Test
+    fun testUnregisterAgentBeforeAgentFullyConnected() {
+        val specifier = UUID.randomUUID().toString()
+        val callback = TestableNetworkCallback()
+        val transports = intArrayOf(TRANSPORT_CELLULAR)
+        // Ensure this NetworkAgent is never unneeded by filing a request with its specifier.
+        requestNetwork(makeTestNetworkRequest(specifier = specifier), callback)
+        val nc = makeTestNetworkCapabilities(specifier, transports)
+        val agent = createNetworkAgent(realContext, initialNc = nc)
+        // Connect the agent
+        agent.register()
+        // Mark agent connected then unregister agent immediately. Verify that both available and
+        // lost callback should be sent still.
+        agent.markConnected()
+        agent.unregister()
+        callback.expectCallback<Available>(agent.network!!)
+        callback.eventuallyExpect<Lost> { it.network == agent.network }
+    }
 }