mobile: Stop the DNS cache when the network is unavailable (envoyprox…

…y#35922) This PR fixes an issue on Android when the network was unavailable, Envoy Mobile would always refresh the DNS cache causing spurious DNS errors. This PR fixes the issue by cancelling the pending DNS queries and stopping the DNS refresh and timeout timers when the network is unavailable. As a result, this requires adding a new `DnsCache::stop` function. The `InternalEngine` API has also been updated by introducing `onDefaultNetworkAvailable`, `onDefaultNetworkChanged` and `onDefaultNetworkUnavailable` callbacks instead of using `setPreferredNetwork`, a method that has been incorrectly used for on network changed callback. By having these new callbacks, we now have a fine-grained control to refresh and stop the DNS caches. The `AndroidNetworkMonitor` has been reimplemented to use [ConnectivityManager.Network](https://developer.android.com/reference/android/net/ConnectivityManager.NetworkCallback) instead of the deprecated API and more tests has also been added. Risk Level: medium Testing: unit and integration tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: dns, mobile --------- Signed-off-by: Fredy Wijaya <[email protected]>
ohadvano · Sep 5, 2024 · c17bbb9 · c17bbb9
1 parent b5bc273
commit c17bbb9
Show file tree

Hide file tree

Showing 20 changed files with 432 additions and 256 deletions.
diff --git a/mobile/library/common/internal_engine.cc b/mobile/library/common/internal_engine.cc
@@ -281,10 +281,14 @@ envoy_status_t InternalEngine::resetConnectivityState() {
   return dispatcher_->post([&]() -> void { connectivity_manager_->resetConnectivityState(); });
 }
 
-envoy_status_t InternalEngine::setPreferredNetwork(NetworkType network) {
-  return dispatcher_->post([&, network]() -> void {
-    envoy_netconf_t configuration_key =
-        Network::ConnectivityManagerImpl::setPreferredNetwork(network);
+void InternalEngine::onDefaultNetworkAvailable() {
+  ENVOY_LOG_MISC(trace, "Calling the default network available callback");
+}
+
+void InternalEngine::onDefaultNetworkChanged(NetworkType network) {
+  ENVOY_LOG_MISC(trace, "Calling the default network changed callback");
+  dispatcher_->post([&, network]() -> void {
+    envoy_netconf_t configuration = Network::ConnectivityManagerImpl::setPreferredNetwork(network);
     if (Runtime::runtimeFeatureEnabled(
             "envoy.reloadable_features.dns_cache_set_ip_version_to_remove")) {
       // The IP version to remove flag must be set first before refreshing the DNS cache so that
@@ -305,10 +309,15 @@ envoy_status_t InternalEngine::setPreferredNetwork(NetworkType network) {
           [](Http::HttpServerPropertiesCache& cache) { cache.resetBrokenness(); };
       cache_manager.forEachThreadLocalCache(clear_brokenness);
     }
-    connectivity_manager_->refreshDns(configuration_key, true);
+    connectivity_manager_->refreshDns(configuration, true);
   });
 }
 
+void InternalEngine::onDefaultNetworkUnavailable() {
+  ENVOY_LOG_MISC(trace, "Calling the default network unavailable callback");
+  dispatcher_->post([&]() -> void { connectivity_manager_->dnsCache()->stop(); });
+}
+
 envoy_status_t InternalEngine::recordCounterInc(absl::string_view elements, envoy_stats_tags tags,
                                                 uint64_t count) {
   return dispatcher_->post(

diff --git a/mobile/library/common/internal_engine.h b/mobile/library/common/internal_engine.h
@@ -106,9 +106,15 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
   // to networkConnectivityManager after doing a dispatcher post (thread context switch)
   envoy_status_t setProxySettings(const char* host, const uint16_t port);
   envoy_status_t resetConnectivityState();
+
+  /**
+   * This function is called when the default network is available. This function is currently
+   * no-op.
+   */
+  void onDefaultNetworkAvailable();
+
   /**
-   * This function does the following on a network change event (such as switching from WiFI to
-   * cellular, WIFi A to WiFI B, etc.).
+   * This function does the following when the default network configuration was changed.
    *
    * - Sets the preferred network.
    * - Check for IPv6 connectivity. If there is no IPv6 no connectivity, it will call
@@ -117,7 +123,15 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
    * - Force refresh the hosts in the DNS cache (will take `setIpVersionToRemove` into account).
    * - Optionally (if configured) clear HTTP/3 broken status.
    */
-  envoy_status_t setPreferredNetwork(NetworkType network);
+  void onDefaultNetworkChanged(NetworkType network);
+
+  /**
+   * This functions does the following when the default network is unavailable.
+   *
+   * - Cancel the DNS pending queries.
+   * - Stop the DNS timeout and refresh timers.
+   */
+  void onDefaultNetworkUnavailable();
 
   /**
    * Increment a counter with a given string of elements and by the given count.

diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidEngineImpl.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidEngineImpl.java
@@ -81,8 +81,18 @@ public void resetConnectivityState() {
   }
 
   @Override
-  public void setPreferredNetwork(EnvoyNetworkType network) {
-    envoyEngine.setPreferredNetwork(network);
+  public void onDefaultNetworkAvailable() {
+    envoyEngine.onDefaultNetworkAvailable();
+  }
+
+  @Override
+  public void onDefaultNetworkChanged(EnvoyNetworkType network) {
+    envoyEngine.onDefaultNetworkChanged(network);
+  }
+
+  @Override
+  public void onDefaultNetworkUnavailable() {
+    envoyEngine.onDefaultNetworkUnavailable();
   }
 
   public void setProxySettings(String host, int port) { envoyEngine.setProxySettings(host, port); }

diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitor.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/AndroidNetworkMonitor.java
@@ -3,41 +3,37 @@
 import io.envoyproxy.envoymobile.engine.types.EnvoyNetworkType;
 
 import android.Manifest;
-import android.annotation.TargetApi;
-import android.content.BroadcastReceiver;
 import android.content.Context;
-import android.content.Intent;
-import android.content.IntentFilter;
 import android.content.pm.PackageManager;
 import android.net.ConnectivityManager;
 import android.net.ConnectivityManager.NetworkCallback;
 import android.net.Network;
 import android.net.NetworkCapabilities;
-import android.net.NetworkInfo;
-import android.net.NetworkRequest;
-import android.os.Build;
+
+import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;
 import androidx.core.content.ContextCompat;
 
 import java.util.Collections;
 
 /**
- * This class makes use of some deprecated APIs, but it's only current purpose is to attempt to
- * distill some notion of a preferred network from the OS, upon which we can assume new sockets will
- * be opened.
+ * This class does the following.
+ * <ul>
+ * <li>When the internet is available: call the
+ * <code>InternalEngine::onDefaultNetworkAvailable</code> callback.</li>
+ *
+ * <li>When the internet is not available: call the
+ * <code>InternalEngine::onDefaultNetworkUnavailable</code> callback.</li>
+ *
+ * <li>When the capabilities are changed: call the
+ * <code>EnvoyEngine::onDefaultNetworkChanged</code>.</li>
+ * </ul>
  */
-@TargetApi(Build.VERSION_CODES.LOLLIPOP)
-public class AndroidNetworkMonitor extends BroadcastReceiver {
+public class AndroidNetworkMonitor {
   private static final String PERMISSION_DENIED_STATS_ELEMENT =
       "android_permissions.network_state_denied";
-
   private static volatile AndroidNetworkMonitor instance = null;
-
-  private int previousNetworkType = ConnectivityManager.TYPE_DUMMY;
-  private EnvoyEngine envoyEngine;
   private ConnectivityManager connectivityManager;
-  private NetworkCallback networkCallback;
-  private NetworkRequest networkRequest;
 
   public static void load(Context context, EnvoyEngine envoyEngine) {
     if (instance != null) {
@@ -61,6 +57,36 @@ public static void shutdown() {
     instance = null;
   }
 
+  private static class DefaultNetworkCallback extends NetworkCallback {
+    private final EnvoyEngine envoyEngine;
+
+    private DefaultNetworkCallback(EnvoyEngine envoyEngine) { this.envoyEngine = envoyEngine; }
+
+    @Override
+    public void onAvailable(@NonNull Network network) {
+      envoyEngine.onDefaultNetworkAvailable();
+    }
+
+    @Override
+    public void onCapabilitiesChanged(@NonNull Network network,
+                                      @NonNull NetworkCapabilities networkCapabilities) {
+      if (networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) {
+        if (networkCapabilities.hasCapability(NetworkCapabilities.TRANSPORT_WIFI)) {
+          envoyEngine.onDefaultNetworkChanged(EnvoyNetworkType.WLAN);
+        } else if (networkCapabilities.hasCapability(NetworkCapabilities.TRANSPORT_CELLULAR)) {
+          envoyEngine.onDefaultNetworkChanged(EnvoyNetworkType.WWAN);
+        } else {
+          envoyEngine.onDefaultNetworkChanged(EnvoyNetworkType.GENERIC);
+        }
+      }
+    }
+
+    @Override
+    public void onLost(@NonNull Network network) {
+      envoyEngine.onDefaultNetworkUnavailable();
+    }
+  }
+
   private AndroidNetworkMonitor(Context context, EnvoyEngine envoyEngine) {
     int permission =
         ContextCompat.checkSelfPermission(context, Manifest.permission.ACCESS_NETWORK_STATE);
@@ -73,42 +99,9 @@ private AndroidNetworkMonitor(Context context, EnvoyEngine envoyEngine) {
       return;
     }
 
-    this.envoyEngine = envoyEngine;
-
     connectivityManager =
         (ConnectivityManager)context.getSystemService(Context.CONNECTIVITY_SERVICE);
-    networkRequest = new NetworkRequest.Builder()
-                         .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
-                         .build();
-
-    networkCallback = new NetworkCallback() {
-      @Override
-      public void onAvailable(Network network) {
-        handleNetworkChange();
-      }
-      @Override
-      public void onCapabilitiesChanged(Network network, NetworkCapabilities networkCapabilities) {
-        handleNetworkChange();
-      }
-      @Override
-      public void onLosing(Network network, int maxMsToLive) {
-        handleNetworkChange();
-      }
-      @Override
-      public void onLost(final Network network) {
-        handleNetworkChange();
-      }
-    };
-
-    try {
-      connectivityManager.registerNetworkCallback(networkRequest, networkCallback);
-
-      context.registerReceiver(this, new IntentFilter() {
-        { addAction(ConnectivityManager.CONNECTIVITY_ACTION); }
-      });
-    } catch (Throwable t) {
-      // no-op
-    }
+    connectivityManager.registerDefaultNetworkCallback(new DefaultNetworkCallback(envoyEngine));
   }
 
   /** @returns The singleton instance of {@link AndroidNetworkMonitor}. */
@@ -117,32 +110,14 @@ public static AndroidNetworkMonitor getInstance() {
     return instance;
   }
 
-  @Override
-  public void onReceive(Context context, Intent intent) {
-    handleNetworkChange();
-  }
-
-  /** @returns True if there is connectivity */
-  public boolean isOnline() { return previousNetworkType != -1; }
-
-  private void handleNetworkChange() {
-    NetworkInfo networkInfo = connectivityManager.getActiveNetworkInfo();
-    int networkType = networkInfo == null ? -1 : networkInfo.getType();
-    if (networkType == previousNetworkType) {
-      return;
-    }
-    previousNetworkType = networkType;
-
-    switch (networkType) {
-    case ConnectivityManager.TYPE_MOBILE:
-      envoyEngine.setPreferredNetwork(EnvoyNetworkType.WWAN);
-      return;
-    case ConnectivityManager.TYPE_WIFI:
-      envoyEngine.setPreferredNetwork(EnvoyNetworkType.WLAN);
-      return;
-    default:
-      envoyEngine.setPreferredNetwork(EnvoyNetworkType.GENERIC);
-    }
+  /**
+   * Returns true if there is an internet connectivity.
+   */
+  public boolean isOnline() {
+    NetworkCapabilities networkCapabilities =
+        connectivityManager.getNetworkCapabilities(connectivityManager.getActiveNetwork());
+    return networkCapabilities != null &&
+        networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
   }
 
   /** Expose connectivityManager only for testing */

diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyEngine.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyEngine.java
@@ -63,12 +63,19 @@ public interface EnvoyEngine {
   void resetConnectivityState();
 
   /**
-   * Update the network interface to the preferred network for opening new
-   * streams.
-   *
-   * @param network The network to be preferred for new streams.
+   * A callback into the Envoy Engine when the default network is available.
+   */
+  void onDefaultNetworkAvailable();
+
+  /**
+   * A callback into the Envoy Engine when the default network configuration was changed.
+   */
+  void onDefaultNetworkChanged(EnvoyNetworkType network);
+
+  /**
+   * A callback into the Envoy Engine when the default network is unavailable.
    */
-  void setPreferredNetwork(EnvoyNetworkType network);
+  void onDefaultNetworkUnavailable();
 
   /**
    * Update proxy settings.

diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyEngineImpl.java
@@ -135,9 +135,21 @@ public void resetConnectivityState() {
   }
 
   @Override
-  public void setPreferredNetwork(EnvoyNetworkType network) {
+  public void onDefaultNetworkAvailable() {
     checkIsTerminated();
-    JniLibrary.setPreferredNetwork(engineHandle, network.getValue());
+    JniLibrary.onDefaultNetworkAvailable(engineHandle);
+  }
+
+  @Override
+  public void onDefaultNetworkChanged(EnvoyNetworkType network) {
+    checkIsTerminated();
+    JniLibrary.onDefaultNetworkChanged(engineHandle, network.getValue());
+  }
+
+  @Override
+  public void onDefaultNetworkUnavailable() {
+    checkIsTerminated();
+    JniLibrary.onDefaultNetworkUnavailable(engineHandle);
   }
 
   public void setProxySettings(String host, int port) {

diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java
@@ -225,14 +225,19 @@ protected static native int registerStringAccessor(String accessorName,
   protected static native int resetConnectivityState(long engine);
 
   /**
-   * Update the network interface to the preferred network for opening new
-   * streams. Note that this state is shared by all engines.
-   *
-   * @param engine  Handle to the engine whose preferred network will be set.
-   * @param network the network to be preferred for new streams.
-   * @return The resulting status of the operation.
+   * A callback into the Envoy Engine when the default network is available.
+   */
+  protected static native void onDefaultNetworkAvailable(long engine);
+
+  /**
+   * A callback into the Envoy Engine when the default network configuration was changed.
+   */
+  protected static native void onDefaultNetworkChanged(long engine, int networkType);
+
+  /**
+   * A callback into the Envoy Engine when the default network is unavailable.
    */
-  protected static native int setPreferredNetwork(long engine, int network);
+  protected static native void onDefaultNetworkUnavailable(long engine);
 
   /**
    * Update the proxy settings.

diff --git a/mobile/library/jni/jni_impl.cc b/mobile/library/jni/jni_impl.cc
@@ -1349,12 +1349,24 @@ Java_io_envoyproxy_envoymobile_engine_JniLibrary_resetConnectivityState(JNIEnv*
   return reinterpret_cast<Envoy::InternalEngine*>(engine)->resetConnectivityState();
 }
 
-extern "C" JNIEXPORT jint JNICALL
-Java_io_envoyproxy_envoymobile_engine_JniLibrary_setPreferredNetwork(JNIEnv* /*env*/,
-                                                                     jclass, // class
-                                                                     jlong engine, jint network) {
-  return reinterpret_cast<Envoy::InternalEngine*>(engine)->setPreferredNetwork(
-      static_cast<Envoy::NetworkType>(network));
+extern "C" JNIEXPORT void JNICALL
+Java_io_envoyproxy_envoymobile_engine_JniLibrary_onDefaultNetworkAvailable(JNIEnv*, jclass,
+                                                                           jlong engine) {
+  reinterpret_cast<Envoy::InternalEngine*>(engine)->onDefaultNetworkAvailable();
+}
+
+extern "C" JNIEXPORT void JNICALL
+Java_io_envoyproxy_envoymobile_engine_JniLibrary_onDefaultNetworkChanged(JNIEnv*, jclass,
+                                                                         jlong engine,
+                                                                         jint network_type) {
+  reinterpret_cast<Envoy::InternalEngine*>(engine)->onDefaultNetworkChanged(
+      static_cast<Envoy::NetworkType>(network_type));
+}
+
+extern "C" JNIEXPORT void JNICALL
+Java_io_envoyproxy_envoymobile_engine_JniLibrary_onDefaultNetworkUnavailable(JNIEnv*, jclass,
+                                                                             jlong engine) {
+  reinterpret_cast<Envoy::InternalEngine*>(engine)->onDefaultNetworkUnavailable();
 }
 
 extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_setProxySettings(

diff --git a/mobile/library/objective-c/EnvoyNetworkMonitor.mm b/mobile/library/objective-c/EnvoyNetworkMonitor.mm
@@ -66,7 +66,7 @@ - (void)startPathMonitor {
 
     if (network != previousNetworkType) {
       NSLog(@"[Envoy] setting preferred network to %d", network);
-      engine->setPreferredNetwork(network);
+      engine->onDefaultNetworkChanged(network);
       previousNetworkType = network;
     }
 
@@ -135,8 +135,8 @@ static void _reachability_callback(SCNetworkReachabilityRef target,
 
   NSLog(@"[Envoy] setting preferred network to %@", isUsingWWAN ? @"WWAN" : @"WLAN");
   EnvoyNetworkMonitor *monitor = (__bridge EnvoyNetworkMonitor *)info;
-  monitor->_engine->setPreferredNetwork(isUsingWWAN ? Envoy::NetworkType::WWAN
-                                                    : Envoy::NetworkType::WLAN);
+  monitor->_engine->onDefaultNetworkChanged(isUsingWWAN ? Envoy::NetworkType::WWAN
+                                                        : Envoy::NetworkType::WLAN);
 }
 
 @end