diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java b/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java index 60859f8c106204d6c25e4414c9e4a419f55135f2..0b60572d5a2b5ce2e1c98c10993b684baa0f8d05 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java @@ -253,12 +253,13 @@ public class MdnsAdvertiser { private boolean hasAnyServiceConflict( @NonNull BiPredicate<Network, InterfaceAdvertiserRequest> applicableAdvertiserFilter, - @NonNull NsdServiceInfo newInfo) { + @NonNull NsdServiceInfo newInfo, + @NonNull Registration originalRegistration) { return any( mAdvertiserRequests, (network, adv) -> applicableAdvertiserFilter.test(network, adv) - && adv.hasServiceConflict(newInfo)); + && adv.hasServiceConflict(newInfo, originalRegistration)); } private boolean hasAnyHostConflict( @@ -283,7 +284,7 @@ public class MdnsAdvertiser { NsdServiceInfo newInfo = registration.getServiceInfo(); int renameServiceCount = 0; - while (hasAnyServiceConflict(applicableAdvertiserFilter, newInfo)) { + while (hasAnyServiceConflict(applicableAdvertiserFilter, newInfo, registration)) { renameServiceCount++; newInfo = registration.makeNewServiceInfoForServiceConflict(renameServiceCount); } @@ -378,8 +379,9 @@ public class MdnsAdvertiser { * Return whether using the proposed new {@link NsdServiceInfo} to add a registration would * cause a conflict of the service in this {@link InterfaceAdvertiserRequest}. */ - boolean hasServiceConflict(@NonNull NsdServiceInfo newInfo) { - return getConflictingRegistrationDueToService(newInfo) >= 0; + boolean hasServiceConflict( + @NonNull NsdServiceInfo newInfo, @NonNull Registration originalRegistration) { + return getConflictingRegistrationDueToService(newInfo, originalRegistration) >= 0; } /** @@ -393,11 +395,16 @@ public class MdnsAdvertiser { } /** Get the ID of a conflicting registration due to service, or -1 if none. */ - int getConflictingRegistrationDueToService(@NonNull NsdServiceInfo info) { + int getConflictingRegistrationDueToService( + @NonNull NsdServiceInfo info, @NonNull Registration originalRegistration) { if (TextUtils.isEmpty(info.getServiceName())) { return -1; } for (int i = 0; i < mPendingRegistrations.size(); i++) { + // Never conflict with itself + if (mPendingRegistrations.valueAt(i) == originalRegistration) { + continue; + } final NsdServiceInfo other = mPendingRegistrations.valueAt(i).getServiceInfo(); if (MdnsUtils.equalsIgnoreDnsCase(info.getServiceName(), other.getServiceName()) && MdnsUtils.equalsIgnoreDnsCase(info.getServiceType(), diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java b/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java index f1deab0e678710f1d476209b7251fe7a7119d6b3..aa51c4177329cdd35c0c9d914c29d5f7cba53584 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsInterfaceAdvertiser.java @@ -347,6 +347,7 @@ public class MdnsInterfaceAdvertiser implements MulticastPacketReader.PacketHand final MdnsProber.ProbingInfo probingInfo = mRecordRepository.setServiceProbing(serviceId); if (probingInfo == null) return false; + mAnnouncer.stop(serviceId); mProber.restartForConflict(probingInfo); return true; } diff --git a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt index c368d5bc3e74b6d04b7dcbe21540b2ef6f06c6d6..9aa3c84fb1835de61bbeee923950c1f70b09bd3b 100644 --- a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt +++ b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt @@ -1220,8 +1220,7 @@ class NsdManagerTest { // Registration must use an updated hostname to avoid the conflict val cb = registrationRecord.expectCallback<ServiceRegistered>(REGISTRATION_TIMEOUT_MS) // Service name is not renamed because there's no conflict on the service name. - // TODO: b/283053491 - enable this check -// assertEquals(serviceName, cb.serviceInfo.serviceName) + assertEquals(serviceName, cb.serviceInfo.serviceName) val hostname = cb.serviceInfo.hostname ?: fail("Missing hostname") hostname.let { assertTrue("Unexpected registered hostname: $it", @@ -1346,8 +1345,77 @@ class NsdManagerTest { } } - // TODO: b/322282952 - Add the test case that the hostname is renamed due to a conflict after - // probing succeeded. + @Test + fun testRegisterServiceWithCustomHostAndAddresses_conflictAfterProbing_hostRenamed() { + val si = makeTestServiceInfo(testNetwork1.network).apply { + hostname = customHostname + hostAddresses = listOf( + parseNumericAddress("192.0.2.24"), + parseNumericAddress("2001:db8::3")) + } + + // Register service on testNetwork1 + val registrationRecord = NsdRegistrationRecord() + val discoveryRecord = NsdDiscoveryRecord() + val registeredService = registerService(registrationRecord, si) + val packetReader = TapPacketReader( + Handler(handlerThread.looper), + testNetwork1.iface.fileDescriptor.fileDescriptor, 1500 /* maxPacketSize */) + packetReader.startAsyncForTest() + handlerThread.waitForIdle(TIMEOUT_MS) + + tryTest { + repeat(3) { + assertNotNull(packetReader.pollForAdvertisement(serviceName, serviceType), + "Expect 3 announcements sent after initial probing") + } + + assertEquals(si.serviceName, registeredService.serviceName) + assertEquals(si.hostname, registeredService.hostname) + + nsdManager.discoverServices(serviceType, NsdManager.PROTOCOL_DNS_SD, + testNetwork1.network, { it.run() }, discoveryRecord) + val discoveredInfo = discoveryRecord.waitForServiceDiscovered( + si.serviceName, serviceType) + + // Send a conflicting announcement + val conflictingAnnouncement = buildConflictingAnnouncementForCustomHost() + packetReader.sendResponse(conflictingAnnouncement) + + // Expect to see probes (RFC6762 9., service is reset to probing state) + assertNotNull(packetReader.pollForProbe(serviceName, serviceType), + "Probe not received within timeout after conflict") + + // Send the conflicting packet again to reply to the probe + packetReader.sendResponse(conflictingAnnouncement) + + val newRegistration = + registrationRecord + .expectCallbackEventually<ServiceRegistered>(REGISTRATION_TIMEOUT_MS) { + it.serviceInfo.serviceName == serviceName + && it.serviceInfo.hostname.let { hostname -> + hostname != null + && hostname.startsWith(customHostname) + && hostname != customHostname + } + } + + val resolvedInfo = resolveService(discoveredInfo) + assertEquals(newRegistration.serviceInfo.serviceName, resolvedInfo.serviceName) + assertEquals(newRegistration.serviceInfo.hostname, resolvedInfo.hostname) + + discoveryRecord.assertNoCallback() + } cleanupStep { + nsdManager.stopServiceDiscovery(discoveryRecord) + discoveryRecord.expectCallback<DiscoveryStopped>() + } cleanupStep { + nsdManager.unregisterService(registrationRecord) + registrationRecord.expectCallback<ServiceUnregistered>() + } cleanup { + packetReader.handler.post { packetReader.stop() } + handlerThread.waitForIdle(TIMEOUT_MS) + } + } @Test fun testRegisterServiceWithCustomHostNoAddresses_noConflictAfterProbing_notRenamed() {