Merge "Fix cache flush always causing response change"

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.
      *

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<MdnsResponse> modified = new ArraySet<>();
         final ArrayList<MdnsResponse> responses = new ArrayList<>(existingResponses.size());
+        final ArrayMap<MdnsResponse, MdnsResponse> 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,19 +264,37 @@ public class MdnsResponseDecoder {
                         findResponsesWithHostName(responses, inetRecord.getName());
                 for (MdnsResponse response : matchingResponses) {
                     if (assignInetRecord(response, inetRecord)) {
+                        final MdnsResponse originalResponse = augmentedToOriginal.get(response);
+                        if (originalResponse == null
+                                || !originalResponse.hasIdenticalRecord(inetRecord)) {
                             modified.add(response);
                         }
                     }
+                }
             } else {
                 MdnsResponse response =
                         findResponseWithHostName(responses, inetRecord.getName());
                 if (response != null) {
                     if (assignInetRecord(response, inetRecord)) {
+                        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);
     }

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<MdnsResponse> 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<MdnsResponse> 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()));

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 */);