From c51a705a45e37e0d0d777776ad22bc75979d5d62 Mon Sep 17 00:00:00 2001
From: chiachangwang <chiachangwang@google.com>
Date: Mon, 13 Mar 2023 02:25:44 +0000
Subject: [PATCH] Correct nullability and add extra check for
 underpinnedNetwork

As the review feedback, this commits address below concern.

1. The TCP keepalive code doesn't use the network parameter
at all. This parameter doesn't seem meaningful for TCP
keepalives. Starting a TCP keepalive with a non-null underpinned
network should throw IllegalArgumentException.

2. The feedback mention that the start version which takes a
@NonNull network should throw NPE if the network is null. But
Starting a NATT keepalive does not always require a underpinned
network. A new IkeSession started from Vpn will also not assign
the underpinned network at the initial stage which means
underpinned will be null until setNetwork() is called. Thus,
the underpinned network should be @Nullable instead.

Fix: 271797087
Test: atest FrameworksNetTests
Change-Id: Ieb57a7b15a06b2ccd94358b65cc00768c4f62e7d
---
 framework/api/system-current.txt                  | 2 +-
 framework/src/android/net/SocketKeepalive.java    | 7 +++++--
 framework/src/android/net/TcpSocketKeepalive.java | 6 ++++++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/framework/api/system-current.txt b/framework/api/system-current.txt
index 196e023d2a..4a2ed8a2a8 100644
--- a/framework/api/system-current.txt
+++ b/framework/api/system-current.txt
@@ -470,7 +470,7 @@ package android.net {
   }
 
   public abstract class SocketKeepalive implements java.lang.AutoCloseable {
-    method public final void start(@IntRange(from=0xa, to=0xe10) int, int, @NonNull android.net.Network);
+    method public final void start(@IntRange(from=0xa, to=0xe10) int, int, @Nullable android.net.Network);
     field public static final int ERROR_NO_SUCH_SLOT = -33; // 0xffffffdf
     field public static final int FLAG_AUTOMATIC_ON_OFF = 1; // 0x1
     field public static final int SUCCESS = 0; // 0x0
diff --git a/framework/src/android/net/SocketKeepalive.java b/framework/src/android/net/SocketKeepalive.java
index 311126ec0a..10daf17a7a 100644
--- a/framework/src/android/net/SocketKeepalive.java
+++ b/framework/src/android/net/SocketKeepalive.java
@@ -21,6 +21,7 @@ import static android.annotation.SystemApi.Client.PRIVILEGED_APPS;
 import android.annotation.IntDef;
 import android.annotation.IntRange;
 import android.annotation.NonNull;
+import android.annotation.Nullable;
 import android.annotation.SystemApi;
 import android.os.Binder;
 import android.os.ParcelFileDescriptor;
@@ -374,12 +375,14 @@ public abstract class SocketKeepalive implements AutoCloseable {
      *                    the supplied {@link Callback} will see a call to
      *                    {@link Callback#onError(int)} with {@link #ERROR_INVALID_INTERVAL}.
      * @param flags Flags to enable/disable available options on this keepalive.
-     * @param underpinnedNetwork The underpinned network of this keepalive.
+     * @param underpinnedNetwork an optional network running over mNetwork that this
+     *                           keepalive is intended to keep up, e.g. an IPSec
+     *                           tunnel running over mNetwork.
      * @hide
      */
     @SystemApi(client = PRIVILEGED_APPS)
     public final void start(@IntRange(from = MIN_INTERVAL_SEC, to = MAX_INTERVAL_SEC)
-            int intervalSec, @StartFlags int flags, @NonNull Network underpinnedNetwork) {
+            int intervalSec, @StartFlags int flags, @Nullable Network underpinnedNetwork) {
         startImpl(intervalSec, flags, underpinnedNetwork);
     }
 
diff --git a/framework/src/android/net/TcpSocketKeepalive.java b/framework/src/android/net/TcpSocketKeepalive.java
index b548f6d2f1..696889f569 100644
--- a/framework/src/android/net/TcpSocketKeepalive.java
+++ b/framework/src/android/net/TcpSocketKeepalive.java
@@ -55,6 +55,12 @@ public final class TcpSocketKeepalive extends SocketKeepalive {
             throw new IllegalArgumentException("Illegal flag value for "
                     + this.getClass().getSimpleName() + " : " + flags);
         }
+
+        if (underpinnedNetwork != null) {
+            throw new IllegalArgumentException("Illegal underpinned network for "
+                    + this.getClass().getSimpleName() + " : " + underpinnedNetwork);
+        }
+
         mExecutor.execute(() -> {
             try {
                 mService.startTcpKeepalive(mNetwork, mPfd, intervalSec, mCallback);