diff --git a/framework-t/src/android/net/nsd/NsdServiceInfo.java b/framework-t/src/android/net/nsd/NsdServiceInfo.java index 2621594e98..200c808565 100644 --- a/framework-t/src/android/net/nsd/NsdServiceInfo.java +++ b/framework-t/src/android/net/nsd/NsdServiceInfo.java @@ -53,6 +53,8 @@ public final class NsdServiceInfo implements Parcelable { @Nullable private Network mNetwork; + private int mInterfaceIndex; + public NsdServiceInfo() { } @@ -312,8 +314,11 @@ public final class NsdServiceInfo implements Parcelable { /** * Get the network where the service can be found. * - * This is never null if this {@link NsdServiceInfo} was obtained from - * {@link NsdManager#discoverServices} or {@link NsdManager#resolveService}. + * This is set if this {@link NsdServiceInfo} was obtained from + * {@link NsdManager#discoverServices} or {@link NsdManager#resolveService}, unless the service + * was found on a network interface that does not have a {@link Network} (such as a tethering + * downstream, where services are advertised from devices connected to this device via + * tethering). */ @Nullable public Network getNetwork() { @@ -329,6 +334,26 @@ public final class NsdServiceInfo implements Parcelable { mNetwork = network; } + /** + * Get the index of the network interface where the service was found. + * + * This is only set when the service was found on an interface that does not have a usable + * Network, in which case {@link #getNetwork()} returns null. + * @return The interface index as per {@link java.net.NetworkInterface#getIndex}, or 0 if unset. + * @hide + */ + public int getInterfaceIndex() { + return mInterfaceIndex; + } + + /** + * Set the index of the network interface where the service was found. + * @hide + */ + public void setInterfaceIndex(int interfaceIndex) { + mInterfaceIndex = interfaceIndex; + } + @Override public String toString() { StringBuilder sb = new StringBuilder(); @@ -375,6 +400,7 @@ public final class NsdServiceInfo implements Parcelable { } dest.writeParcelable(mNetwork, 0); + dest.writeInt(mInterfaceIndex); } /** Implement the Parcelable interface */ @@ -405,6 +431,7 @@ public final class NsdServiceInfo implements Parcelable { info.mTxtRecord.put(in.readString(), valueArray); } info.mNetwork = in.readParcelable(null, Network.class); + info.mInterfaceIndex = in.readInt(); return info; } diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java index 6def44f3c0..95e6114255 100644 --- a/service-t/src/com/android/server/NsdService.java +++ b/service-t/src/com/android/server/NsdService.java @@ -23,6 +23,7 @@ import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; import android.net.ConnectivityManager; +import android.net.INetd; import android.net.LinkProperties; import android.net.Network; import android.net.mdns.aidl.DiscoveryInfo; @@ -466,7 +467,7 @@ public class NsdService extends INsdManager.Stub { // interfaces that do not have an associated Network. break; } - servInfo.setNetwork(new Network(foundNetId)); + setServiceNetworkForCallback(servInfo, info.netId, info.interfaceIdx); clientInfo.onServiceFound(clientId, servInfo); break; } @@ -476,10 +477,11 @@ public class NsdService extends INsdManager.Stub { final String type = info.registrationType; final int lostNetId = info.netId; servInfo = new NsdServiceInfo(name, type); - // The network could be null if it was torn down when the service is lost - // TODO: avoid returning null in that case, possibly by remembering found - // services on the same interface index and their network at the time - servInfo.setNetwork(lostNetId == 0 ? null : new Network(lostNetId)); + // The network could be set to null (netId 0) if it was torn down when the + // service is lost + // TODO: avoid returning null in that case, possibly by remembering + // found services on the same interface index and their network at the time + setServiceNetworkForCallback(servInfo, lostNetId, info.interfaceIdx); clientInfo.onServiceLost(clientId, servInfo); break; } @@ -557,7 +559,6 @@ public class NsdService extends INsdManager.Stub { final GetAddressInfo info = (GetAddressInfo) obj; final String address = info.address; final int netId = info.netId; - final Network network = netId == NETID_UNSET ? null : new Network(netId); InetAddress serviceHost = null; try { serviceHost = InetAddress.getByName(address); @@ -568,9 +569,10 @@ public class NsdService extends INsdManager.Stub { // If the resolved service is on an interface without a network, consider it // as a failure: it would not be usable by apps as they would need // privileged permissions. - if (network != null && serviceHost != null) { + if (netId != NETID_UNSET && serviceHost != null) { clientInfo.mResolvedService.setHost(serviceHost); - clientInfo.mResolvedService.setNetwork(network); + setServiceNetworkForCallback(clientInfo.mResolvedService, + netId, info.interfaceIdx); clientInfo.onResolveServiceSucceeded( clientId, clientInfo.mResolvedService); } else { @@ -590,6 +592,26 @@ public class NsdService extends INsdManager.Stub { } } + private static void setServiceNetworkForCallback(NsdServiceInfo info, int netId, int ifaceIdx) { + switch (netId) { + case NETID_UNSET: + info.setNetwork(null); + break; + case INetd.LOCAL_NET_ID: + // Special case for LOCAL_NET_ID: Networks on netId 99 are not generally + // visible / usable for apps, so do not return it. Store the interface + // index instead, so at least if the client tries to resolve the service + // with that NsdServiceInfo, it will be done on the same interface. + // If they recreate the NsdServiceInfo themselves, resolution would be + // done on all interfaces as before T, which should also work. + info.setNetwork(null); + info.setInterfaceIndex(ifaceIdx); + break; + default: + info.setNetwork(new Network(netId)); + } + } + // The full service name is escaped from standard DNS rules on mdnsresponder, making it suitable // for passing to standard system DNS APIs such as res_query() . Thus, make the service name // unescape for getting right service address. See "Notes on DNS Name Escaping" on @@ -767,9 +789,8 @@ public class NsdService extends INsdManager.Stub { String type = service.getServiceType(); int port = service.getPort(); byte[] textRecord = service.getTxtRecord(); - final Network network = service.getNetwork(); - final int registerInterface = getNetworkInterfaceIndex(network); - if (network != null && registerInterface == IFACE_IDX_ANY) { + final int registerInterface = getNetworkInterfaceIndex(service); + if (service.getNetwork() != null && registerInterface == IFACE_IDX_ANY) { Log.e(TAG, "Interface to register service on not found"); return false; } @@ -781,10 +802,9 @@ public class NsdService extends INsdManager.Stub { } private boolean discoverServices(int discoveryId, NsdServiceInfo serviceInfo) { - final Network network = serviceInfo.getNetwork(); final String type = serviceInfo.getServiceType(); - final int discoverInterface = getNetworkInterfaceIndex(network); - if (network != null && discoverInterface == IFACE_IDX_ANY) { + final int discoverInterface = getNetworkInterfaceIndex(serviceInfo); + if (serviceInfo.getNetwork() != null && discoverInterface == IFACE_IDX_ANY) { Log.e(TAG, "Interface to discover service on not found"); return false; } @@ -798,9 +818,8 @@ public class NsdService extends INsdManager.Stub { private boolean resolveService(int resolveId, NsdServiceInfo service) { final String name = service.getServiceName(); final String type = service.getServiceType(); - final Network network = service.getNetwork(); - final int resolveInterface = getNetworkInterfaceIndex(network); - if (network != null && resolveInterface == IFACE_IDX_ANY) { + final int resolveInterface = getNetworkInterfaceIndex(service); + if (service.getNetwork() != null && resolveInterface == IFACE_IDX_ANY) { Log.e(TAG, "Interface to resolve service on not found"); return false; } @@ -816,8 +835,17 @@ public class NsdService extends INsdManager.Stub { * this is to support the legacy mdnsresponder implementation, which historically resolved * services on an unspecified network. */ - private int getNetworkInterfaceIndex(Network network) { - if (network == null) return IFACE_IDX_ANY; + private int getNetworkInterfaceIndex(NsdServiceInfo serviceInfo) { + final Network network = serviceInfo.getNetwork(); + if (network == null) { + // Fallback to getInterfaceIndex if present (typically if the NsdServiceInfo was + // provided by NsdService from discovery results, and the service was found on an + // interface that has no app-usable Network). + if (serviceInfo.getInterfaceIndex() != 0) { + return serviceInfo.getInterfaceIndex(); + } + return IFACE_IDX_ANY; + } final ConnectivityManager cm = mContext.getSystemService(ConnectivityManager.class); if (cm == null) { diff --git a/tests/common/Android.bp b/tests/common/Android.bp index 509e881a8b..58731e0c74 100644 --- a/tests/common/Android.bp +++ b/tests/common/Android.bp @@ -140,6 +140,30 @@ java_defaults { ], } +// defaults for tests that need to build against framework-connectivity's @hide APIs, but also +// using fully @hide classes that are jarjared (because they have no API member). Similar to +// framework-connectivity-test-defaults above but uses pre-jarjar class names. +// Only usable from targets that have visibility on framework-connectivity-pre-jarjar, and apply +// connectivity jarjar rules so that references to jarjared classes still match: this is limited to +// connectivity internal tests only. +java_defaults { + name: "framework-connectivity-internal-test-defaults", + sdk_version: "core_platform", // tests can use @CorePlatformApi's + libs: [ + // order matters: classes in framework-connectivity are resolved before framework, + // meaning @hide APIs in framework-connectivity are resolved before @SystemApi + // stubs in framework + "framework-connectivity-pre-jarjar", + "framework-connectivity-t-pre-jarjar", + "framework-tethering.impl", + "framework", + + // if sdk_version="" this gets automatically included, but here we need to add manually. + "framework-res", + ], + defaults_visibility: ["//packages/modules/Connectivity/tests:__subpackages__"], +} + // Defaults for tests that want to run in mainline-presubmit. // Not widely used because many of our tests have AndroidTest.xml files and // use the mainline-param config-descriptor metadata in AndroidTest.xml. diff --git a/tests/unit/Android.bp b/tests/unit/Android.bp index e6cf22aa73..3ea27f7a7a 100644 --- a/tests/unit/Android.bp +++ b/tests/unit/Android.bp @@ -87,7 +87,7 @@ java_defaults { name: "FrameworksNetTestsDefaults", min_sdk_version: "30", defaults: [ - "framework-connectivity-test-defaults", + "framework-connectivity-internal-test-defaults", ], srcs: [ "java/**/*.java", diff --git a/tests/unit/java/android/net/nsd/NsdServiceInfoTest.java b/tests/unit/java/android/net/nsd/NsdServiceInfoTest.java index 035437708e..64355edaf2 100644 --- a/tests/unit/java/android/net/nsd/NsdServiceInfoTest.java +++ b/tests/unit/java/android/net/nsd/NsdServiceInfoTest.java @@ -125,6 +125,7 @@ public class NsdServiceInfoTest { fullInfo.setPort(4242); fullInfo.setHost(LOCALHOST); fullInfo.setNetwork(new Network(123)); + fullInfo.setInterfaceIndex(456); checkParcelable(fullInfo); NsdServiceInfo noHostInfo = new NsdServiceInfo(); @@ -175,6 +176,7 @@ public class NsdServiceInfoTest { assertEquals(original.getHost(), result.getHost()); assertTrue(original.getPort() == result.getPort()); assertEquals(original.getNetwork(), result.getNetwork()); + assertEquals(original.getInterfaceIndex(), result.getInterfaceIndex()); // Assert equality of attribute map. Map originalMap = original.getAttributes(); diff --git a/tests/unit/java/com/android/server/NsdServiceTest.java b/tests/unit/java/com/android/server/NsdServiceTest.java index 7ecf1f3d60..9365beeb47 100644 --- a/tests/unit/java/com/android/server/NsdServiceTest.java +++ b/tests/unit/java/com/android/server/NsdServiceTest.java @@ -19,7 +19,9 @@ package com.android.server; import static libcore.junit.util.compat.CoreCompatChangeRule.DisableCompatChanges; import static libcore.junit.util.compat.CoreCompatChangeRule.EnableCompatChanges; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.any; @@ -37,6 +39,12 @@ import static org.mockito.Mockito.when; import android.compat.testing.PlatformCompatChangeRule; import android.content.ContentResolver; import android.content.Context; +import android.net.INetd; +import android.net.InetAddresses; +import android.net.mdns.aidl.DiscoveryInfo; +import android.net.mdns.aidl.GetAddressInfo; +import android.net.mdns.aidl.IMDnsEventListener; +import android.net.mdns.aidl.ResolutionInfo; import android.net.nsd.INsdManagerCallback; import android.net.nsd.INsdServiceConnector; import android.net.nsd.MDnsManager; @@ -64,6 +72,7 @@ import org.junit.Test; import org.junit.rules.TestRule; import org.junit.runner.RunWith; import org.mockito.AdditionalAnswers; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -280,6 +289,105 @@ public class NsdServiceTest { verify(mMockMDnsM, never()).stopDaemon(); } + @Test + public void testDiscoverOnTetheringDownstream() throws Exception { + NsdService service = makeService(); + NsdManager client = connectClient(service); + + final String serviceType = "a_type"; + final String serviceName = "a_name"; + final String domainName = "mytestdevice.local"; + final int interfaceIdx = 123; + final NsdManager.DiscoveryListener discListener = mock(NsdManager.DiscoveryListener.class); + client.discoverServices(serviceType, NsdManager.PROTOCOL_DNS_SD, discListener); + waitForIdle(); + + final ArgumentCaptor listenerCaptor = + ArgumentCaptor.forClass(IMDnsEventListener.class); + verify(mMockMDnsM).registerEventListener(listenerCaptor.capture()); + final ArgumentCaptor discIdCaptor = ArgumentCaptor.forClass(Integer.class); + verify(mMockMDnsM).discover(discIdCaptor.capture(), eq(serviceType), + eq(0) /* interfaceIdx */); + // NsdManager uses a separate HandlerThread to dispatch callbacks (on ServiceHandler), so + // this needs to use a timeout + verify(discListener, timeout(TIMEOUT_MS)).onDiscoveryStarted(serviceType); + + final DiscoveryInfo discoveryInfo = new DiscoveryInfo( + discIdCaptor.getValue(), + IMDnsEventListener.SERVICE_FOUND, + serviceName, + serviceType, + domainName, + interfaceIdx, + INetd.LOCAL_NET_ID); // LOCAL_NET_ID (99) used on tethering downstreams + final IMDnsEventListener eventListener = listenerCaptor.getValue(); + eventListener.onServiceDiscoveryStatus(discoveryInfo); + waitForIdle(); + + final ArgumentCaptor discoveredInfoCaptor = + ArgumentCaptor.forClass(NsdServiceInfo.class); + verify(discListener, timeout(TIMEOUT_MS)).onServiceFound(discoveredInfoCaptor.capture()); + final NsdServiceInfo foundInfo = discoveredInfoCaptor.getValue(); + assertEquals(serviceName, foundInfo.getServiceName()); + assertEquals(serviceType, foundInfo.getServiceType()); + assertNull(foundInfo.getHost()); + assertNull(foundInfo.getNetwork()); + assertEquals(interfaceIdx, foundInfo.getInterfaceIndex()); + + // After discovering the service, verify resolving it + final NsdManager.ResolveListener resolveListener = mock(NsdManager.ResolveListener.class); + client.resolveService(foundInfo, resolveListener); + waitForIdle(); + + final ArgumentCaptor resolvIdCaptor = ArgumentCaptor.forClass(Integer.class); + verify(mMockMDnsM).resolve(resolvIdCaptor.capture(), eq(serviceName), eq(serviceType), + eq("local.") /* domain */, eq(interfaceIdx)); + + final int servicePort = 10123; + final String serviceFullName = serviceName + "." + serviceType; + final ResolutionInfo resolutionInfo = new ResolutionInfo( + resolvIdCaptor.getValue(), + IMDnsEventListener.SERVICE_RESOLVED, + null /* serviceName */, + null /* serviceType */, + null /* domain */, + serviceFullName, + domainName, + servicePort, + new byte[0] /* txtRecord */, + interfaceIdx); + + doReturn(true).when(mMockMDnsM).getServiceAddress(anyInt(), any(), anyInt()); + eventListener.onServiceResolutionStatus(resolutionInfo); + waitForIdle(); + + final ArgumentCaptor getAddrIdCaptor = ArgumentCaptor.forClass(Integer.class); + verify(mMockMDnsM).getServiceAddress(getAddrIdCaptor.capture(), eq(domainName), + eq(interfaceIdx)); + + final String serviceAddress = "192.0.2.123"; + final GetAddressInfo addressInfo = new GetAddressInfo( + getAddrIdCaptor.getValue(), + IMDnsEventListener.SERVICE_GET_ADDR_SUCCESS, + serviceFullName, + serviceAddress, + interfaceIdx, + INetd.LOCAL_NET_ID); + eventListener.onGettingServiceAddressStatus(addressInfo); + waitForIdle(); + + final ArgumentCaptor resInfoCaptor = + ArgumentCaptor.forClass(NsdServiceInfo.class); + verify(resolveListener, timeout(TIMEOUT_MS)).onServiceResolved(resInfoCaptor.capture()); + final NsdServiceInfo resolvedService = resInfoCaptor.getValue(); + assertEquals(serviceName, resolvedService.getServiceName()); + assertEquals("." + serviceType, resolvedService.getServiceType()); + assertEquals(InetAddresses.parseNumericAddress(serviceAddress), resolvedService.getHost()); + assertEquals(servicePort, resolvedService.getPort()); + assertNull(resolvedService.getNetwork()); + assertEquals(interfaceIdx, resolvedService.getInterfaceIndex()); + } + private void waitForIdle() { HandlerUtils.waitForIdle(mHandler, TIMEOUT_MS); }