LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Implement proper discovery with subtypes

Browse Source 
Apps may want to discover a particular subtype, such as
_color._sub._printer._tcp.local, which may return services like
Printer1._printer._tcp.local. The previous code was trying to discover,
and would return service types named _color._sub._printer._tcp.local,
even though the actual service type is still _printer._tcp.local. This
is a regression compared to S/T.

Fix this by passing the subtype to MdnsDiscoveryManager in its options
instead of the service type, and ensure that MdnsDiscoveryManager only
sends callbacks to callers that have requested the given subtype (a
color that asked for _color._sub._printer._tcp.local should not receive
BlackAndWhite._printer._tcp.local).

Bug: 266167702
Test: atest
(cherry picked from https://android-review.googlesource.com/q/commit:f2d064112c34eb703dabdd5a48008ba4b3183a0b)
Merged-In: I21367c66534078667718a9b54dfc858b12ba7103
Change-Id: I21367c66534078667718a9b54dfc858b12ba7103
This commit is contained in:
Remi NGUYEN VAN
2023-05-11 19:18:44 +09:00
committed by Cherrypicker Worker
parent 45c1eaadb4
commit b366d24bd5
4 changed files with 214 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

81
tests/unit/java/com/android/server/NsdServiceTest.java
View File

@@ -23,7 +23,7 @@ import static android.net.nsd.NsdManager.FAILURE_BAD_PARAMETERS;
import static android.net.nsd.NsdManager.FAILURE_INTERNAL_ERROR;
import static android.net.nsd.NsdManager.FAILURE_OPERATION_NOT_RUNNING;
import static com.android.server.NsdService.constructServiceType;
import static com.android.server.NsdService.parseTypeAndSubtype;
import static com.android.testutils.ContextUtils.mockService;
import static libcore.junit.util.compat.CoreCompatChangeRule.DisableCompatChanges;
@@ -77,6 +77,7 @@ import android.os.IBinder;
import android.os.Looper;
import android.os.Message;
import android.os.RemoteException;
import android.util.Pair;
import androidx.annotation.NonNull;
import androidx.test.filters.SmallTest;
@@ -84,6 +85,7 @@ import androidx.test.filters.SmallTest;
import com.android.server.NsdService.Dependencies;
import com.android.server.connectivity.mdns.MdnsAdvertiser;
import com.android.server.connectivity.mdns.MdnsDiscoveryManager;
import com.android.server.connectivity.mdns.MdnsSearchOptions;
import com.android.server.connectivity.mdns.MdnsServiceBrowserListener;
import com.android.server.connectivity.mdns.MdnsServiceInfo;
import com.android.server.connectivity.mdns.MdnsSocketProvider;
@@ -104,6 +106,7 @@ import org.mockito.MockitoAnnotations;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Objects;
@@ -968,6 +971,35 @@ public class NsdServiceTest {
.onStartDiscoveryFailed(serviceTypeWithoutTcpOrUdpEnding, FAILURE_INTERNAL_ERROR);
}
@Test
@EnableCompatChanges(ENABLE_PLATFORM_MDNS_BACKEND)
public void testDiscoveryWithMdnsDiscoveryManager_UsesSubtypes() {
final String typeWithSubtype = SERVICE_TYPE + ",_subtype";
final NsdManager client = connectClient(mService);
final NsdServiceInfo regInfo = new NsdServiceInfo("Instance", typeWithSubtype);
final Network network = new Network(999);
regInfo.setHostAddresses(List.of(parseNumericAddress("192.0.2.123")));
regInfo.setPort(12345);
regInfo.setNetwork(network);
final RegistrationListener regListener = mock(RegistrationListener.class);
client.registerService(regInfo, NsdManager.PROTOCOL_DNS_SD, Runnable::run, regListener);
waitForIdle();
// TODO: also pass the subtype to MdnsAdvertiser
verify(mAdvertiser).addService(anyInt(), argThat(s ->
"Instance".equals(s.getServiceName())
&& SERVICE_TYPE.equals(s.getServiceType())));
final DiscoveryListener discListener = mock(DiscoveryListener.class);
client.discoverServices(typeWithSubtype, PROTOCOL, network, Runnable::run, discListener);
waitForIdle();
final ArgumentCaptor<MdnsSearchOptions> optionsCaptor =
ArgumentCaptor.forClass(MdnsSearchOptions.class);
verify(mDiscoveryManager).registerListener(eq(SERVICE_TYPE + ".local"), any(),
optionsCaptor.capture());
assertEquals(Collections.singletonList("subtype"), optionsCaptor.getValue().getSubtypes());
}
@Test
public void testResolutionWithMdnsDiscoveryManager() throws UnknownHostException {
setMdnsDiscoveryManagerEnabled();
@@ -976,7 +1008,7 @@ public class NsdServiceTest {
final ResolveListener resolveListener = mock(ResolveListener.class);
final Network network = new Network(999);
final String serviceType = "_nsd._service._tcp";
final String constructedServiceType = "_nsd._sub._service._tcp.local";
final String constructedServiceType = "_service._tcp.local";
final ArgumentCaptor<MdnsServiceBrowserListener> listenerCaptor =
ArgumentCaptor.forClass(MdnsServiceBrowserListener.class);
final NsdServiceInfo request = new NsdServiceInfo(SERVICE_NAME, serviceType);
@@ -984,12 +1016,14 @@ public class NsdServiceTest {
client.resolveService(request, resolveListener);
waitForIdle();
verify(mSocketProvider).startMonitoringSockets();
// TODO(b/266167702): this is a bug, as registerListener should be done _service._tcp, and
// _sub should be in the list of subtypes in the options.
final ArgumentCaptor<MdnsSearchOptions> optionsCaptor =
ArgumentCaptor.forClass(MdnsSearchOptions.class);
verify(mDiscoveryManager).registerListener(eq(constructedServiceType),
listenerCaptor.capture(), argThat(options ->
network.equals(options.getNetwork())
&& SERVICE_NAME.equals(options.getResolveInstanceName())));
listenerCaptor.capture(),
optionsCaptor.capture());
assertEquals(network, optionsCaptor.getValue().getNetwork());
// Subtypes are not used for resolution, only for discovery
assertEquals(Collections.emptyList(), optionsCaptor.getValue().getSubtypes());
final MdnsServiceBrowserListener listener = listenerCaptor.getValue();
final MdnsServiceInfo mdnsServiceInfo = new MdnsServiceInfo(
@@ -1013,8 +1047,7 @@ public class NsdServiceTest {
verify(resolveListener, timeout(TIMEOUT_MS)).onServiceResolved(infoCaptor.capture());
final NsdServiceInfo info = infoCaptor.getValue();
assertEquals(SERVICE_NAME, info.getServiceName());
// TODO(b/266167702): this should be ._service._tcp (as per legacy behavior)
assertEquals("._nsd._sub._service._tcp", info.getServiceType());
assertEquals("._service._tcp", info.getServiceType());
assertEquals(PORT, info.getPort());
assertTrue(info.getAttributes().containsKey("key"));
assertEquals(1, info.getAttributes().size());
@@ -1222,7 +1255,7 @@ public class NsdServiceTest {
final ResolveListener resolveListener = mock(ResolveListener.class);
final Network network = new Network(999);
final String serviceType = "_nsd._service._tcp";
final String constructedServiceType = "_nsd._sub._service._tcp.local";
final String constructedServiceType = "_service._tcp.local";
final ArgumentCaptor<MdnsServiceBrowserListener> listenerCaptor =
ArgumentCaptor.forClass(MdnsServiceBrowserListener.class);
final NsdServiceInfo request = new NsdServiceInfo(SERVICE_NAME, serviceType);
@@ -1230,8 +1263,14 @@ public class NsdServiceTest {
client.resolveService(request, resolveListener);
waitForIdle();
verify(mSocketProvider).startMonitoringSockets();
final ArgumentCaptor<MdnsSearchOptions> optionsCaptor =
ArgumentCaptor.forClass(MdnsSearchOptions.class);
verify(mDiscoveryManager).registerListener(eq(constructedServiceType),
listenerCaptor.capture(), argThat(options -> network.equals(options.getNetwork())));
listenerCaptor.capture(),
optionsCaptor.capture());
assertEquals(network, optionsCaptor.getValue().getNetwork());
// Subtypes are not used for resolution, only for discovery
assertEquals(Collections.emptyList(), optionsCaptor.getValue().getSubtypes());
client.stopServiceResolution(resolveListener);
waitForIdle();
@@ -1246,16 +1285,22 @@ public class NsdServiceTest {
}
@Test
public void testConstructServiceType() {
public void testParseTypeAndSubtype() {
final String serviceType1 = "test._tcp";
final String serviceType2 = "_test._quic";
final String serviceType3 = "_123._udp.";
final String serviceType4 = "_TEST._999._tcp.";
final String serviceType3 = "_test._quic,_test1,_test2";
final String serviceType4 = "_123._udp.";
final String serviceType5 = "_TEST._999._tcp.";
final String serviceType6 = "_998._tcp.,_TEST";
final String serviceType7 = "_997._tcp,_TEST";
assertEquals(null, constructServiceType(serviceType1));
assertEquals(null, constructServiceType(serviceType2));
assertEquals("_123._udp", constructServiceType(serviceType3));
assertEquals("_TEST._sub._999._tcp", constructServiceType(serviceType4));
assertNull(parseTypeAndSubtype(serviceType1));
assertNull(parseTypeAndSubtype(serviceType2));
assertNull(parseTypeAndSubtype(serviceType3));
assertEquals(new Pair<>("_123._udp", null), parseTypeAndSubtype(serviceType4));
assertEquals(new Pair<>("_999._tcp", "_TEST"), parseTypeAndSubtype(serviceType5));
assertEquals(new Pair<>("_998._tcp", "_TEST"), parseTypeAndSubtype(serviceType6));
assertEquals(new Pair<>("_997._tcp", "_TEST"), parseTypeAndSubtype(serviceType7));
}
@Test

83
tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java
View File

@@ -41,6 +41,7 @@ import android.net.InetAddresses;
import android.net.Network;
import android.text.TextUtils;
import com.android.net.module.util.CollectionUtils;
import com.android.net.module.util.SharedLog;
import com.android.server.connectivity.mdns.MdnsServiceInfo.TextEntry;
import com.android.server.connectivity.mdns.MdnsServiceTypeClient.QueryTaskConfig;
@@ -52,6 +53,7 @@ import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatcher;
import org.mockito.Captor;
import org.mockito.InOrder;
import org.mockito.Mock;
@@ -1056,6 +1058,87 @@ public class MdnsServiceTypeClientTests {
inOrder.verify(mockListenerTwo).onServiceRemoved(matchServiceName(otherInstance));
}
@Test
public void testProcessResponse_SubtypeDiscoveryLimitedToSubtype() {
client = new MdnsServiceTypeClient(
SERVICE_TYPE, mockSocketClient, currentThreadExecutor, mockNetwork, mockSharedLog);
final String matchingInstance = "instance1";
final String subtype = "_subtype";
final String otherInstance = "instance2";
final String ipV4Address = "192.0.2.0";
final String ipV6Address = "2001:db8::";
final MdnsSearchOptions options = MdnsSearchOptions.newBuilder()
// Search with different case. Note MdnsSearchOptions subtype doesn't start with "_"
.addSubtype("Subtype").build();
client.startSendAndReceive(mockListenerOne, options);
client.startSendAndReceive(mockListenerTwo, MdnsSearchOptions.getDefaultOptions());
// Complete response from instanceName
final MdnsPacket packetWithoutSubtype = createResponse(
matchingInstance, ipV4Address, 5353, SERVICE_TYPE_LABELS,
Collections.emptyMap() /* textAttributes */, TEST_TTL);
final MdnsPointerRecord originalPtr = (MdnsPointerRecord) CollectionUtils.findFirst(
packetWithoutSubtype.answers, r -> r instanceof MdnsPointerRecord);
// Add a subtype PTR record
final ArrayList<MdnsRecord> newAnswers = new ArrayList<>(packetWithoutSubtype.answers);
newAnswers.add(new MdnsPointerRecord(
// PTR should be _subtype._sub._type._tcp.local -> instance1._type._tcp.local
Stream.concat(Stream.of(subtype, "_sub"), Arrays.stream(SERVICE_TYPE_LABELS))
.toArray(String[]::new),
originalPtr.getReceiptTime(), originalPtr.getCacheFlush(), originalPtr.getTtl(),
originalPtr.getPointer()));
final MdnsPacket packetWithSubtype = new MdnsPacket(
packetWithoutSubtype.flags,
packetWithoutSubtype.questions,
newAnswers,
packetWithoutSubtype.authorityRecords,
packetWithoutSubtype.additionalRecords);
client.processResponse(packetWithSubtype, INTERFACE_INDEX, mockNetwork);
// Complete response from otherInstanceName, without subtype
client.processResponse(createResponse(
otherInstance, ipV4Address, 5353, SERVICE_TYPE_LABELS,
Collections.emptyMap() /* textAttributes */, TEST_TTL),
INTERFACE_INDEX, mockNetwork);
// Address update from otherInstanceName
client.processResponse(createResponse(
otherInstance, ipV6Address, 5353, SERVICE_TYPE_LABELS,
Collections.emptyMap(), TEST_TTL), INTERFACE_INDEX, mockNetwork);
// Goodbye from otherInstanceName
client.processResponse(createResponse(
otherInstance, ipV6Address, 5353, SERVICE_TYPE_LABELS,
Collections.emptyMap(), 0L /* ttl */), INTERFACE_INDEX, mockNetwork);
// mockListenerOne gets notified for the requested instance
final ArgumentMatcher<MdnsServiceInfo> subtypeInstanceMatcher = info ->
info.getServiceInstanceName().equals(matchingInstance)
&& info.getSubtypes().equals(Collections.singletonList(subtype));
verify(mockListenerOne).onServiceNameDiscovered(argThat(subtypeInstanceMatcher));
verify(mockListenerOne).onServiceFound(argThat(subtypeInstanceMatcher));
// ...but does not get any callback for the other instance
verify(mockListenerOne, never()).onServiceFound(matchServiceName(otherInstance));
verify(mockListenerOne, never()).onServiceNameDiscovered(matchServiceName(otherInstance));
verify(mockListenerOne, never()).onServiceUpdated(matchServiceName(otherInstance));
verify(mockListenerOne, never()).onServiceRemoved(matchServiceName(otherInstance));
// mockListenerTwo gets notified for both though
final InOrder inOrder = inOrder(mockListenerTwo);
inOrder.verify(mockListenerTwo).onServiceNameDiscovered(argThat(subtypeInstanceMatcher));
inOrder.verify(mockListenerTwo).onServiceFound(argThat(subtypeInstanceMatcher));
inOrder.verify(mockListenerTwo).onServiceNameDiscovered(matchServiceName(otherInstance));
inOrder.verify(mockListenerTwo).onServiceFound(matchServiceName(otherInstance));
inOrder.verify(mockListenerTwo).onServiceUpdated(matchServiceName(otherInstance));
inOrder.verify(mockListenerTwo).onServiceRemoved(matchServiceName(otherInstance));
}
@Test
public void testNotifyAllServicesRemoved() {
client = new MdnsServiceTypeClient(