diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsResponse.java b/service-t/src/com/android/server/connectivity/mdns/MdnsResponse.java index ec1e4627fe..e2288c1824 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsResponse.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsResponse.java @@ -95,6 +95,14 @@ public class MdnsResponse { return !isSame; } + /** + * @return True if this response contains an identical (original TTL included) record. + */ + public boolean hasIdenticalRecord(@NonNull MdnsRecord record) { + final int existing = records.indexOf(record); + return existing >= 0 && recordsAreSame(record, records.get(existing)); + } + /** * Adds a pointer record. * diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsResponseDecoder.java b/service-t/src/com/android/server/connectivity/mdns/MdnsResponseDecoder.java index 42f610771a..eff18804c5 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsResponseDecoder.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsResponseDecoder.java @@ -20,6 +20,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.net.Network; import android.os.SystemClock; +import android.util.ArrayMap; import android.util.ArraySet; import android.util.Pair; @@ -140,8 +141,11 @@ public class MdnsResponseDecoder { final ArraySet modified = new ArraySet<>(); final ArrayList responses = new ArrayList<>(existingResponses.size()); + final ArrayMap augmentedToOriginal = new ArrayMap<>(); for (MdnsResponse existing : existingResponses) { - responses.add(new MdnsResponse(existing)); + final MdnsResponse copy = new MdnsResponse(existing); + responses.add(copy); + augmentedToOriginal.put(copy, existing); } // The response records are structured in a hierarchy, where some records reference // others, as follows: @@ -167,7 +171,7 @@ public class MdnsResponseDecoder { // A: host name -> IP address // Loop 1: find PTR records, which identify distinct service instances. - long now = SystemClock.elapsedRealtime(); + long now = clock.elapsedRealtime(); for (MdnsRecord record : records) { if (record instanceof MdnsPointerRecord) { String[] name = record.getName(); @@ -260,7 +264,11 @@ public class MdnsResponseDecoder { findResponsesWithHostName(responses, inetRecord.getName()); for (MdnsResponse response : matchingResponses) { if (assignInetRecord(response, inetRecord)) { - modified.add(response); + final MdnsResponse originalResponse = augmentedToOriginal.get(response); + if (originalResponse == null + || !originalResponse.hasIdenticalRecord(inetRecord)) { + modified.add(response); + } } } } else { @@ -268,12 +276,26 @@ public class MdnsResponseDecoder { findResponseWithHostName(responses, inetRecord.getName()); if (response != null) { if (assignInetRecord(response, inetRecord)) { - modified.add(response); + final MdnsResponse originalResponse = augmentedToOriginal.get(response); + if (originalResponse == null + || !originalResponse.hasIdenticalRecord(inetRecord)) { + modified.add(response); + } } } } } + // Only responses that have new or modified address records were added to the modified set. + // Make sure responses that have lost address records are added to the set too. + for (int i = 0; i < augmentedToOriginal.size(); i++) { + final MdnsResponse augmented = augmentedToOriginal.keyAt(i); + final MdnsResponse original = augmentedToOriginal.valueAt(i); + if (augmented.getRecords().size() != original.getRecords().size()) { + modified.add(augmented); + } + } + return Pair.create(modified, responses); } diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsResponseDecoderTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsResponseDecoderTests.java index 6eb83da639..05eca844a0 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsResponseDecoderTests.java +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsResponseDecoderTests.java @@ -28,6 +28,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import android.net.Network; @@ -417,6 +418,31 @@ public class MdnsResponseDecoderTests { response.getInet4AddressRecord().getInet4Address()); } + @Test + public void testDecodeWithIpv4AddressRemove() throws IOException { + MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, List.of( + new PacketAndRecordClass(DATAIN_PTR_1, + MdnsPointerRecord.class), + new PacketAndRecordClass(DATAIN_SERVICE_1, + MdnsServiceRecord.class), + new PacketAndRecordClass(DATAIN_IPV4_1, + MdnsInet4AddressRecord.class), + new PacketAndRecordClass(DATAIN_IPV4_2, + MdnsInet4AddressRecord.class))); + // Now update the response removing the second v4 address + final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null); + + final byte[] removedAddrResponse = makeResponsePacket( + List.of(DATAIN_PTR_1, DATAIN_SERVICE_1, DATAIN_IPV4_1)); + final ArraySet changes = decode( + decoder, removedAddrResponse, List.of(response)); + + assertEquals(1, changes.size()); + assertEquals(1, changes.valueAt(0).getInet4AddressRecords().size()); + assertEquals(parseNumericAddress("10.1.2.3"), + changes.valueAt(0).getInet4AddressRecords().get(0).getInet4Address()); + } + @Test public void testDecodeWithIpv6AddressChange() throws IOException { MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, List.of( @@ -437,6 +463,29 @@ public class MdnsResponseDecoderTests { response.getInet6AddressRecord().getInet6Address()); } + @Test + public void testDecodeWithIpv6AddressRemoved() throws IOException { + MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, List.of( + new PacketAndRecordClass(DATAIN_PTR_1, + MdnsPointerRecord.class), + new PacketAndRecordClass(DATAIN_SERVICE_1, + MdnsServiceRecord.class), + new PacketAndRecordClass(DATAIN_IPV6_1, + MdnsInet6AddressRecord.class), + new PacketAndRecordClass(DATAIN_IPV6_2, + MdnsInet6AddressRecord.class))); + // Now update the response adding an address + final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null); + final byte[] removedAddrResponse = makeResponsePacket( + List.of(DATAIN_PTR_1, DATAIN_SERVICE_1, DATAIN_IPV6_1)); + final ArraySet updatedResponses = decode( + decoder, removedAddrResponse, List.of(response)); + assertEquals(1, updatedResponses.size()); + assertEquals(1, updatedResponses.valueAt(0).getInet6AddressRecords().size()); + assertEquals(parseNumericAddress("aabb:ccdd:1122:3344:a0b0:c0d0:1020:3040"), + updatedResponses.valueAt(0).getInet6AddressRecords().get(0).getInet6Address()); + } + @Test public void testDecodeWithChangeOnText() throws IOException { MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, List.of( @@ -504,11 +553,12 @@ public class MdnsResponseDecoderTests { new PacketAndRecordClass(DATAIN_IPV4_1, MdnsInet4AddressRecord.class), new PacketAndRecordClass(DATAIN_IPV6_1, MdnsInet6AddressRecord.class), new PacketAndRecordClass(DATAIN_PTR_1, MdnsPointerRecord.class), - new PacketAndRecordClass(DATAIN_SERVICE_2, MdnsServiceRecord.class), + new PacketAndRecordClass(DATAIN_SERVICE_1, MdnsServiceRecord.class), new PacketAndRecordClass(DATAIN_TEXT_1, MdnsTextRecord.class)); // Create a two identical responses. - MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, recordList); + MdnsResponse response = makeMdnsResponse(12L /* time */, DATAIN_SERVICE_NAME_1, recordList); + doReturn(34L).when(mClock).elapsedRealtime(); final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null); final byte[] identicalResponse = makeResponsePacket( recordList.stream().map(p -> p.packetData).collect(Collectors.toList())); diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java index a696150cff..b9fdf184da 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java @@ -1078,21 +1078,17 @@ public class MdnsServiceTypeClientTests { 0 /* flags */, Collections.emptyList() /* questions */, List.of( - // TODO: cacheFlush will cause addresses to be cleared and re-added every - // time, which is considered a change and triggers extra - // onServiceChanged callbacks. Sets cacheFlush bit to false until the - // issue is fixed. new MdnsServiceRecord(serviceName, updatedReceiptTime, - false /* cacheFlush */, TEST_TTL, 0 /* servicePriority */, + true /* cacheFlush */, TEST_TTL, 0 /* servicePriority */, 0 /* serviceWeight */, 1234 /* servicePort */, hostname), new MdnsTextRecord(serviceName, updatedReceiptTime, - false /* cacheFlush */, TEST_TTL, + true /* cacheFlush */, TEST_TTL, Collections.emptyList() /* entries */), new MdnsInetAddressRecord(hostname, updatedReceiptTime, - false /* cacheFlush */, TEST_TTL, + true /* cacheFlush */, TEST_TTL, InetAddresses.parseNumericAddress(ipV4Address)), new MdnsInetAddressRecord(hostname, updatedReceiptTime, - false /* cacheFlush */, TEST_TTL, + true /* cacheFlush */, TEST_TTL, InetAddresses.parseNumericAddress(ipV6Address))), Collections.emptyList() /* authorityRecords */, Collections.emptyList() /* additionalRecords */);