Fix a bug where the PAC proxy port is not set correctly.
Test: new test for this behavior in the preliminary change
Test: FrameworksNetTests NetworkStackTests
Fixes: 138810051
Fixes: 140610528
Change-Id: I95a979d232fb60ece2e33e972bf5d66d20357a1f
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index c95295c..7cf8672 100755
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -34,6 +34,7 @@
import static android.net.ConnectivityManager.BLOCKED_METERED_REASON_MASK;
import static android.net.ConnectivityManager.BLOCKED_REASON_LOCKDOWN_VPN;
import static android.net.ConnectivityManager.BLOCKED_REASON_NONE;
+import static android.net.ConnectivityManager.CALLBACK_IP_CHANGED;
import static android.net.ConnectivityManager.CONNECTIVITY_ACTION;
import static android.net.ConnectivityManager.FIREWALL_RULE_ALLOW;
import static android.net.ConnectivityManager.FIREWALL_RULE_DEFAULT;
@@ -548,7 +549,7 @@
/**
* PAC manager has received new port.
*/
- private static final int EVENT_PROXY_HAS_CHANGED = 16;
+ private static final int EVENT_PAC_PROXY_HAS_CHANGED = 16;
/**
* used internally when registering NetworkProviders
@@ -1322,7 +1323,7 @@
*/
public ProxyTracker makeProxyTracker(@NonNull Context context,
@NonNull Handler connServiceHandler) {
- return new ProxyTracker(context, connServiceHandler, EVENT_PROXY_HAS_CHANGED);
+ return new ProxyTracker(context, connServiceHandler, EVENT_PAC_PROXY_HAS_CHANGED);
}
/**
@@ -1868,7 +1869,7 @@
@VisibleForTesting
void simulateUpdateProxyInfo(@Nullable final Network network,
@NonNull final ProxyInfo proxyInfo) {
- Message.obtain(mHandler, EVENT_PROXY_HAS_CHANGED,
+ Message.obtain(mHandler, EVENT_PAC_PROXY_HAS_CHANGED,
new Pair<>(network, proxyInfo)).sendToTarget();
}
@@ -4654,6 +4655,21 @@
rematchAllNetworksAndRequests();
mLingerMonitor.noteDisconnect(nai);
+ if (null == getDefaultNetwork() && nai.linkProperties.getHttpProxy() != null) {
+ // The obvious place to do this would be in makeDefault(), however makeDefault() is
+ // not called by the rematch in this case. This is because the code above unset
+ // this network from the default request's satisfier, and that is what the rematch
+ // is using as its source data to know what the old satisfier was. So as far as the
+ // rematch above is concerned, the old default network was null.
+ // Therefore if there is no new default, the default network was null and is still
+ // null, thus there was no change so makeDefault() is not called. So if the old
+ // network had a proxy and there is no new default, the proxy tracker should be told
+ // that there is no longer a default proxy.
+ // Strictly speaking this is not essential because having a proxy setting when
+ // there is no network is harmless, but it's still counter-intuitive so reset to null.
+ mProxyTracker.setDefaultProxy(null);
+ }
+
// Immediate teardown.
if (nai.teardownDelayMs == 0) {
destroyNetwork(nai);
@@ -5676,9 +5692,9 @@
mProxyTracker.loadDeprecatedGlobalHttpProxy();
break;
}
- case EVENT_PROXY_HAS_CHANGED: {
+ case EVENT_PAC_PROXY_HAS_CHANGED: {
final Pair<Network, ProxyInfo> arg = (Pair<Network, ProxyInfo>) msg.obj;
- handleApplyDefaultProxy(arg.second);
+ handlePacProxyServiceStarted(arg.first, arg.second);
break;
}
case EVENT_REGISTER_NETWORK_PROVIDER: {
@@ -6133,12 +6149,19 @@
return mProxyTracker.getGlobalProxy();
}
- private void handleApplyDefaultProxy(@Nullable ProxyInfo proxy) {
- if (proxy != null && TextUtils.isEmpty(proxy.getHost())
- && Uri.EMPTY.equals(proxy.getPacFileUrl())) {
- proxy = null;
- }
+ private void handlePacProxyServiceStarted(@Nullable Network net, @Nullable ProxyInfo proxy) {
mProxyTracker.setDefaultProxy(proxy);
+ final NetworkAgentInfo nai = getDefaultNetwork();
+ // TODO : this method should check that net == nai.network, unfortunately at this point
+ // 'net' is always null in practice (see PacProxyService#sendPacBroadcast). PAC proxy
+ // is only ever installed on the default network so in practice this is okay.
+ if (null == nai) return;
+ // PAC proxies only work on the default network. Therefore, only the default network
+ // should have its link properties fixed up for PAC proxies.
+ mProxyTracker.updateDefaultNetworkProxyPortForPAC(nai.linkProperties, nai.network);
+ if (nai.everConnected()) {
+ notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_IP_CHANGED);
+ }
}
// If the proxy has changed from oldLp to newLp, resend proxy broadcast. This method gets called
@@ -7964,6 +7987,7 @@
// updateMtu(lp, null);
// }
if (isDefaultNetwork(networkAgent)) {
+ mProxyTracker.updateDefaultNetworkProxyPortForPAC(newLp, null);
updateTcpBufferSizes(newLp.getTcpBufferSizes());
}
@@ -7976,7 +8000,7 @@
mDnsManager.updatePrivateDnsStatus(netId, newLp);
if (isDefaultNetwork(networkAgent)) {
- handleApplyDefaultProxy(newLp.getHttpProxy());
+ mProxyTracker.setDefaultProxy(newLp.getHttpProxy());
} else if (networkAgent.everConnected()) {
updateProxy(newLp, oldLp);
}
@@ -9023,6 +9047,17 @@
}
}
+ private void resetHttpProxyForNonDefaultNetwork(NetworkAgentInfo oldDefaultNetwork) {
+ if (null == oldDefaultNetwork) return;
+ // The network stopped being the default. If it was using a PAC proxy, then the
+ // proxy needs to be reset, otherwise HTTP requests on this network may be sent
+ // to the local proxy server, which would forward them over the newly default network.
+ final ProxyInfo proxyInfo = oldDefaultNetwork.linkProperties.getHttpProxy();
+ if (null == proxyInfo || !proxyInfo.isPacProxy()) return;
+ oldDefaultNetwork.linkProperties.setHttpProxy(new ProxyInfo(proxyInfo.getPacFileUrl()));
+ notifyNetworkCallbacks(oldDefaultNetwork, CALLBACK_IP_CHANGED);
+ }
+
private void makeDefault(@NonNull final NetworkRequestInfo nri,
@Nullable final NetworkAgentInfo oldDefaultNetwork,
@Nullable final NetworkAgentInfo newDefaultNetwork) {
@@ -9048,8 +9083,9 @@
mLingerMonitor.noteLingerDefaultNetwork(oldDefaultNetwork, newDefaultNetwork);
}
mNetworkActivityTracker.updateDataActivityTracking(newDefaultNetwork, oldDefaultNetwork);
- handleApplyDefaultProxy(null != newDefaultNetwork
+ mProxyTracker.setDefaultProxy(null != newDefaultNetwork
? newDefaultNetwork.linkProperties.getHttpProxy() : null);
+ resetHttpProxyForNonDefaultNetwork(oldDefaultNetwork);
updateTcpBufferSizes(null != newDefaultNetwork
? newDefaultNetwork.linkProperties.getTcpBufferSizes() : null);
notifyIfacesChangedForNetworkStats();
diff --git a/service/src/com/android/server/connectivity/ProxyTracker.java b/service/src/com/android/server/connectivity/ProxyTracker.java
index b3cbb2a..bda4b8f 100644
--- a/service/src/com/android/server/connectivity/ProxyTracker.java
+++ b/service/src/com/android/server/connectivity/ProxyTracker.java
@@ -27,6 +27,7 @@
import android.content.ContentResolver;
import android.content.Context;
import android.content.Intent;
+import android.net.LinkProperties;
import android.net.Network;
import android.net.PacProxyManager;
import android.net.Proxy;
@@ -95,6 +96,7 @@
}
public void onPacProxyInstalled(@Nullable Network network, @NonNull ProxyInfo proxy) {
+ Log.i(TAG, "PAC proxy installed on network " + network + " : " + proxy);
mConnectivityServiceHandler
.sendMessage(mConnectivityServiceHandler
.obtainMessage(mEvent, new Pair<>(network, proxy)));
@@ -328,6 +330,12 @@
* @param proxyInfo the proxy spec, or null for no proxy.
*/
public void setDefaultProxy(@Nullable ProxyInfo proxyInfo) {
+ // The code has been accepting empty proxy objects forever, so for backward
+ // compatibility it should continue doing so.
+ if (proxyInfo != null && TextUtils.isEmpty(proxyInfo.getHost())
+ && Uri.EMPTY.equals(proxyInfo.getPacFileUrl())) {
+ proxyInfo = null;
+ }
synchronized (mProxyLock) {
if (Objects.equals(mDefaultProxy, proxyInfo)) return;
if (proxyInfo != null && !proxyInfo.isValid()) {
@@ -355,4 +363,51 @@
}
}
}
+
+ private boolean isPacProxy(@Nullable final ProxyInfo info) {
+ return null != info && info.isPacProxy();
+ }
+
+ /**
+ * Adjust the proxy in the link properties if necessary.
+ *
+ * It is necessary when the proxy in the passed property is for PAC, and the default proxy
+ * is also for PAC. This is because the original LinkProperties from the network agent don't
+ * include the port for the local proxy as it's not known at creation time, but this class
+ * knows it after the proxy service is started.
+ *
+ * This is safe because there can only ever be one proxy service running on the device, so
+ * if the ProxyInfo in the LinkProperties is for PAC, then the port is necessarily the one
+ * ProxyTracker knows about.
+ *
+ * @param lp the LinkProperties to fix up.
+ * @param network the network of the local proxy server.
+ */
+ // TODO: Leave network unused to support local proxy server per network in the future.
+ public void updateDefaultNetworkProxyPortForPAC(@NonNull final LinkProperties lp,
+ @Nullable Network network) {
+ final ProxyInfo defaultProxy = getDefaultProxy();
+ if (isPacProxy(lp.getHttpProxy()) && isPacProxy(defaultProxy)) {
+ synchronized (mProxyLock) {
+ // At this time, this method can only be called for the default network's LP.
+ // Therefore the PAC file URL in the LP must match the one in the default proxy,
+ // and we just update the port.
+ // Note that the global proxy, if any, is set out of band by the DPM and becomes
+ // the default proxy (it overrides it, see {@link getDefaultProxy}). The PAC URL
+ // in the global proxy might not be the one in the LP of the default
+ // network, so discount this case.
+ if (null == mGlobalProxy && !lp.getHttpProxy().getPacFileUrl()
+ .equals(defaultProxy.getPacFileUrl())) {
+ throw new IllegalStateException("Unexpected discrepancy between proxy in LP of "
+ + "default network and default proxy. The former has a PAC URL of "
+ + lp.getHttpProxy().getPacFileUrl() + " while the latter has "
+ + defaultProxy.getPacFileUrl());
+ }
+ }
+ // If this network has a PAC proxy and proxy tracker already knows about
+ // it, now is the right time to patch it in. If proxy tracker does not know
+ // about it yet, then it will be patched in when it learns about it.
+ lp.setHttpProxy(defaultProxy);
+ }
+ }
}