From c0618a69f127451597d3d2ae2d3c292fb4e80d79 Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Wed, 10 Dec 2014 15:12:18 -0500 Subject: [PATCH] Fix several HTTP proxy issues with multinetworking. 1. Send PROXY_CHANGE_ACTION broadcast when any network's proxy changes, not just the default network. 2. When a process is bound to a particular Network, update the proxy system properties to those for the bound Network, and keep them updated when PROXY_CHANGE_ACTION broadcasts are received. 3. Make Network.openConnection() use the proxy for the Network. bug:17905627 bug:17420465 bug:18144582 (cherry-pick of https://android-review.googlesource.com/#/c/115170) Change-Id: Ia2819985e6108a8c121e74c683a5646becfd0a97 --- .../java/android/net/ConnectivityManager.java | 47 +++++++++++++---- .../android/net/IConnectivityManager.aidl | 2 +- core/java/android/net/Network.java | 37 ++++++++++++-- core/java/android/net/ProxyInfo.java | 3 +- .../android/server/ConnectivityService.java | 50 ++++++++++++++++++- 5 files changed, 123 insertions(+), 16 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 4215f207ea..e0f6b8f4c0 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -452,6 +452,13 @@ public class ConnectivityManager { public static final int NETID_UNSET = 0; private final IConnectivityManager mService; + /** + * A kludge to facilitate static access where a Context pointer isn't available, like in the + * case of the static set/getProcessDefaultNetwork methods and from the Network class. + * TODO: Remove this after deprecating the static methods in favor of non-static methods or + * methods that take a Context argument. + */ + private static ConnectivityManager sInstance; private INetworkManagementService mNMService; @@ -1397,6 +1404,7 @@ public class ConnectivityManager { */ public ConnectivityManager(IConnectivityManager service) { mService = checkNotNull(service, "missing IConnectivityManager"); + sInstance = this; } /** {@hide} */ @@ -1418,6 +1426,18 @@ public class ConnectivityManager { } } + /** + * @deprecated - use getSystemService. This is a kludge to support static access in certain + * situations where a Context pointer is unavailable. + * @hide + */ + public static ConnectivityManager getInstance() { + if (sInstance == null) { + throw new IllegalStateException("No ConnectivityManager yet constructed"); + } + return sInstance; + } + /** * Get the set of tetherable, available interfaces. This list is limited by * device configuration and current interface existence. @@ -1749,20 +1769,26 @@ public class ConnectivityManager { } /** - * Get the HTTP proxy settings for the current default network. Note that - * if a global proxy is set, it will override any per-network setting. + * Get the current default HTTP proxy settings. If a global proxy is set it will be returned, + * otherwise if this process is bound to a {@link Network} using + * {@link #setProcessDefaultNetwork} then that {@code Network}'s proxy is returned, otherwise + * the default network's proxy is returned. * * @return the {@link ProxyInfo} for the current HTTP proxy, or {@code null} if no * HTTP proxy is active. - * - *

This method requires the call to hold the permission - * {@link android.Manifest.permission#ACCESS_NETWORK_STATE}. - * {@hide} - * @deprecated Deprecated in favor of {@link #getLinkProperties} + * @hide */ - public ProxyInfo getProxy() { + public ProxyInfo getDefaultProxy() { + final Network network = getProcessDefaultNetwork(); + if (network != null) { + final ProxyInfo globalProxy = getGlobalProxy(); + if (globalProxy != null) return globalProxy; + final LinkProperties lp = getLinkProperties(network); + if (lp != null) return lp.getHttpProxy(); + return null; + } try { - return mService.getProxy(); + return mService.getDefaultProxy(); } catch (RemoteException e) { return null; } @@ -2475,6 +2501,9 @@ public class ConnectivityManager { return true; } if (NetworkUtils.bindProcessToNetwork(netId)) { + // Set HTTP proxy system properties to match network. + // TODO: Deprecate this static method and replace it with a non-static version. + Proxy.setHttpProxySystemProperty(getInstance().getDefaultProxy()); // Must flush DNS cache as new network may have different DNS resolutions. InetAddress.clearDnsCache(); // Must flush socket pool as idle sockets will be bound to previous network and may diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index d9921a6df0..46af112d1d 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -100,7 +100,7 @@ interface IConnectivityManager void setGlobalProxy(in ProxyInfo p); - ProxyInfo getProxy(); + ProxyInfo getDefaultProxy(); void setDataDependency(int networkType, boolean met); diff --git a/core/java/android/net/Network.java b/core/java/android/net/Network.java index 4fa0593c5e..5c126964ad 100644 --- a/core/java/android/net/Network.java +++ b/core/java/android/net/Network.java @@ -27,6 +27,7 @@ import java.net.DatagramSocket; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.MalformedURLException; +import java.net.ProxySelector; import java.net.Socket; import java.net.SocketAddress; import java.net.SocketException; @@ -244,16 +245,46 @@ public class Network implements Parcelable { * @see java.net.URL#openConnection() */ public URLConnection openConnection(URL url) throws IOException { + final ConnectivityManager cm = ConnectivityManager.getInstance(); + // TODO: Should this be optimized to avoid fetching the global proxy for every request? + ProxyInfo proxyInfo = cm.getGlobalProxy(); + if (proxyInfo == null) { + // TODO: Should this be optimized to avoid fetching LinkProperties for every request? + final LinkProperties lp = cm.getLinkProperties(this); + if (lp != null) proxyInfo = lp.getHttpProxy(); + } + java.net.Proxy proxy = null; + if (proxyInfo != null) { + proxy = proxyInfo.makeProxy(); + } else { + proxy = java.net.Proxy.NO_PROXY; + } + return openConnection(url, proxy); + } + + /** + * Opens the specified {@link URL} on this {@code Network}, such that all traffic will be sent + * on this Network. The URL protocol must be {@code HTTP} or {@code HTTPS}. + * + * @param proxy the proxy through which the connection will be established. + * @return a {@code URLConnection} to the resource referred to by this URL. + * @throws MalformedURLException if the URL protocol is not HTTP or HTTPS. + * @throws IllegalArgumentException if the argument proxy is null. + * @throws IOException if an error occurs while opening the connection. + * @see java.net.URL#openConnection() + * @hide + */ + public URLConnection openConnection(URL url, java.net.Proxy proxy) throws IOException { + if (proxy == null) throw new IllegalArgumentException("proxy is null"); maybeInitHttpClient(); String protocol = url.getProtocol(); OkHttpClient client; // TODO: HttpHandler creates OkHttpClients that share the default ResponseCache. // Could this cause unexpected behavior? - // TODO: Should the network's proxy be specified? if (protocol.equals("http")) { - client = HttpHandler.createHttpOkHttpClient(null /* proxy */); + client = HttpHandler.createHttpOkHttpClient(proxy); } else if (protocol.equals("https")) { - client = HttpsHandler.createHttpsOkHttpClient(null /* proxy */); + client = HttpsHandler.createHttpsOkHttpClient(proxy); } else { // OkHttpClient only supports HTTP and HTTPS and returns a null URLStreamHandler if // passed another protocol. diff --git a/core/java/android/net/ProxyInfo.java b/core/java/android/net/ProxyInfo.java index 7694420e70..a3cad77fa9 100644 --- a/core/java/android/net/ProxyInfo.java +++ b/core/java/android/net/ProxyInfo.java @@ -260,7 +260,8 @@ public class ProxyInfo implements Parcelable { if (!Uri.EMPTY.equals(mPacFileUrl)) { sb.append("PAC Script: "); sb.append(mPacFileUrl); - } else if (mHost != null) { + } + if (mHost != null) { sb.append("["); sb.append(mHost); sb.append("] "); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index a86d564f6b..7a60beb872 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2618,7 +2618,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - public ProxyInfo getProxy() { + public ProxyInfo getDefaultProxy() { // this information is already available as a world read/writable jvm property // so this API change wouldn't have a benifit. It also breaks the passing // of proxy info to all the JVMs. @@ -2630,6 +2630,34 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + // Convert empty ProxyInfo's to null as null-checks are used to determine if proxies are present + // (e.g. if mGlobalProxy==null fall back to network-specific proxy, if network-specific + // proxy is null then there is no proxy in place). + private ProxyInfo canonicalizeProxyInfo(ProxyInfo proxy) { + if (proxy != null && TextUtils.isEmpty(proxy.getHost()) + && (proxy.getPacFileUrl() == null || Uri.EMPTY.equals(proxy.getPacFileUrl()))) { + proxy = null; + } + return proxy; + } + + // ProxyInfo equality function with a couple modifications over ProxyInfo.equals() to make it + // better for determining if a new proxy broadcast is necessary: + // 1. Canonicalize empty ProxyInfos to null so an empty proxy compares equal to null so as to + // avoid unnecessary broadcasts. + // 2. Make sure all parts of the ProxyInfo's compare true, including the host when a PAC URL + // is in place. This is important so legacy PAC resolver (see com.android.proxyhandler) + // changes aren't missed. The legacy PAC resolver pretends to be a simple HTTP proxy but + // actually uses the PAC to resolve; this results in ProxyInfo's with PAC URL, host and port + // all set. + private boolean proxyInfoEqual(ProxyInfo a, ProxyInfo b) { + a = canonicalizeProxyInfo(a); + b = canonicalizeProxyInfo(b); + // ProxyInfo.equals() doesn't check hosts when PAC URLs are present, but we need to check + // hosts even when PAC URLs are present to account for the legacy PAC resolver. + return Objects.equals(a, b) && (a == null || Objects.equals(a.getHost(), b.getHost())); + } + public void setGlobalProxy(ProxyInfo proxyProperties) { enforceConnectivityInternalPermission(); @@ -2747,6 +2775,20 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + // If the proxy has changed from oldLp to newLp, resend proxy broadcast with default proxy. + // This method gets called when any network changes proxy, but the broadcast only ever contains + // the default proxy (even if it hasn't changed). + // TODO: Deprecate the broadcast extras as they aren't necessarily applicable in a multi-network + // world where an app might be bound to a non-default network. + private void updateProxy(LinkProperties newLp, LinkProperties oldLp, NetworkAgentInfo nai) { + ProxyInfo newProxyInfo = newLp == null ? null : newLp.getHttpProxy(); + ProxyInfo oldProxyInfo = oldLp == null ? null : oldLp.getHttpProxy(); + + if (!proxyInfoEqual(newProxyInfo, oldProxyInfo)) { + sendProxyBroadcast(getDefaultProxy()); + } + } + private void handleDeprecatedGlobalHttpProxy() { String proxy = Settings.Global.getString(mContext.getContentResolver(), Settings.Global.HTTP_PROXY); @@ -3660,7 +3702,11 @@ public class ConnectivityService extends IConnectivityManager.Stub updateDnses(newLp, oldLp, netId, flushDns, useDefaultDns); updateClat(newLp, oldLp, networkAgent); - if (isDefaultNetwork(networkAgent)) handleApplyDefaultProxy(newLp.getHttpProxy()); + if (isDefaultNetwork(networkAgent)) { + handleApplyDefaultProxy(newLp.getHttpProxy()); + } else { + updateProxy(newLp, oldLp, networkAgent); + } // TODO - move this check to cover the whole function if (!Objects.equals(newLp, oldLp)) { notifyIfacesChanged();