From d51966a41b1a12750b72bd03e6b581be2284bb33 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 13:27:00 +0900 Subject: [PATCH 1/9] Cleanup of LinkProperties. Test: runtest Change-Id: I7299de93a79901635ce755a2d933666ee43767d5 --- core/java/android/net/LinkProperties.java | 185 ++++++++++++---------- 1 file changed, 98 insertions(+), 87 deletions(-) diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index ab29d9950e..bd2db92b78 100644 --- a/core/java/android/net/LinkProperties.java +++ b/core/java/android/net/LinkProperties.java @@ -32,6 +32,7 @@ import java.util.Collections; import java.util.Hashtable; import java.util.List; import java.util.Objects; +import java.util.StringJoiner; /** * Describes the properties of a network link. @@ -48,13 +49,13 @@ import java.util.Objects; public final class LinkProperties implements Parcelable { // The interface described by the network link. private String mIfaceName; - private ArrayList mLinkAddresses = new ArrayList(); - private ArrayList mDnses = new ArrayList(); - private ArrayList mValidatedPrivateDnses = new ArrayList(); + private ArrayList mLinkAddresses = new ArrayList<>(); + private ArrayList mDnses = new ArrayList<>(); + private ArrayList mValidatedPrivateDnses = new ArrayList<>(); private boolean mUsePrivateDns; private String mPrivateDnsServerName; private String mDomains; - private ArrayList mRoutes = new ArrayList(); + private ArrayList mRoutes = new ArrayList<>(); private ProxyInfo mHttpProxy; private int mMtu; // in the format "rmem_min,rmem_def,rmem_max,wmem_min,wmem_def,wmem_max" @@ -66,15 +67,14 @@ public final class LinkProperties implements Parcelable { // Stores the properties of links that are "stacked" above this link. // Indexed by interface name to allow modification and to prevent duplicates being added. - private Hashtable mStackedLinks = - new Hashtable(); + private Hashtable mStackedLinks = new Hashtable<>(); /** * @hide */ public static class CompareResult { - public final List removed = new ArrayList(); - public final List added = new ArrayList(); + public final List removed = new ArrayList<>(); + public final List added = new ArrayList<>(); public CompareResult() {} @@ -93,12 +93,9 @@ public final class LinkProperties implements Parcelable { @Override public String toString() { - String retVal = "removed=["; - for (T addr : removed) retVal += addr.toString() + ","; - retVal += "] added=["; - for (T addr : added) retVal += addr.toString() + ","; - retVal += "]"; - return retVal; + return "removed=[" + TextUtils.join(",", removed) + + "] added=[" + TextUtils.join(",", added) + + "]"; } } @@ -120,7 +117,7 @@ public final class LinkProperties implements Parcelable { public static ProvisioningChange compareProvisioning( LinkProperties before, LinkProperties after) { if (before.isProvisioned() && after.isProvisioned()) { - // On dualstack networks, DHCPv4 renewals can occasionally fail. + // On dual-stack networks, DHCPv4 renewals can occasionally fail. // When this happens, IPv6-reachable services continue to function // normally but IPv4-only services (naturally) fail. // @@ -131,7 +128,7 @@ public final class LinkProperties implements Parcelable { // // For users, this is confusing and unexpected behaviour, and is // not necessarily easy to diagnose. Therefore, we treat changing - // from a dualstack network to an IPv6-only network equivalent to + // from a dual-stack network to an IPv6-only network equivalent to // a total loss of provisioning. // // For one such example of this, see b/18867306. @@ -139,7 +136,7 @@ public final class LinkProperties implements Parcelable { // Additionally, losing IPv6 provisioning can result in TCP // connections getting stuck until timeouts fire and other // baffling failures. Therefore, loss of either IPv4 or IPv6 on a - // previously dualstack network is deemed a lost of provisioning. + // previously dual-stack network is deemed a lost of provisioning. if ((before.isIPv4Provisioned() && !after.isIPv4Provisioned()) || (before.isIPv6Provisioned() && !after.isIPv6Provisioned())) { return ProvisioningChange.LOST_PROVISIONING; @@ -165,22 +162,19 @@ public final class LinkProperties implements Parcelable { */ public LinkProperties(LinkProperties source) { if (source != null) { - mIfaceName = source.getInterfaceName(); - for (LinkAddress l : source.getLinkAddresses()) mLinkAddresses.add(l); - for (InetAddress i : source.getDnsServers()) mDnses.add(i); - for (InetAddress i : source.getValidatedPrivateDnsServers()) { - mValidatedPrivateDnses.add(i); - } + mIfaceName = source.mIfaceName; + mLinkAddresses.addAll(source.mLinkAddresses); + mDnses.addAll(source.mDnses); + mValidatedPrivateDnses.addAll(source.mValidatedPrivateDnses); mUsePrivateDns = source.mUsePrivateDns; mPrivateDnsServerName = source.mPrivateDnsServerName; - mDomains = source.getDomains(); - for (RouteInfo r : source.getRoutes()) mRoutes.add(r); - mHttpProxy = (source.getHttpProxy() == null) ? - null : new ProxyInfo(source.getHttpProxy()); + mDomains = source.mDomains; + mRoutes.addAll(source.mRoutes); + mHttpProxy = (source.mHttpProxy == null) ? null : new ProxyInfo(source.mHttpProxy); for (LinkProperties l: source.mStackedLinks.values()) { addStackedLink(l); } - setMtu(source.getMtu()); + setMtu(source.mMtu); mTcpBufferSizes = source.mTcpBufferSizes; } } @@ -194,7 +188,7 @@ public final class LinkProperties implements Parcelable { */ public void setInterfaceName(String iface) { mIfaceName = iface; - ArrayList newRoutes = new ArrayList(mRoutes.size()); + ArrayList newRoutes = new ArrayList<>(mRoutes.size()); for (RouteInfo route : mRoutes) { newRoutes.add(routeWithInterface(route)); } @@ -214,8 +208,8 @@ public final class LinkProperties implements Parcelable { * @hide */ public List getAllInterfaceNames() { - List interfaceNames = new ArrayList(mStackedLinks.size() + 1); - if (mIfaceName != null) interfaceNames.add(new String(mIfaceName)); + List interfaceNames = new ArrayList<>(mStackedLinks.size() + 1); + if (mIfaceName != null) interfaceNames.add(mIfaceName); for (LinkProperties stacked: mStackedLinks.values()) { interfaceNames.addAll(stacked.getAllInterfaceNames()); } @@ -229,11 +223,11 @@ public final class LinkProperties implements Parcelable { * prefix lengths for each address. This is a simplified utility alternative to * {@link LinkProperties#getLinkAddresses}. * - * @return An umodifiable {@link List} of {@link InetAddress} for this link. + * @return An unmodifiable {@link List} of {@link InetAddress} for this link. * @hide */ public List getAddresses() { - List addresses = new ArrayList(); + List addresses = new ArrayList<>(); for (LinkAddress linkAddress : mLinkAddresses) { addresses.add(linkAddress.getAddress()); } @@ -245,7 +239,7 @@ public final class LinkProperties implements Parcelable { * @hide */ public List getAllAddresses() { - List addresses = new ArrayList(); + List addresses = new ArrayList<>(); for (LinkAddress linkAddress : mLinkAddresses) { addresses.add(linkAddress.getAddress()); } @@ -322,8 +316,7 @@ public final class LinkProperties implements Parcelable { * @hide */ public List getAllLinkAddresses() { - List addresses = new ArrayList(); - addresses.addAll(mLinkAddresses); + List addresses = new ArrayList<>(mLinkAddresses); for (LinkProperties stacked: mStackedLinks.values()) { addresses.addAll(stacked.getAllLinkAddresses()); } @@ -391,7 +384,7 @@ public final class LinkProperties implements Parcelable { /** * Returns all the {@link InetAddress} for DNS servers on this link. * - * @return An umodifiable {@link List} of {@link InetAddress} for DNS servers on + * @return An unmodifiable {@link List} of {@link InetAddress} for DNS servers on * this link. */ public List getDnsServers() { @@ -643,7 +636,7 @@ public final class LinkProperties implements Parcelable { * @hide */ public void ensureDirectlyConnectedRoutes() { - for (LinkAddress addr: mLinkAddresses) { + for (LinkAddress addr : mLinkAddresses) { addRoute(new RouteInfo(addr, null, mIfaceName)); } } @@ -653,8 +646,7 @@ public final class LinkProperties implements Parcelable { * @hide */ public List getAllRoutes() { - List routes = new ArrayList<>(); - routes.addAll(mRoutes); + List routes = new ArrayList<>(mRoutes); for (LinkProperties stacked: mStackedLinks.values()) { routes.addAll(stacked.getAllRoutes()); } @@ -685,7 +677,7 @@ public final class LinkProperties implements Parcelable { /** * Adds a stacked link. * - * If there is already a stacked link with the same interfacename as link, + * If there is already a stacked link with the same interface name as link, * that link is replaced with link. Otherwise, link is added to the list * of stacked links. If link is null, nothing changes. * @@ -725,9 +717,9 @@ public final class LinkProperties implements Parcelable { */ public @NonNull List getStackedLinks() { if (mStackedLinks.isEmpty()) { - return Collections.EMPTY_LIST; + return Collections.emptyList(); } - List stacked = new ArrayList(); + List stacked = new ArrayList<>(); for (LinkProperties link : mStackedLinks.values()) { stacked.add(new LinkProperties(link)); } @@ -761,57 +753,76 @@ public final class LinkProperties implements Parcelable { @Override public String toString() { - String ifaceName = (mIfaceName == null ? "" : "InterfaceName: " + mIfaceName + " "); + // Space as a separator, so no need for spaces at start/end of the individual fragments. + final StringJoiner resultJoiner = new StringJoiner(" ", "{", "}"); - String linkAddresses = "LinkAddresses: ["; - for (LinkAddress addr : mLinkAddresses) linkAddresses += addr.toString() + ","; - linkAddresses += "] "; + if (mIfaceName != null) { + resultJoiner.add("InterfaceName:"); + resultJoiner.add(mIfaceName); + } - String dns = "DnsAddresses: ["; - for (InetAddress addr : mDnses) dns += addr.getHostAddress() + ","; - dns += "] "; + resultJoiner.add("LinkAddresses: ["); + if (!mLinkAddresses.isEmpty()) { + resultJoiner.add(TextUtils.join(",", mLinkAddresses)); + } + resultJoiner.add("]"); - String usePrivateDns = "UsePrivateDns: " + mUsePrivateDns + " "; + resultJoiner.add("DnsAddresses: ["); + if (!mDnses.isEmpty()) { + resultJoiner.add(TextUtils.join(",", mDnses)); + } + resultJoiner.add("]"); + + if (mUsePrivateDns) { + resultJoiner.add("UsePrivateDns: true"); + } - String privateDnsServerName = ""; if (mPrivateDnsServerName != null) { - privateDnsServerName = "PrivateDnsServerName: " + mPrivateDnsServerName + " "; + resultJoiner.add("PrivateDnsServerName:"); + resultJoiner.add(mPrivateDnsServerName); } - String validatedPrivateDns = ""; if (!mValidatedPrivateDnses.isEmpty()) { - validatedPrivateDns = "ValidatedPrivateDnsAddresses: ["; - for (InetAddress addr : mValidatedPrivateDnses) { - validatedPrivateDns += addr.getHostAddress() + ","; + final StringJoiner validatedPrivateDnsesJoiner = + new StringJoiner(",", "ValidatedPrivateDnsAddresses: [", "]"); + for (final InetAddress addr : mValidatedPrivateDnses) { + validatedPrivateDnsesJoiner.add(addr.getHostAddress()); } - validatedPrivateDns += "] "; + resultJoiner.add(validatedPrivateDnsesJoiner.toString()); } - String domainName = "Domains: " + mDomains; + resultJoiner.add("Domains:"); + resultJoiner.add(mDomains); - String mtu = " MTU: " + mMtu; + resultJoiner.add("MTU:"); + resultJoiner.add(Integer.toString(mMtu)); - String tcpBuffSizes = ""; if (mTcpBufferSizes != null) { - tcpBuffSizes = " TcpBufferSizes: " + mTcpBufferSizes; + resultJoiner.add("TcpBufferSizes:"); + resultJoiner.add(mTcpBufferSizes); } - String routes = " Routes: ["; - for (RouteInfo route : mRoutes) routes += route.toString() + ","; - routes += "] "; - String proxy = (mHttpProxy == null ? "" : " HttpProxy: " + mHttpProxy.toString() + " "); + resultJoiner.add("Routes: ["); + if (!mRoutes.isEmpty()) { + resultJoiner.add(TextUtils.join(",", mRoutes)); + } + resultJoiner.add("]"); - String stacked = ""; - if (mStackedLinks.values().size() > 0) { - stacked += " Stacked: ["; - for (LinkProperties link: mStackedLinks.values()) { - stacked += " [" + link.toString() + " ],"; + if (mHttpProxy != null) { + resultJoiner.add("HttpProxy:"); + resultJoiner.add(mHttpProxy.toString()); + } + + final Collection stackedLinksValues = mStackedLinks.values(); + if (!stackedLinksValues.isEmpty()) { + final StringJoiner stackedLinksJoiner = new StringJoiner(",", "Stacked: [", "]"); + for (final LinkProperties lp : stackedLinksValues) { + stackedLinksJoiner.add("[ " + lp + " ]"); } - stacked += "] "; + resultJoiner.add(stackedLinksJoiner.toString()); } - return "{" + ifaceName + linkAddresses + routes + dns + usePrivateDns - + privateDnsServerName + validatedPrivateDns + domainName + mtu + tcpBuffSizes + proxy - + stacked + "}"; + + return resultJoiner.toString(); } /** @@ -1028,7 +1039,7 @@ public final class LinkProperties implements Parcelable { if (mDomains == null) { if (targetDomains != null) return false; } else { - if (mDomains.equals(targetDomains) == false) return false; + if (!mDomains.equals(targetDomains)) return false; } return (mDnses.size() == targetDnses.size()) ? mDnses.containsAll(targetDnses) : false; @@ -1130,7 +1141,6 @@ public final class LinkProperties implements Parcelable { return Objects.equals(mTcpBufferSizes, target.mTcpBufferSizes); } - @Override /** * Compares this {@code LinkProperties} instance against the target * LinkProperties in {@code obj}. Two LinkPropertieses are equal if @@ -1145,13 +1155,14 @@ public final class LinkProperties implements Parcelable { * @param obj the object to be tested for equality. * @return {@code true} if both objects are equal, {@code false} otherwise. */ + @Override public boolean equals(Object obj) { if (this == obj) return true; if (!(obj instanceof LinkProperties)) return false; LinkProperties target = (LinkProperties) obj; - /** + /* * This method does not check that stacked interfaces are equal, because * stacked interfaces are not so much a property of the link as a * description of connections between links. @@ -1258,12 +1269,13 @@ public final class LinkProperties implements Parcelable { } - @Override /** - * generate hashcode based on significant fields + * Generate hashcode based on significant fields + * * Equal objects must produce the same hash code, while unequal objects * may have the same hash codes. */ + @Override public int hashCode() { return ((null == mIfaceName) ? 0 : mIfaceName.hashCode() + mLinkAddresses.size() * 31 @@ -1313,7 +1325,7 @@ public final class LinkProperties implements Parcelable { } else { dest.writeByte((byte)0); } - ArrayList stackedLinks = new ArrayList(mStackedLinks.values()); + ArrayList stackedLinks = new ArrayList<>(mStackedLinks.values()); dest.writeList(stackedLinks); } @@ -1331,7 +1343,7 @@ public final class LinkProperties implements Parcelable { } int addressCount = in.readInt(); for (int i = 0; i < addressCount; i++) { - netProp.addLinkAddress((LinkAddress) in.readParcelable(null)); + netProp.addLinkAddress(in.readParcelable(null)); } addressCount = in.readInt(); for (int i = 0; i < addressCount; i++) { @@ -1353,10 +1365,10 @@ public final class LinkProperties implements Parcelable { netProp.setTcpBufferSizes(in.readString()); addressCount = in.readInt(); for (int i = 0; i < addressCount; i++) { - netProp.addRoute((RouteInfo) in.readParcelable(null)); + netProp.addRoute(in.readParcelable(null)); } if (in.readByte() == 1) { - netProp.setHttpProxy((ProxyInfo) in.readParcelable(null)); + netProp.setHttpProxy(in.readParcelable(null)); } ArrayList stackedLinks = new ArrayList(); in.readList(stackedLinks, LinkProperties.class.getClassLoader()); @@ -1377,10 +1389,9 @@ public final class LinkProperties implements Parcelable { */ public static boolean isValidMtu(int mtu, boolean ipv6) { if (ipv6) { - if (mtu >= MIN_MTU_V6 && mtu <= MAX_MTU) return true; + return mtu >= MIN_MTU_V6 && mtu <= MAX_MTU; } else { - if (mtu >= MIN_MTU && mtu <= MAX_MTU) return true; + return mtu >= MIN_MTU && mtu <= MAX_MTU; } - return false; } } From 040a41f850cc90a12183e55f241fd37c6cd3c800 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 13:46:52 +0900 Subject: [PATCH 2/9] Small cleanup of Network. Test: runtest Change-Id: I56dbd37bb8f890938d360f45835de72be4beb91a --- core/java/android/net/Network.java | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/core/java/android/net/Network.java b/core/java/android/net/Network.java index 8f956075d2..d1cccac243 100644 --- a/core/java/android/net/Network.java +++ b/core/java/android/net/Network.java @@ -136,7 +136,7 @@ public class Network implements Parcelable { /** * Specify whether or not Private DNS should be bypassed when attempting - * to use {@link getAllByName()}/{@link getByName()} methods on the given + * to use {@link #getAllByName(String)}/{@link #getByName(String)} methods on the given * instance for hostname resolution. * * @hide @@ -163,13 +163,6 @@ public class Network implements Parcelable { * A {@code SocketFactory} that produces {@code Socket}'s bound to this network. */ private class NetworkBoundSocketFactory extends SocketFactory { - private final int mNetId; - - public NetworkBoundSocketFactory(int netId) { - super(); - mNetId = netId; - } - private Socket connectToHost(String host, int port, SocketAddress localAddress) throws IOException { // Lookup addresses only on this Network. @@ -195,7 +188,8 @@ public class Network implements Parcelable { } @Override - public Socket createSocket(String host, int port, InetAddress localHost, int localPort) throws IOException { + public Socket createSocket(String host, int port, InetAddress localHost, int localPort) + throws IOException { return connectToHost(host, port, new InetSocketAddress(localHost, localPort)); } @@ -259,7 +253,7 @@ public class Network implements Parcelable { if (mNetworkBoundSocketFactory == null) { synchronized (mLock) { if (mNetworkBoundSocketFactory == null) { - mNetworkBoundSocketFactory = new NetworkBoundSocketFactory(netId); + mNetworkBoundSocketFactory = new NetworkBoundSocketFactory(); } } } @@ -454,7 +448,7 @@ public class Network implements Parcelable { @Override public boolean equals(Object obj) { - if (obj instanceof Network == false) return false; + if (!(obj instanceof Network)) return false; Network other = (Network)obj; return this.netId == other.netId; } From 0ceb26580f5b7c5c78571fa911fe0e96d2d2b950 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 16:11:34 +0900 Subject: [PATCH 3/9] Remove a useless parameter. This argument is useless because all its callers pass the same value : false. Test: runtest Change-Id: Id921caa4ffadef535a5bbcfea052283a07320b28 --- .../core/java/com/android/server/ConnectivityService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index e3e0d7906b..ec11bd9374 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -971,7 +971,7 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new IllegalStateException("No free netIds"); } - private NetworkState getFilteredNetworkState(int networkType, int uid, boolean ignoreBlocked) { + private NetworkState getFilteredNetworkState(int networkType, int uid) { if (mLegacyTypeTracker.isTypeSupported(networkType)) { final NetworkAgentInfo nai = mLegacyTypeTracker.getNetworkForType(networkType); final NetworkState state; @@ -989,7 +989,7 @@ public class ConnectivityService extends IConnectivityManager.Stub state = new NetworkState(info, new LinkProperties(), capabilities, null, null, null); } - filterNetworkStateForUid(state, uid, ignoreBlocked); + filterNetworkStateForUid(state, uid, false); return state; } else { return NetworkState.EMPTY; @@ -1194,7 +1194,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return state.networkInfo; } } - final NetworkState state = getFilteredNetworkState(networkType, uid, false); + final NetworkState state = getFilteredNetworkState(networkType, uid); return state.networkInfo; } @@ -1229,7 +1229,7 @@ public class ConnectivityService extends IConnectivityManager.Stub public Network getNetworkForType(int networkType) { enforceAccessPermission(); final int uid = Binder.getCallingUid(); - NetworkState state = getFilteredNetworkState(networkType, uid, false); + NetworkState state = getFilteredNetworkState(networkType, uid); if (!isNetworkWithLinkPropertiesBlocked(state.linkProperties, uid, false)) { return state.network; } From 5d70ba4502771db8871d9e7278f192d89f24c074 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 16:44:04 +0900 Subject: [PATCH 4/9] [PT01] Introduce ProxyTracker The goal of this is to simplify ConnectivityService by reducing the amount of code it contains. This is small enough to be obviously correct and followup changes will move code into this class. Test: runtest Change-Id: Ic5ab19b521e98ae397c9bf657856820304362fbb --- .../android/server/ConnectivityService.java | 96 ++++++++++--------- .../server/connectivity/ProxyTracker.java | 43 +++++++++ 2 files changed, 95 insertions(+), 44 deletions(-) create mode 100644 services/core/java/com/android/server/connectivity/ProxyTracker.java diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index ec11bd9374..05b929f158 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -153,6 +153,7 @@ import com.android.server.connectivity.NetworkNotificationManager; import com.android.server.connectivity.NetworkNotificationManager.NotificationType; import com.android.server.connectivity.PacManager; import com.android.server.connectivity.PermissionMonitor; +import com.android.server.connectivity.ProxyTracker; import com.android.server.connectivity.Tethering; import com.android.server.connectivity.Vpn; import com.android.server.connectivity.tethering.TetheringDependencies; @@ -426,13 +427,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private int mNetTransitionWakeLockTimeout; private final PowerManager.WakeLock mPendingIntentWakeLock; - // track the current default http proxy - tell the world if we get a new one (real change) - private volatile ProxyInfo mDefaultProxy = null; - private final Object mProxyLock = new Object(); - private boolean mDefaultProxyDisabled = false; - - // track the global proxy. - private ProxyInfo mGlobalProxy = null; + // A helper object to track the current default HTTP proxy. ConnectivityService needs to tell + // the world when it changes. + private final ProxyTracker mProxyTracker = new ProxyTracker(); private PacManager mPacManager = null; @@ -3286,9 +3283,11 @@ public class ConnectivityService extends IConnectivityManager.Stub // so this API change wouldn't have a benefit. It also breaks the passing // of proxy info to all the JVMs. // enforceAccessPermission(); - synchronized (mProxyLock) { - ProxyInfo ret = mGlobalProxy; - if ((ret == null) && !mDefaultProxyDisabled) ret = mDefaultProxy; + synchronized (mProxyTracker.mProxyLock) { + ProxyInfo ret = mProxyTracker.mGlobalProxy; + if ((ret == null) && !mProxyTracker.mDefaultProxyDisabled) { + ret = mProxyTracker.mDefaultProxy; + } return ret; } } @@ -3341,10 +3340,15 @@ public class ConnectivityService extends IConnectivityManager.Stub public void setGlobalProxy(ProxyInfo proxyProperties) { enforceConnectivityInternalPermission(); - synchronized (mProxyLock) { - if (proxyProperties == mGlobalProxy) return; - if (proxyProperties != null && proxyProperties.equals(mGlobalProxy)) return; - if (mGlobalProxy != null && mGlobalProxy.equals(proxyProperties)) return; + synchronized (mProxyTracker.mProxyLock) { + if (proxyProperties == mProxyTracker.mGlobalProxy) return; + if (proxyProperties != null && proxyProperties.equals(mProxyTracker.mGlobalProxy)) { + return; + } + if (mProxyTracker.mGlobalProxy != null + && mProxyTracker.mGlobalProxy.equals(proxyProperties)) { + return; + } String host = ""; int port = 0; @@ -3357,15 +3361,15 @@ public class ConnectivityService extends IConnectivityManager.Stub log("Invalid proxy properties, ignoring: " + proxyProperties.toString()); return; } - mGlobalProxy = new ProxyInfo(proxyProperties); - host = mGlobalProxy.getHost(); - port = mGlobalProxy.getPort(); - exclList = mGlobalProxy.getExclusionListAsString(); + mProxyTracker.mGlobalProxy = new ProxyInfo(proxyProperties); + host = mProxyTracker.mGlobalProxy.getHost(); + port = mProxyTracker.mGlobalProxy.getPort(); + exclList = mProxyTracker.mGlobalProxy.getExclusionListAsString(); if (!Uri.EMPTY.equals(proxyProperties.getPacFileUrl())) { pacFileUrl = proxyProperties.getPacFileUrl().toString(); } } else { - mGlobalProxy = null; + mProxyTracker.mGlobalProxy = null; } ContentResolver res = mContext.getContentResolver(); final long token = Binder.clearCallingIdentity(); @@ -3379,8 +3383,8 @@ public class ConnectivityService extends IConnectivityManager.Stub Binder.restoreCallingIdentity(token); } - if (mGlobalProxy == null) { - proxyProperties = mDefaultProxy; + if (mProxyTracker.mGlobalProxy == null) { + proxyProperties = mProxyTracker.mDefaultProxy; } sendProxyBroadcast(proxyProperties); } @@ -3405,8 +3409,8 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } - synchronized (mProxyLock) { - mGlobalProxy = proxyProperties; + synchronized (mProxyTracker.mProxyLock) { + mProxyTracker.mGlobalProxy = proxyProperties; } } } @@ -3416,8 +3420,8 @@ public class ConnectivityService extends IConnectivityManager.Stub // so this API change wouldn't have a benefit. It also breaks the passing // of proxy info to all the JVMs. // enforceAccessPermission(); - synchronized (mProxyLock) { - return mGlobalProxy; + synchronized (mProxyTracker.mProxyLock) { + return mProxyTracker.mGlobalProxy; } } @@ -3426,9 +3430,11 @@ public class ConnectivityService extends IConnectivityManager.Stub && Uri.EMPTY.equals(proxy.getPacFileUrl())) { proxy = null; } - synchronized (mProxyLock) { - if (mDefaultProxy != null && mDefaultProxy.equals(proxy)) return; - if (mDefaultProxy == proxy) return; // catches repeated nulls + synchronized (mProxyTracker.mProxyLock) { + if (mProxyTracker.mDefaultProxy != null && mProxyTracker.mDefaultProxy.equals(proxy)) { + return; + } + if (mProxyTracker.mDefaultProxy == proxy) return; // catches repeated nulls if (proxy != null && !proxy.isValid()) { if (DBG) log("Invalid proxy properties, ignoring: " + proxy.toString()); return; @@ -3439,17 +3445,17 @@ public class ConnectivityService extends IConnectivityManager.Stub // global (to get the correct local port), and send a broadcast. // TODO: Switch PacManager to have its own message to send back rather than // reusing EVENT_HAS_CHANGED_PROXY and this call to handleApplyDefaultProxy. - if ((mGlobalProxy != null) && (proxy != null) + if ((mProxyTracker.mGlobalProxy != null) && (proxy != null) && (!Uri.EMPTY.equals(proxy.getPacFileUrl())) - && proxy.getPacFileUrl().equals(mGlobalProxy.getPacFileUrl())) { - mGlobalProxy = proxy; - sendProxyBroadcast(mGlobalProxy); + && proxy.getPacFileUrl().equals(mProxyTracker.mGlobalProxy.getPacFileUrl())) { + mProxyTracker.mGlobalProxy = proxy; + sendProxyBroadcast(mProxyTracker.mGlobalProxy); return; } - mDefaultProxy = proxy; + mProxyTracker.mDefaultProxy = proxy; - if (mGlobalProxy != null) return; - if (!mDefaultProxyDisabled) { + if (mProxyTracker.mGlobalProxy != null) return; + if (!mProxyTracker.mDefaultProxyDisabled) { sendProxyBroadcast(proxy); } } @@ -5521,10 +5527,11 @@ public class ConnectivityService extends IConnectivityManager.Stub if (networkAgent.isVPN()) { // Temporarily disable the default proxy (not global). - synchronized (mProxyLock) { - if (!mDefaultProxyDisabled) { - mDefaultProxyDisabled = true; - if (mGlobalProxy == null && mDefaultProxy != null) { + synchronized (mProxyTracker.mProxyLock) { + if (!mProxyTracker.mDefaultProxyDisabled) { + mProxyTracker.mDefaultProxyDisabled = true; + if (mProxyTracker.mGlobalProxy == null + && mProxyTracker.mDefaultProxy != null) { sendProxyBroadcast(null); } } @@ -5550,11 +5557,12 @@ public class ConnectivityService extends IConnectivityManager.Stub } else if (state == NetworkInfo.State.DISCONNECTED) { networkAgent.asyncChannel.disconnect(); if (networkAgent.isVPN()) { - synchronized (mProxyLock) { - if (mDefaultProxyDisabled) { - mDefaultProxyDisabled = false; - if (mGlobalProxy == null && mDefaultProxy != null) { - sendProxyBroadcast(mDefaultProxy); + synchronized (mProxyTracker.mProxyLock) { + if (mProxyTracker.mDefaultProxyDisabled) { + mProxyTracker.mDefaultProxyDisabled = false; + if (mProxyTracker.mGlobalProxy == null + && mProxyTracker.mDefaultProxy != null) { + sendProxyBroadcast(mProxyTracker.mDefaultProxy); } } } diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java new file mode 100644 index 0000000000..1b2dd5e721 --- /dev/null +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -0,0 +1,43 @@ +/** + * Copyright (c) 2018, The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.connectivity; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.net.ProxyInfo; + +import com.android.internal.annotations.GuardedBy; + +/** + * A class to handle proxy for ConnectivityService. + * + * @hide + */ +public class ProxyTracker { + // TODO : make this private and import as much managing logic from ConnectivityService as + // possible + @NonNull + public final Object mProxyLock = new Object(); + @Nullable + @GuardedBy("mProxyLock") + public ProxyInfo mGlobalProxy = null; + @Nullable + @GuardedBy("mProxyLock") + public volatile ProxyInfo mDefaultProxy = null; + @GuardedBy("mProxyLock") + public boolean mDefaultProxyDisabled = false; +} From 7d97afc73fcad8c805bd0dcb510b7d81c55a0824 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 17:41:29 +0900 Subject: [PATCH 5/9] [PT02] Move static methods to ProxyTracker No logic changes. Only changes are adding nullability annotations, final modifiers, and adding an s in a comment. Test: runtests Change-Id: If4986a25bb36819de8ff459c4c0439c56d4e5a50 --- .../android/server/ConnectivityService.java | 30 +---------------- .../server/connectivity/ProxyTracker.java | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 05b929f158..69887201fd 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3309,34 +3309,6 @@ 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(); @@ -3470,7 +3442,7 @@ public class ConnectivityService extends IConnectivityManager.Stub ProxyInfo newProxyInfo = newLp == null ? null : newLp.getHttpProxy(); ProxyInfo oldProxyInfo = oldLp == null ? null : oldLp.getHttpProxy(); - if (!proxyInfoEqual(newProxyInfo, oldProxyInfo)) { + if (!ProxyTracker.proxyInfoEqual(newProxyInfo, oldProxyInfo)) { sendProxyBroadcast(getDefaultProxy()); } } diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java index 1b2dd5e721..d1c2c26cdc 100644 --- a/services/core/java/com/android/server/connectivity/ProxyTracker.java +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -19,9 +19,13 @@ package com.android.server.connectivity; import android.annotation.NonNull; import android.annotation.Nullable; import android.net.ProxyInfo; +import android.net.Uri; +import android.text.TextUtils; import com.android.internal.annotations.GuardedBy; +import java.util.Objects; + /** * A class to handle proxy for ConnectivityService. * @@ -40,4 +44,33 @@ public class ProxyTracker { public volatile ProxyInfo mDefaultProxy = null; @GuardedBy("mProxyLock") public boolean mDefaultProxyDisabled = false; + + // 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). + @Nullable + private static ProxyInfo canonicalizeProxyInfo(@Nullable final ProxyInfo proxy) { + if (proxy != null && TextUtils.isEmpty(proxy.getHost()) + && (proxy.getPacFileUrl() == null || Uri.EMPTY.equals(proxy.getPacFileUrl()))) { + return null; + } + return proxy; + } + + // ProxyInfo equality functions 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. + public static boolean proxyInfoEqual(@Nullable final ProxyInfo a, @Nullable final ProxyInfo b) { + final ProxyInfo pa = canonicalizeProxyInfo(a); + final ProxyInfo pb = 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(pa, pb) && (pa == null || Objects.equals(pa.getHost(), pb.getHost())); + } } From 777e2e59e6a2ae3bfe09ec89232fae12904d4cea Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 18:02:37 +0900 Subject: [PATCH 6/9] [PT03] Move some more code into ProxyTracker. Add finals and annotations. Remove comments that have lost their context (they were in the context of disabling a permission check that had been added, but constituted an API change that would not serve any real purpose). Test: runtest Change-Id: I1f879b2c105d2127072b88233d72097a0d78fe14 --- .../android/server/ConnectivityService.java | 30 ++++--------------- .../server/connectivity/ProxyTracker.java | 18 +++++++++++ 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 69887201fd..e8e8a39dee 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3278,24 +3278,10 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.networkMonitor.forceReevaluation(uid); } - private ProxyInfo getDefaultProxy() { - // this information is already available as a world read/writable jvm property - // so this API change wouldn't have a benefit. It also breaks the passing - // of proxy info to all the JVMs. - // enforceAccessPermission(); - synchronized (mProxyTracker.mProxyLock) { - ProxyInfo ret = mProxyTracker.mGlobalProxy; - if ((ret == null) && !mProxyTracker.mDefaultProxyDisabled) { - ret = mProxyTracker.mDefaultProxy; - } - return ret; - } - } - @Override public ProxyInfo getProxyForNetwork(Network network) { - if (network == null) return getDefaultProxy(); - final ProxyInfo globalProxy = getGlobalProxy(); + if (network == null) return mProxyTracker.getDefaultProxy(); + final ProxyInfo globalProxy = mProxyTracker.getGlobalProxy(); if (globalProxy != null) return globalProxy; if (!NetworkUtils.queryUserAccess(Binder.getCallingUid(), network.netId)) return null; // Don't call getLinkProperties() as it requires ACCESS_NETWORK_STATE permission, which @@ -3387,14 +3373,10 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + @Override + @Nullable public ProxyInfo getGlobalProxy() { - // this information is already available as a world read/writable jvm property - // so this API change wouldn't have a benefit. It also breaks the passing - // of proxy info to all the JVMs. - // enforceAccessPermission(); - synchronized (mProxyTracker.mProxyLock) { - return mProxyTracker.mGlobalProxy; - } + return mProxyTracker.getGlobalProxy(); } private void handleApplyDefaultProxy(ProxyInfo proxy) { @@ -3443,7 +3425,7 @@ public class ConnectivityService extends IConnectivityManager.Stub ProxyInfo oldProxyInfo = oldLp == null ? null : oldLp.getHttpProxy(); if (!ProxyTracker.proxyInfoEqual(newProxyInfo, oldProxyInfo)) { - sendProxyBroadcast(getDefaultProxy()); + sendProxyBroadcast(mProxyTracker.getDefaultProxy()); } } diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java index d1c2c26cdc..5567acafa2 100644 --- a/services/core/java/com/android/server/connectivity/ProxyTracker.java +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -73,4 +73,22 @@ public class ProxyTracker { // hosts even when PAC URLs are present to account for the legacy PAC resolver. return Objects.equals(pa, pb) && (pa == null || Objects.equals(pa.getHost(), pb.getHost())); } + + @Nullable + public ProxyInfo getDefaultProxy() { + // This information is already available as a world read/writable jvm property. + synchronized (mProxyLock) { + final ProxyInfo ret = mGlobalProxy; + if ((ret == null) && !mDefaultProxyDisabled) return mDefaultProxy; + return ret; + } + } + + @Nullable + public ProxyInfo getGlobalProxy() { + // This information is already available as a world read/writable jvm property. + synchronized (mProxyLock) { + return mGlobalProxy; + } + } } From faf4a2083db0432d0bcfc97379e338c641caab2b Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 19:20:08 +0900 Subject: [PATCH 7/9] [PT04] Move PacManager into ProxyTracker. Test: runtest Change-Id: I45d22857459fe1746d484ac04f8d5cd81fc61835 --- .../com/android/server/ConnectivityService.java | 9 +++------ .../android/server/connectivity/ProxyTracker.java | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index e8e8a39dee..76a40787f8 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -429,9 +429,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // A helper object to track the current default HTTP proxy. ConnectivityService needs to tell // the world when it changes. - private final ProxyTracker mProxyTracker = new ProxyTracker(); - - private PacManager mPacManager = null; + private final ProxyTracker mProxyTracker; final private SettingsObserver mSettingsObserver; @@ -730,6 +728,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mPolicyManagerInternal = checkNotNull( LocalServices.getService(NetworkPolicyManagerInternal.class), "missing NetworkPolicyManagerInternal"); + mProxyTracker = new ProxyTracker(context, mHandler, EVENT_PROXY_HAS_CHANGED); mKeyStore = KeyStore.getInstance(); mTelephonyManager = (TelephonyManager) mContext.getSystemService(Context.TELEPHONY_SERVICE); @@ -840,8 +839,6 @@ public class ConnectivityService extends IConnectivityManager.Stub final DataConnectionStats dataConnectionStats = new DataConnectionStats(mContext); dataConnectionStats.startMonitoring(); - mPacManager = new PacManager(mContext, mHandler, EVENT_PROXY_HAS_CHANGED); - mUserManager = (UserManager) context.getSystemService(Context.USER_SERVICE); mKeepaliveTracker = new KeepaliveTracker(mHandler); @@ -3454,7 +3451,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void sendProxyBroadcast(ProxyInfo proxy) { if (proxy == null) proxy = new ProxyInfo("", 0, ""); - if (mPacManager.setCurrentProxyScriptUrl(proxy)) return; + if (mProxyTracker.setCurrentProxyScriptUrl(proxy)) return; if (DBG) log("sending Proxy Broadcast for " + proxy); Intent intent = new Intent(Proxy.PROXY_CHANGE_ACTION); intent.addFlags(Intent.FLAG_RECEIVER_REPLACE_PENDING | diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java index 5567acafa2..c41779a6df 100644 --- a/services/core/java/com/android/server/connectivity/ProxyTracker.java +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -18,8 +18,10 @@ package com.android.server.connectivity; import android.annotation.NonNull; import android.annotation.Nullable; +import android.content.Context; import android.net.ProxyInfo; import android.net.Uri; +import android.os.Handler; import android.text.TextUtils; import com.android.internal.annotations.GuardedBy; @@ -45,6 +47,14 @@ public class ProxyTracker { @GuardedBy("mProxyLock") public boolean mDefaultProxyDisabled = false; + @NonNull + private final PacManager mPacManager; + + public ProxyTracker(@NonNull final Context context, + @NonNull final Handler connectivityServiceInternalHandler, final int pacChangedEvent) { + mPacManager = new PacManager(context, connectivityServiceInternalHandler, pacChangedEvent); + } + // 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). @@ -91,4 +101,8 @@ public class ProxyTracker { return mGlobalProxy; } } + + public boolean setCurrentProxyScriptUrl(@NonNull final ProxyInfo proxy) { + return mPacManager.setCurrentProxyScriptUrl(proxy); + } } From 5c0ca509d5e69de3e7dad6e90a69b7b45d643968 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 19:30:29 +0900 Subject: [PATCH 8/9] [PT05] Move sendProxyBroadcast into ProxyTracker. Test: runtest Change-Id: I2c149d29d4b75d3978021b940b6bc58f677b8d17 --- .../android/server/ConnectivityService.java | 28 ++++-------------- .../server/connectivity/ProxyTracker.java | 29 +++++++++++++++++++ 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 76a40787f8..3d923a9300 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3341,7 +3341,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (mProxyTracker.mGlobalProxy == null) { proxyProperties = mProxyTracker.mDefaultProxy; } - sendProxyBroadcast(proxyProperties); + mProxyTracker.sendProxyBroadcast(proxyProperties); } } @@ -3400,14 +3400,14 @@ public class ConnectivityService extends IConnectivityManager.Stub && (!Uri.EMPTY.equals(proxy.getPacFileUrl())) && proxy.getPacFileUrl().equals(mProxyTracker.mGlobalProxy.getPacFileUrl())) { mProxyTracker.mGlobalProxy = proxy; - sendProxyBroadcast(mProxyTracker.mGlobalProxy); + mProxyTracker.sendProxyBroadcast(mProxyTracker.mGlobalProxy); return; } mProxyTracker.mDefaultProxy = proxy; if (mProxyTracker.mGlobalProxy != null) return; if (!mProxyTracker.mDefaultProxyDisabled) { - sendProxyBroadcast(proxy); + mProxyTracker.sendProxyBroadcast(proxy); } } } @@ -3422,7 +3422,7 @@ public class ConnectivityService extends IConnectivityManager.Stub ProxyInfo oldProxyInfo = oldLp == null ? null : oldLp.getHttpProxy(); if (!ProxyTracker.proxyInfoEqual(newProxyInfo, oldProxyInfo)) { - sendProxyBroadcast(mProxyTracker.getDefaultProxy()); + mProxyTracker.sendProxyBroadcast(mProxyTracker.getDefaultProxy()); } } @@ -3449,22 +3449,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private void sendProxyBroadcast(ProxyInfo proxy) { - if (proxy == null) proxy = new ProxyInfo("", 0, ""); - if (mProxyTracker.setCurrentProxyScriptUrl(proxy)) return; - if (DBG) log("sending Proxy Broadcast for " + proxy); - Intent intent = new Intent(Proxy.PROXY_CHANGE_ACTION); - intent.addFlags(Intent.FLAG_RECEIVER_REPLACE_PENDING | - Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT); - intent.putExtra(Proxy.EXTRA_PROXY_INFO, proxy); - final long ident = Binder.clearCallingIdentity(); - try { - mContext.sendStickyBroadcastAsUser(intent, UserHandle.ALL); - } finally { - Binder.restoreCallingIdentity(ident); - } - } - private static class SettingsObserver extends ContentObserver { final private HashMap mUriEventMap; final private Context mContext; @@ -5483,7 +5467,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mProxyTracker.mDefaultProxyDisabled = true; if (mProxyTracker.mGlobalProxy == null && mProxyTracker.mDefaultProxy != null) { - sendProxyBroadcast(null); + mProxyTracker.sendProxyBroadcast(null); } } } @@ -5513,7 +5497,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mProxyTracker.mDefaultProxyDisabled = false; if (mProxyTracker.mGlobalProxy == null && mProxyTracker.mDefaultProxy != null) { - sendProxyBroadcast(mProxyTracker.mDefaultProxy); + mProxyTracker.sendProxyBroadcast(mProxyTracker.mDefaultProxy); } } } diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java index c41779a6df..89cacdcf0b 100644 --- a/services/core/java/com/android/server/connectivity/ProxyTracker.java +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -19,10 +19,15 @@ package com.android.server.connectivity; import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; +import android.content.Intent; +import android.net.Proxy; import android.net.ProxyInfo; import android.net.Uri; +import android.os.Binder; import android.os.Handler; +import android.os.UserHandle; import android.text.TextUtils; +import android.util.Slog; import com.android.internal.annotations.GuardedBy; @@ -34,6 +39,12 @@ import java.util.Objects; * @hide */ public class ProxyTracker { + private static final String TAG = ProxyTracker.class.getSimpleName(); + private static final boolean DBG = true; + + @NonNull + private final Context mContext; + // TODO : make this private and import as much managing logic from ConnectivityService as // possible @NonNull @@ -52,6 +63,7 @@ public class ProxyTracker { public ProxyTracker(@NonNull final Context context, @NonNull final Handler connectivityServiceInternalHandler, final int pacChangedEvent) { + mContext = context; mPacManager = new PacManager(context, connectivityServiceInternalHandler, pacChangedEvent); } @@ -105,4 +117,21 @@ public class ProxyTracker { public boolean setCurrentProxyScriptUrl(@NonNull final ProxyInfo proxy) { return mPacManager.setCurrentProxyScriptUrl(proxy); } + + // TODO : make the argument NonNull final + public void sendProxyBroadcast(@Nullable ProxyInfo proxy) { + if (proxy == null) proxy = new ProxyInfo("", 0, ""); + if (setCurrentProxyScriptUrl(proxy)) return; + if (DBG) Slog.d(TAG, "sending Proxy Broadcast for " + proxy); + Intent intent = new Intent(Proxy.PROXY_CHANGE_ACTION); + intent.addFlags(Intent.FLAG_RECEIVER_REPLACE_PENDING | + Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT); + intent.putExtra(Proxy.EXTRA_PROXY_INFO, proxy); + final long ident = Binder.clearCallingIdentity(); + try { + mContext.sendStickyBroadcastAsUser(intent, UserHandle.ALL); + } finally { + Binder.restoreCallingIdentity(ident); + } + } } From 8cbd2dd7e4fb0268db7cdf7666d0f62972531d3e Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 18:37:59 +0900 Subject: [PATCH 9/9] [PT06] Move setGlobalProxy into ProxyTracker Test: runtest Change-Id: I6abd2221882db368a411b7174c66d8bd3b6b5110 --- .../android/server/ConnectivityService.java | 53 ++----------------- .../server/connectivity/ProxyTracker.java | 47 ++++++++++++++++ 2 files changed, 50 insertions(+), 50 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 3d923a9300..dadd3274ee 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3292,57 +3292,10 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - public void setGlobalProxy(ProxyInfo proxyProperties) { + @Override + public void setGlobalProxy(final ProxyInfo proxyProperties) { enforceConnectivityInternalPermission(); - - synchronized (mProxyTracker.mProxyLock) { - if (proxyProperties == mProxyTracker.mGlobalProxy) return; - if (proxyProperties != null && proxyProperties.equals(mProxyTracker.mGlobalProxy)) { - return; - } - if (mProxyTracker.mGlobalProxy != null - && mProxyTracker.mGlobalProxy.equals(proxyProperties)) { - return; - } - - String host = ""; - int port = 0; - String exclList = ""; - String pacFileUrl = ""; - if (proxyProperties != null && (!TextUtils.isEmpty(proxyProperties.getHost()) || - !Uri.EMPTY.equals(proxyProperties.getPacFileUrl()))) { - if (!proxyProperties.isValid()) { - if (DBG) - log("Invalid proxy properties, ignoring: " + proxyProperties.toString()); - return; - } - mProxyTracker.mGlobalProxy = new ProxyInfo(proxyProperties); - host = mProxyTracker.mGlobalProxy.getHost(); - port = mProxyTracker.mGlobalProxy.getPort(); - exclList = mProxyTracker.mGlobalProxy.getExclusionListAsString(); - if (!Uri.EMPTY.equals(proxyProperties.getPacFileUrl())) { - pacFileUrl = proxyProperties.getPacFileUrl().toString(); - } - } else { - mProxyTracker.mGlobalProxy = null; - } - ContentResolver res = mContext.getContentResolver(); - final long token = Binder.clearCallingIdentity(); - try { - Settings.Global.putString(res, Settings.Global.GLOBAL_HTTP_PROXY_HOST, host); - Settings.Global.putInt(res, Settings.Global.GLOBAL_HTTP_PROXY_PORT, port); - Settings.Global.putString(res, Settings.Global.GLOBAL_HTTP_PROXY_EXCLUSION_LIST, - exclList); - Settings.Global.putString(res, Settings.Global.GLOBAL_HTTP_PROXY_PAC, pacFileUrl); - } finally { - Binder.restoreCallingIdentity(token); - } - - if (mProxyTracker.mGlobalProxy == null) { - proxyProperties = mProxyTracker.mDefaultProxy; - } - mProxyTracker.sendProxyBroadcast(proxyProperties); - } + mProxyTracker.setGlobalProxy(proxyProperties); } private void loadGlobalProxy() { diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java index 89cacdcf0b..daaedd8131 100644 --- a/services/core/java/com/android/server/connectivity/ProxyTracker.java +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -18,6 +18,7 @@ package com.android.server.connectivity; import android.annotation.NonNull; import android.annotation.Nullable; +import android.content.ContentResolver; import android.content.Context; import android.content.Intent; import android.net.Proxy; @@ -26,6 +27,7 @@ import android.net.Uri; import android.os.Binder; import android.os.Handler; import android.os.UserHandle; +import android.provider.Settings; import android.text.TextUtils; import android.util.Slog; @@ -134,4 +136,49 @@ public class ProxyTracker { Binder.restoreCallingIdentity(ident); } } + + public void setGlobalProxy(@Nullable ProxyInfo proxyProperties) { + synchronized (mProxyLock) { + if (proxyProperties == mGlobalProxy) return; + if (proxyProperties != null && proxyProperties.equals(mGlobalProxy)) return; + if (mGlobalProxy != null && mGlobalProxy.equals(proxyProperties)) return; + + String host = ""; + int port = 0; + String exclList = ""; + String pacFileUrl = ""; + if (proxyProperties != null && (!TextUtils.isEmpty(proxyProperties.getHost()) || + !Uri.EMPTY.equals(proxyProperties.getPacFileUrl()))) { + if (!proxyProperties.isValid()) { + if (DBG) Slog.d(TAG, "Invalid proxy properties, ignoring: " + proxyProperties); + return; + } + mGlobalProxy = new ProxyInfo(proxyProperties); + host = mGlobalProxy.getHost(); + port = mGlobalProxy.getPort(); + exclList = mGlobalProxy.getExclusionListAsString(); + if (!Uri.EMPTY.equals(proxyProperties.getPacFileUrl())) { + pacFileUrl = proxyProperties.getPacFileUrl().toString(); + } + } else { + mGlobalProxy = null; + } + final ContentResolver res = mContext.getContentResolver(); + final long token = Binder.clearCallingIdentity(); + try { + Settings.Global.putString(res, Settings.Global.GLOBAL_HTTP_PROXY_HOST, host); + Settings.Global.putInt(res, Settings.Global.GLOBAL_HTTP_PROXY_PORT, port); + Settings.Global.putString(res, Settings.Global.GLOBAL_HTTP_PROXY_EXCLUSION_LIST, + exclList); + Settings.Global.putString(res, Settings.Global.GLOBAL_HTTP_PROXY_PAC, pacFileUrl); + } finally { + Binder.restoreCallingIdentity(token); + } + + if (mGlobalProxy == null) { + proxyProperties = mDefaultProxy; + } + sendProxyBroadcast(proxyProperties); + } + } }