Add missing SRV/TXT/address records to responses

MdnsResponses were only updated through
MdnsResponse.mergeRecordsFrom(MndsResponse), meaning that MdnsResponse
cannot be updated unless a MdnsResponse can be built from the update
packet.

Because a PTR record is required to build a MdnsResponse, this means
that it was not possible to add records to a MdnsResponse unless the
update packet contains a PTR record.

Fix this by having MdnsServiceTypeClient augment its current known
responses in addition to creating new responses from incoming
MdnsPackets.

Bug: 267570781
Test: atest NsdManagerTest#testPtrOnlyResponse with
      mdns_discovery_manager_version set to 1.
Change-Id: Iba293b4772fcd79e94ee6412c9af8225a3349f12

service-t/src/com/android/server/connectivity/mdns/MdnsResponse.java

@@ -267,78 +267,33 @@ public class MdnsResponse {
     }
 
     /**
-     * Merges any records that are present in another response into this one.
+     * Drop address records if they are for a hostname that does not match the service record.
      *
-     * @return <code>true</code> if any records were added or updated.
+     * @return True if the records were dropped.
      */
-    public synchronized boolean mergeRecordsFrom(MdnsResponse other) {
+    public synchronized boolean dropUnmatchedAddressRecords() {
-        lastUpdateTime = other.lastUpdateTime;
+        if (this.serviceRecord == null) return false;
+        boolean dropAddressRecords = false;
 
-        boolean updated = false;
+        if (this.inet4AddressRecord != null) {
-
+            if (!Arrays.equals(
-        List<MdnsPointerRecord> pointerRecords = other.getPointerRecords();
+                    this.serviceRecord.getServiceHost(), this.inet4AddressRecord.getName())) {
-        if (pointerRecords != null) {
+                dropAddressRecords = true;
-            for (MdnsPointerRecord pointerRecord : pointerRecords) {
+            }
-                if (addPointerRecord(pointerRecord)) {
+        }
-                    updated = true;
+        if (this.inet6AddressRecord != null) {
-                }
+            if (!Arrays.equals(
+                    this.serviceRecord.getServiceHost(), this.inet6AddressRecord.getName())) {
+                dropAddressRecords = true;
             }
         }
 
-        MdnsServiceRecord serviceRecord = other.getServiceRecord();
+        if (dropAddressRecords) {
-        if (serviceRecord != null) {
+            setInet4AddressRecord(null);
-            if (setServiceRecord(serviceRecord)) {
+            setInet6AddressRecord(null);
-                updated = true;
+            return true;
-            }
         }
-
+        return false;
-        MdnsTextRecord textRecord = other.getTextRecord();
-        if (textRecord != null) {
-            if (setTextRecord(textRecord)) {
-                updated = true;
-            }
-        }
-
-        MdnsInetAddressRecord otherInet4AddressRecord = other.getInet4AddressRecord();
-        if (otherInet4AddressRecord != null && otherInet4AddressRecord.getInet4Address() != null) {
-            if (setInet4AddressRecord(otherInet4AddressRecord)) {
-                updated = true;
-            }
-        }
-
-        MdnsInetAddressRecord otherInet6AddressRecord = other.getInet6AddressRecord();
-        if (otherInet6AddressRecord != null && otherInet6AddressRecord.getInet6Address() != null) {
-            if (setInet6AddressRecord(otherInet6AddressRecord)) {
-                updated = true;
-            }
-        }
-
-        // If the hostname in the service record no longer matches the hostname in either of the
-        // address records, then drop the address records.
-        if (this.serviceRecord != null) {
-            boolean dropAddressRecords = false;
-
-            if (this.inet4AddressRecord != null) {
-                if (!Arrays.equals(
-                        this.serviceRecord.getServiceHost(), this.inet4AddressRecord.getName())) {
-                    dropAddressRecords = true;
-                }
-            }
-            if (this.inet6AddressRecord != null) {
-                if (!Arrays.equals(
-                        this.serviceRecord.getServiceHost(), this.inet6AddressRecord.getName())) {
-                    dropAddressRecords = true;
-                }
-            }
-
-            if (dropAddressRecords) {
-                setInet4AddressRecord(null);
-                setInet6AddressRecord(null);
-                updated = true;
-            }
-        }
-
-        return updated;
     }
 
     /**

service-t/src/com/android/server/connectivity/mdns/MdnsResponseDecoder.java

@@ -20,12 +20,14 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.net.Network;
 import android.os.SystemClock;
+import android.util.ArraySet;
 
 import com.android.server.connectivity.mdns.util.MdnsLogger;
 
 import java.io.EOFException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 
 /** A class that decodes mDNS responses from UDP packets. */
@@ -116,16 +118,18 @@ public class MdnsResponseDecoder {
     }
 
     /**
-     * Builds mDNS responses for the desired service type from a packet.
+     * Augments a list of {@link MdnsResponse} with records from a packet. The class does not check
+     * the resulting responses for completeness; the caller should do that.
      *
-     * The class does not check the responses for completeness; the caller should do that.
+     * @param mdnsPacket the response packet with the new records
-     *
+     * @param existingResponses list of existing responses. Will not be modified.
-     * @param mdnsPacket The packet to read from.
+     * @param interfaceIndex the network interface index (or
-     * @param interfaceIndex the network interface index (or {@link
+     * {@link MdnsSocket#INTERFACE_INDEX_UNSPECIFIED} if not known) at which the packet was received
-     *     MdnsSocket#INTERFACE_INDEX_UNSPECIFIED} if not known) at which the packet was received.
      * @param network the network at which the packet was received, or null if it is unknown.
+     * @return The set of response instances that were modified or newly added.
      */
-    public List<MdnsResponse> buildResponses(@NonNull MdnsPacket mdnsPacket, int interfaceIndex,
+    public ArraySet<MdnsResponse> augmentResponses(@NonNull MdnsPacket mdnsPacket,
+            @NonNull Collection<MdnsResponse> existingResponses, int interfaceIndex,
             @Nullable Network network) {
         final ArrayList<MdnsRecord> records = new ArrayList<>(
                 mdnsPacket.questions.size() + mdnsPacket.answers.size()
@@ -134,8 +138,11 @@ public class MdnsResponseDecoder {
         records.addAll(mdnsPacket.authorityRecords);
         records.addAll(mdnsPacket.additionalRecords);
 
-        final ArrayList<MdnsResponse> responses = new ArrayList<>();
+        final ArraySet<MdnsResponse> modified = new ArraySet<>();
-
+        final ArrayList<MdnsResponse> responses = new ArrayList<>(existingResponses.size());
+        for (MdnsResponse existing : existingResponses) {
+            responses.add(new MdnsResponse(existing));
+        }
         // The response records are structured in a hierarchy, where some records reference
         // others, as follows:
         //
@@ -178,9 +185,10 @@ public class MdnsResponseDecoder {
                         response = new MdnsResponse(now, interfaceIndex, network);
                         responses.add(response);
                     }
-                    // Set interface index earlier because some responses have PTR record only.
+
-                    // Need to know every response is getting from which interface.
+                    if (response.addPointerRecord((MdnsPointerRecord) record)) {
-                    response.addPointerRecord((MdnsPointerRecord) record);
+                        modified.add(response);
+                    }
                 }
             }
         }
@@ -190,14 +198,15 @@ public class MdnsResponseDecoder {
             if (record instanceof MdnsServiceRecord) {
                 MdnsServiceRecord serviceRecord = (MdnsServiceRecord) record;
                 MdnsResponse response = findResponseWithPointer(responses, serviceRecord.getName());
-                if (response != null) {
+                if (response != null && response.setServiceRecord(serviceRecord)) {
-                    response.setServiceRecord(serviceRecord);
+                    response.dropUnmatchedAddressRecords();
+                    modified.add(response);
                 }
             } else if (record instanceof MdnsTextRecord) {
                 MdnsTextRecord textRecord = (MdnsTextRecord) record;
                 MdnsResponse response = findResponseWithPointer(responses, textRecord.getName());
-                if (response != null) {
+                if (response != null && response.setTextRecord(textRecord)) {
-                    response.setTextRecord(textRecord);
+                    modified.add(response);
                 }
             }
         }
@@ -210,26 +219,33 @@ public class MdnsResponseDecoder {
                     List<MdnsResponse> matchingResponses =
                             findResponsesWithHostName(responses, inetRecord.getName());
                     for (MdnsResponse response : matchingResponses) {
-                        assignInetRecord(response, inetRecord);
+                        if (assignInetRecord(response, inetRecord)) {
+                            modified.add(response);
+                        }
                     }
                 } else {
                     MdnsResponse response =
                             findResponseWithHostName(responses, inetRecord.getName());
                     if (response != null) {
-                        assignInetRecord(response, inetRecord);
+                        if (assignInetRecord(response, inetRecord)) {
+                            modified.add(response);
+                        }
                     }
                 }
             }
         }
-        return responses;
+
+        return modified;
     }
 
-    private static void assignInetRecord(MdnsResponse response, MdnsInetAddressRecord inetRecord) {
+    private static boolean assignInetRecord(
+            MdnsResponse response, MdnsInetAddressRecord inetRecord) {
         if (inetRecord.getInet4Address() != null) {
-            response.setInet4AddressRecord(inetRecord);
+            return response.setInet4AddressRecord(inetRecord);
         } else if (inetRecord.getInet6Address() != null) {
-            response.setInet6AddressRecord(inetRecord);
+            return response.setInet6AddressRecord(inetRecord);
         }
+        return false;
     }
 
     private static List<MdnsResponse> findResponsesWithHostName(

service-t/src/com/android/server/connectivity/mdns/MdnsServiceTypeClient.java

@@ -23,6 +23,7 @@ import android.annotation.Nullable;
 import android.net.Network;
 import android.text.TextUtils;
 import android.util.ArrayMap;
+import android.util.ArraySet;
 import android.util.Pair;
 
 import com.android.internal.annotations.GuardedBy;
@@ -50,6 +51,7 @@ public class MdnsServiceTypeClient {
     private static final int DEFAULT_MTU = 1500;
     private static final MdnsLogger LOGGER = new MdnsLogger("MdnsServiceTypeClient");
 
+
     private final String serviceType;
     private final String[] serviceTypeLabels;
     private final MdnsSocketClientBase socketClient;
@@ -216,25 +218,15 @@ public class MdnsServiceTypeClient {
      */
     public synchronized void processResponse(@NonNull MdnsPacket packet, int interfaceIndex,
             Network network) {
-        final List<MdnsResponse> responses = responseDecoder.buildResponses(packet, interfaceIndex,
+        synchronized (lock) {
-                network);
+            final ArraySet<MdnsResponse> modifiedResponses = responseDecoder.augmentResponses(
-        for (MdnsResponse response : responses) {
+                    packet, instanceNameToResponse.values(), interfaceIndex, network);
-            if (shouldRemoveServiceAfterTtlExpires()) {
+
-                // Because {@link QueryTask} and {@link processResponse} are running in different
+            for (MdnsResponse modified : modifiedResponses) {
-                // threads. We need to synchronize {@link lock} to protect
+                if (modified.isGoodbye()) {
-                // {@link instanceNameToResponse} won’t be modified at the same time.
+                    onGoodbyeReceived(modified.getServiceInstanceName());
-                synchronized (lock) {
-                    if (response.isGoodbye()) {
-                        onGoodbyeReceived(response.getServiceInstanceName());
-                    } else {
-                        onResponseReceived(response);
-                    }
-                }
-            } else {
-                if (response.isGoodbye()) {
-                    onGoodbyeReceived(response.getServiceInstanceName());
                 } else {
-                    onResponseReceived(response);
+                    onResponseModified(modified);
                 }
             }
         }
@@ -246,31 +238,26 @@ public class MdnsServiceTypeClient {
         }
     }
 
-    private void onResponseReceived(@NonNull MdnsResponse response) {
+    private void onResponseModified(@NonNull MdnsResponse response) {
-        MdnsResponse currentResponse;
+        final MdnsResponse currentResponse =
-        currentResponse = instanceNameToResponse.get(response.getServiceInstanceName());
+                instanceNameToResponse.get(response.getServiceInstanceName());
 
         boolean newServiceFound = false;
-        boolean existingServiceChanged = false;
         boolean serviceBecomesComplete = false;
         if (currentResponse == null) {
             newServiceFound = true;
-            currentResponse = response;
             String serviceInstanceName = response.getServiceInstanceName();
             if (serviceInstanceName != null) {
-                instanceNameToResponse.put(serviceInstanceName, currentResponse);
+                instanceNameToResponse.put(serviceInstanceName, response);
             }
         } else {
             boolean before = currentResponse.isComplete();
-            existingServiceChanged = currentResponse.mergeRecordsFrom(response);
+            instanceNameToResponse.put(response.getServiceInstanceName(), response);
-            boolean after = currentResponse.isComplete();
+            boolean after = response.isComplete();
             serviceBecomesComplete = !before && after;
         }
-        if (!newServiceFound && !existingServiceChanged) {
-            return;
-        }
         MdnsServiceInfo serviceInfo =
-                buildMdnsServiceInfoFromResponse(currentResponse, serviceTypeLabels);
+                buildMdnsServiceInfoFromResponse(response, serviceTypeLabels);
 
         for (int i = 0; i < listeners.size(); i++) {
             final MdnsServiceBrowserListener listener = listeners.keyAt(i);
@@ -278,7 +265,7 @@ public class MdnsServiceTypeClient {
                 listener.onServiceNameDiscovered(serviceInfo);
             }
 
-            if (currentResponse.isComplete()) {
+            if (response.isComplete()) {
                 if (newServiceFound || serviceBecomesComplete) {
                     listener.onServiceFound(serviceInfo);
                 } else {

tests/unit/java/com/android/server/connectivity/mdns/MdnsResponseDecoderTests.java

@@ -21,17 +21,21 @@ import static android.net.InetAddresses.parseNumericAddress;
 import static com.android.server.connectivity.mdns.MdnsResponseDecoder.Clock;
 import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2;
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 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.mock;
 
-import android.net.InetAddresses;
 import android.net.Network;
+import android.util.ArraySet;
 
 import com.android.net.module.util.HexDump;
+import com.android.server.connectivity.mdns.MdnsResponseTests.MdnsInet4AddressRecord;
+import com.android.server.connectivity.mdns.MdnsResponseTests.MdnsInet6AddressRecord;
 import com.android.testutils.DevSdkIgnoreRule;
 import com.android.testutils.DevSdkIgnoreRunner;
 
@@ -45,7 +49,11 @@ import java.net.DatagramPacket;
 import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetSocketAddress;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
 
 @RunWith(DevSdkIgnoreRunner.class)
 @DevSdkIgnoreRule.IgnoreUpTo(SC_V2)
@@ -148,6 +156,42 @@ public class MdnsResponseDecoderTests {
             + "010001000000780004C0A8018A0000000000000000000000000000"
             + "000000");
 
+    // MDNS record for name "testhost1" with an IPv4 address of 10.1.2.3
+    private static final byte[] DATAIN_IPV4_1 = HexDump.hexStringToByteArray(
+            "0974657374686f73743100000100010000007800040a010203");
+    // MDNS record for name "testhost1" with an IPv4 address of 10.1.2.4
+    private static final byte[] DATAIN_IPV4_2 = HexDump.hexStringToByteArray(
+            "0974657374686f73743100000100010000007800040a010204");
+    // MDNS record w/name "testhost1" & IPv6 address of aabb:ccdd:1122:3344:a0b0:c0d0:1020:3040
+    private static final byte[] DATAIN_IPV6_1 = HexDump.hexStringToByteArray(
+            "0974657374686f73743100001c0001000000780010aabbccdd11223344a0b0c0d010203040");
+    // MDNS record w/name "testhost1" & IPv6 address of aabb:ccdd:1122:3344:a0b0:c0d0:1020:3030
+    private static final byte[] DATAIN_IPV6_2 = HexDump.hexStringToByteArray(
+            "0974657374686f73743100001c0001000000780010aabbccdd11223344a0b0c0d010203030");
+    // MDNS record w/name "test" & PTR to foo.bar.quxx
+    private static final byte[] DATAIN_PTR_1 = HexDump.hexStringToByteArray(
+            "047465737400000C000100001194000E03666F6F03626172047175787800");
+    // MDNS record w/name "test" & PTR to foo.bar.quxy
+    private static final byte[] DATAIN_PTR_2 = HexDump.hexStringToByteArray(
+            "047465737400000C000100001194000E03666F6F03626172047175787900");
+    // SRV record for: scapy.DNSRRSRV(rrname='foo.bar.quxx', ttl=120, port=1234, target='testhost1')
+    private static final byte[] DATAIN_SERVICE_1 = HexDump.hexStringToByteArray(
+            "03666f6f03626172047175787800002100010000007800110000000004d20974657374686f73743100");
+    // SRV record for: scapy.DNSRRSRV(rrname='foo.bar.quxx', ttl=120, port=1234, target='testhost2')
+    private static final byte[] DATAIN_SERVICE_2 = HexDump.hexStringToByteArray(
+            "03666f6f03626172047175787800002100010000007800110000000004d20974657374686f73743200");
+    // TXT record for: scapy.DNSRR(rrname='foo.bar.quxx', type='TXT', ttl=120,
+    //     rdata=[b'a=hello there', b'b=1234567890', b'xyz=!$$$'])
+    private static final byte[] DATAIN_TEXT_1 = HexDump.hexStringToByteArray(
+            "03666f6f03626172047175787800001000010000007800240d613d68656c6c6f2074686572650c623d3132"
+                    + "33343536373839300878797a3d21242424");
+
+    // TXT record for: scapy.DNSRR(rrname='foo.bar.quxx', type='TXT', ttl=120,
+    //     rdata=[b'a=hello there', b'b=1234567890', b'xyz=!$$$'])
+    private static final byte[] DATAIN_TEXT_2 = HexDump.hexStringToByteArray(
+            "03666f6f03626172047175787800001000010000007800240d613d68656c6c6f2074686572650c623d3132"
+                    + "33343536373839300878797a3d21402324");
+
     private static final String CAST_SERVICE_NAME = "_googlecast";
     private static final String[] CAST_SERVICE_TYPE =
             new String[] {CAST_SERVICE_NAME, "_tcp", "local"};
@@ -155,7 +199,7 @@ public class MdnsResponseDecoderTests {
     private static final String[] MATTER_SERVICE_TYPE =
             new String[] {MATTER_SERVICE_NAME, "_tcp", "local"};
 
-    private List<MdnsResponse> responses;
+    private ArraySet<MdnsResponse> responses;
 
     private final Clock mClock = mock(Clock.class);
 
@@ -176,7 +220,7 @@ public class MdnsResponseDecoderTests {
 
     @Test
     public void testDecodeMultipleAnswerPacket() throws IOException {
-        MdnsResponse response = responses.get(0);
+        MdnsResponse response = responses.valueAt(0);
         assertTrue(response.isComplete());
 
         MdnsInetAddressRecord inet4AddressRecord = response.getInet4AddressRecord();
@@ -226,7 +270,7 @@ public class MdnsResponseDecoderTests {
 
         responses = decode(decoder, data6);
         assertEquals(1, responses.size());
-        MdnsResponse response = responses.get(0);
+        MdnsResponse response = responses.valueAt(0);
         assertTrue(response.isComplete());
 
         MdnsInetAddressRecord inet6AddressRecord = response.getInet6AddressRecord();
@@ -241,13 +285,13 @@ public class MdnsResponseDecoderTests {
 
     @Test
     public void testIsComplete() {
-        MdnsResponse response = new MdnsResponse(responses.get(0));
+        MdnsResponse response = new MdnsResponse(responses.valueAt(0));
         assertTrue(response.isComplete());
 
         response.clearPointerRecords();
         assertFalse(response.isComplete());
 
-        response = new MdnsResponse(responses.get(0));
+        response = new MdnsResponse(responses.valueAt(0));
         response.setInet4AddressRecord(null);
         assertFalse(response.isComplete());
 
@@ -259,11 +303,11 @@ public class MdnsResponseDecoderTests {
         response.setInet6AddressRecord(null);
         assertFalse(response.isComplete());
 
-        response = new MdnsResponse(responses.get(0));
+        response = new MdnsResponse(responses.valueAt(0));
         response.setServiceRecord(null);
         assertFalse(response.isComplete());
 
-        response = new MdnsResponse(responses.get(0));
+        response = new MdnsResponse(responses.valueAt(0));
         response.setTextRecord(null);
         assertFalse(response.isComplete());
     }
@@ -280,12 +324,13 @@ public class MdnsResponseDecoderTests {
         assertNotNull(parsedPacket);
 
         final Network network = mock(Network.class);
-        responses = decoder.buildResponses(parsedPacket,
+        responses = decoder.augmentResponses(parsedPacket,
+                /* existingResponses= */ Collections.emptyList(),
                 /* interfaceIndex= */ 10, network /* expireOnExit= */);
 
         assertEquals(responses.size(), 1);
-        assertEquals(responses.get(0).getInterfaceIndex(), 10);
+        assertEquals(responses.valueAt(0).getInterfaceIndex(), 10);
-        assertEquals(network, responses.get(0).getNetwork());
+        assertEquals(network, responses.valueAt(0).getNetwork());
     }
 
     @Test
@@ -300,17 +345,17 @@ public class MdnsResponseDecoderTests {
         // This should emit two records:
         assertEquals(2, responses.size());
 
-        MdnsResponse response1 = responses.get(0);
+        MdnsResponse response1 = responses.valueAt(0);
-        MdnsResponse response2 = responses.get(0);
+        MdnsResponse response2 = responses.valueAt(0);
 
         // Both of which are complete:
         assertTrue(response1.isComplete());
         assertTrue(response2.isComplete());
 
         // And should both have the same IPv6 address:
-        assertEquals(InetAddresses.parseNumericAddress("2605:a601:a846:5700:3e61:5ff:fe0c:89f8"),
+        assertEquals(parseNumericAddress("2605:a601:a846:5700:3e61:5ff:fe0c:89f8"),
                 response1.getInet6AddressRecord().getInet6Address());
-        assertEquals(InetAddresses.parseNumericAddress("2605:a601:a846:5700:3e61:5ff:fe0c:89f8"),
+        assertEquals(parseNumericAddress("2605:a601:a846:5700:3e61:5ff:fe0c:89f8"),
                 response2.getInet6AddressRecord().getInet6Address());
     }
 
@@ -328,17 +373,206 @@ public class MdnsResponseDecoderTests {
         assertEquals(2, responses.size());
 
         // But only the first is complete:
-        assertTrue(responses.get(0).isComplete());
+        assertTrue(responses.valueAt(0).isComplete());
-        assertFalse(responses.get(1).isComplete());
+        assertFalse(responses.valueAt(1).isComplete());
+    }
+
+    @Test
+    public void testDecodeWithIpv4AddressChange() throws IOException {
+        MdnsResponse response = makeMdnsResponse(0, List.of(
+                new PacketAndRecordClass(DATAIN_PTR_1,
+                        MdnsPointerRecord.class),
+                new PacketAndRecordClass(DATAIN_SERVICE_1,
+                        MdnsServiceRecord.class),
+                new PacketAndRecordClass(DATAIN_IPV4_1,
+                        MdnsInet4AddressRecord.class)));
+        // Now update the response with another address
+        final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null);
+        final ArraySet<MdnsResponse> updatedResponses = decode(
+                decoder, makeResponsePacket(DATAIN_IPV4_2), List.of(response));
+        assertEquals(1, updatedResponses.size());
+        assertEquals(parseNumericAddress("10.1.2.4"),
+                updatedResponses.valueAt(0).getInet4AddressRecord().getInet4Address());
+        assertEquals(parseNumericAddress("10.1.2.3"),
+                response.getInet4AddressRecord().getInet4Address());
+    }
+
+    @Test
+    public void testDecodeWithIpv6AddressChange() throws IOException {
+        MdnsResponse response = makeMdnsResponse(0, List.of(
+                new PacketAndRecordClass(DATAIN_PTR_1,
+                        MdnsPointerRecord.class),
+                new PacketAndRecordClass(DATAIN_SERVICE_1,
+                        MdnsServiceRecord.class),
+                new PacketAndRecordClass(DATAIN_IPV6_1,
+                        MdnsInet6AddressRecord.class)));
+        // Now update the response with another address
+        final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null);
+        final ArraySet<MdnsResponse> updatedResponses = decode(
+                decoder, makeResponsePacket(DATAIN_IPV6_2), List.of(response));
+        assertEquals(1, updatedResponses.size());
+        assertEquals(parseNumericAddress("aabb:ccdd:1122:3344:a0b0:c0d0:1020:3030"),
+                updatedResponses.valueAt(0).getInet6AddressRecord().getInet6Address());
+        assertEquals(parseNumericAddress("aabb:ccdd:1122:3344:a0b0:c0d0:1020:3040"),
+                response.getInet6AddressRecord().getInet6Address());
+    }
+
+    @Test
+    public void testDecodeWithChangeOnText() throws IOException {
+        MdnsResponse response = makeMdnsResponse(0, List.of(
+                new PacketAndRecordClass(DATAIN_PTR_1,
+                        MdnsPointerRecord.class),
+                new PacketAndRecordClass(DATAIN_SERVICE_1,
+                        MdnsServiceRecord.class),
+                new PacketAndRecordClass(DATAIN_TEXT_1,
+                        MdnsTextRecord.class)));
+        // Now update the response with another address
+        final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null);
+        final ArraySet<MdnsResponse> updatedResponses = decode(
+                decoder, makeResponsePacket(DATAIN_TEXT_2), List.of(response));
+        assertEquals(1, updatedResponses.size());
+        assertEquals(List.of(
+                new MdnsServiceInfo.TextEntry("a", "hello there"),
+                new MdnsServiceInfo.TextEntry("b", "1234567890"),
+                new MdnsServiceInfo.TextEntry("xyz", "!@#$")),
+                updatedResponses.valueAt(0).getTextRecord().getEntries());
+    }
+
+    @Test
+    public void testDecodeWithChangeOnService() throws IOException {
+        MdnsResponse response = makeMdnsResponse(0, List.of(
+                new PacketAndRecordClass(DATAIN_PTR_1,
+                        MdnsPointerRecord.class),
+                new PacketAndRecordClass(DATAIN_SERVICE_1,
+                        MdnsServiceRecord.class),
+                new PacketAndRecordClass(DATAIN_IPV4_1,
+                        MdnsInet4AddressRecord.class)));
+        assertArrayEquals(new String[] { "testhost1" },
+                response.getServiceRecord().getServiceHost());
+        assertNotNull(response.getInet4AddressRecord());
+        // Now update the response with another hostname
+        final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null);
+        final ArraySet<MdnsResponse> updatedResponses = decode(
+                decoder, makeResponsePacket(DATAIN_SERVICE_2), List.of(response));
+        assertEquals(1, updatedResponses.size());
+        assertArrayEquals(new String[] { "testhost2" },
+                updatedResponses.valueAt(0).getServiceRecord().getServiceHost());
+        // Hostname changed, so address records are dropped
+        assertNull(updatedResponses.valueAt(0).getInet4AddressRecord());
+    }
+
+    @Test
+    public void testDecodeWithChangeOnPtr() throws IOException {
+        MdnsResponse response = makeMdnsResponse(0, List.of(
+                new PacketAndRecordClass(DATAIN_PTR_1,
+                        MdnsPointerRecord.class),
+                new PacketAndRecordClass(DATAIN_SERVICE_1,
+                        MdnsServiceRecord.class)));
+        // Now update the response with another address
+        final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null);
+        final ArraySet<MdnsResponse> updatedResponses = decode(
+                decoder, makeResponsePacket(DATAIN_PTR_2), List.of(response));
+        assertEquals(1, updatedResponses.size());
+        assertArrayEquals(new String[] { "foo", "bar", "quxy" },
+                updatedResponses.valueAt(0).getPointerRecords().get(0).getPointer());
+    }
+
+    @Test
+    public void testDecodeWithNoChange() throws IOException {
+        List<PacketAndRecordClass> recordList =
+                Arrays.asList(
+                        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_TEXT_1, MdnsTextRecord.class));
+        // Create a two identical responses.
+        MdnsResponse response = makeMdnsResponse(0, recordList);
+
+        final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null);
+        final byte[] identicalResponse = makeResponsePacket(
+                recordList.stream().map(p -> p.packetData).collect(Collectors.toList()));
+        final ArraySet<MdnsResponse> changes = decode(
+                decoder, identicalResponse, List.of(response));
+
+        // Decoding should not indicate any change.
+        assertEquals(0, changes.size());
+    }
+
+    private static MdnsResponse makeMdnsResponse(long time, List<PacketAndRecordClass> responseList)
+            throws IOException {
+        final MdnsResponse response = new MdnsResponse(
+                time, 999 /* interfaceIndex */, mock(Network.class));
+        for (PacketAndRecordClass responseData : responseList) {
+            DatagramPacket packet =
+                    new DatagramPacket(responseData.packetData, responseData.packetData.length);
+            MdnsPacketReader reader = new MdnsPacketReader(packet);
+            String[] name = reader.readLabels();
+            reader.skip(2); // skip record type indication.
+            // Apply the right kind of record to the response.
+            if (responseData.recordClass == MdnsInet4AddressRecord.class) {
+                response.setInet4AddressRecord(new MdnsInet4AddressRecord(name, reader));
+            } else if (responseData.recordClass == MdnsInet6AddressRecord.class) {
+                response.setInet6AddressRecord(new MdnsInet6AddressRecord(name, reader));
+            } else if (responseData.recordClass == MdnsPointerRecord.class) {
+                response.addPointerRecord(new MdnsPointerRecord(name, reader));
+            } else if (responseData.recordClass == MdnsServiceRecord.class) {
+                response.setServiceRecord(new MdnsServiceRecord(name, reader));
+            } else if (responseData.recordClass == MdnsTextRecord.class) {
+                response.setTextRecord(new MdnsTextRecord(name, reader));
+            } else {
+                fail("Unsupported/unexpected MdnsRecord subtype used in test - invalid test!");
+            }
+        }
+        return response;
+    }
+
+    private static byte[] makeResponsePacket(byte[] responseRecord) throws IOException {
+        return makeResponsePacket(List.of(responseRecord));
+    }
+
+    private static byte[] makeResponsePacket(List<byte[]> responseRecords) throws IOException {
+        final MdnsPacketWriter writer = new MdnsPacketWriter(1500);
+        writer.writeUInt16(0); // Transaction ID (advertisement: 0)
+        writer.writeUInt16(0x8400); // Flags: response, authoritative
+        writer.writeUInt16(0); // questions count
+        writer.writeUInt16(responseRecords.size()); // answers count
+        writer.writeUInt16(0); // authority entries count
+        writer.writeUInt16(0); // additional records count
+
+        for (byte[] record : responseRecords) {
+            writer.writeBytes(record);
+        }
+        final DatagramPacket packet = writer.getPacket(new InetSocketAddress(0 /* port */));
+        return Arrays.copyOf(packet.getData(), packet.getLength());
     }
 
 
-    private List<MdnsResponse> decode(MdnsResponseDecoder decoder, byte[] data)
+    // This helper class just wraps the data bytes of a response packet with the contained record
+    // type.
+    // Its only purpose is to make the test code a bit more readable.
+    private static class PacketAndRecordClass {
+        public final byte[] packetData;
+        public final Class<?> recordClass;
+
+        PacketAndRecordClass(byte[] data, Class<?> c) {
+            packetData = data;
+            recordClass = c;
+        }
+    }
+
+    private ArraySet<MdnsResponse> decode(MdnsResponseDecoder decoder, byte[] data)
             throws MdnsPacket.ParseException {
+        return decode(decoder, data, Collections.emptyList());
+    }
+
+    private ArraySet<MdnsResponse> decode(MdnsResponseDecoder decoder, byte[] data,
+            Collection<MdnsResponse> existingResponses) throws MdnsPacket.ParseException {
         final MdnsPacket parsedPacket = MdnsResponseDecoder.parseResponse(data, data.length);
         assertNotNull(parsedPacket);
 
-        return decoder.buildResponses(parsedPacket,
+        return decoder.augmentResponses(parsedPacket,
+                existingResponses,
                 MdnsSocket.INTERFACE_INDEX_UNSPECIFIED, mock(Network.class));
     }
 }

tests/unit/java/com/android/server/connectivity/mdns/MdnsResponseTests.java

@@ -25,7 +25,6 @@ import static org.junit.Assert.assertFalse;
 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.mock;
 
 import static java.util.Collections.emptyList;
@@ -36,13 +35,11 @@ import com.android.net.module.util.HexDump;
 import com.android.testutils.DevSdkIgnoreRule;
 import com.android.testutils.DevSdkIgnoreRunner;
 
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import java.io.IOException;
 import java.net.DatagramPacket;
-import java.util.Arrays;
 import java.util.List;
 
 // The record test data does not use compressed names (label pointers), since that would require
@@ -52,36 +49,24 @@ import java.util.List;
 public class MdnsResponseTests {
     private static final String TAG = "MdnsResponseTests";
     // MDNS response packet for name "test" with an IPv4 address of 10.1.2.3
-    private static final byte[] dataIn_ipv4_1 = HexDump.hexStringToByteArray(
+    private static final byte[] DATAIN_IPV4 = HexDump.hexStringToByteArray(
             "0474657374000001" + "0001000011940004" + "0A010203");
-    // MDNS response packet for name "tess" with an IPv4 address of 10.1.2.4
-    private static final byte[] dataIn_ipv4_2 = HexDump.hexStringToByteArray(
-            "0474657373000001" + "0001000011940004" + "0A010204");
     // MDNS response w/name "test" & IPv6 address of aabb:ccdd:1122:3344:a0b0:c0d0:1020:3040
-    private static final byte[] dataIn_ipv6_1 = HexDump.hexStringToByteArray(
+    private static final byte[] DATAIN_IPV6 = HexDump.hexStringToByteArray(
             "047465737400001C" + "0001000011940010" + "AABBCCDD11223344" + "A0B0C0D010203040");
-    // MDNS response w/name "test" & IPv6 address of aabb:ccdd:1122:3344:a0b0:c0d0:1020:3030
-    private static final byte[] dataIn_ipv6_2 = HexDump.hexStringToByteArray(
-            "047465737400001C" + "0001000011940010" + "AABBCCDD11223344" + "A0B0C0D010203030");
     // MDNS response w/name "test" & PTR to foo.bar.quxx
-    private static final byte[] dataIn_ptr_1 = HexDump.hexStringToByteArray(
+    private static final byte[] DATAIN_PTR = HexDump.hexStringToByteArray(
             "047465737400000C" + "000100001194000E" + "03666F6F03626172" + "047175787800");
-    // MDNS response w/name "test" & PTR to foo.bar.quxy
-    private static final byte[] dataIn_ptr_2 = HexDump.hexStringToByteArray(
-            "047465737400000C" + "000100001194000E" + "03666F6F03626172" + "047175787900");
     // MDNS response w/name "test" & Service for host foo.bar.quxx
-    private static final byte[] dataIn_service_1 = HexDump.hexStringToByteArray(
+    private static final byte[] DATAIN_SERVICE = HexDump.hexStringToByteArray(
             "0474657374000021"
             + "0001000011940014"
             + "000100FF1F480366"
             + "6F6F036261720471"
             + "75787800");
-    // MDNS response w/name "test" & Service for host test
-    private static final byte[] dataIn_service_2 = HexDump.hexStringToByteArray(
-            "0474657374000021" + "000100001194000B" + "000100FF1F480474" + "657374");
     // MDNS response w/name "test" & the following text strings:
     // "a=hello there", "b=1234567890", and "xyz=!$$$"
-    private static final byte[] dataIn_text_1 = HexDump.hexStringToByteArray(
+    private static final byte[] DATAIN_TEXT = HexDump.hexStringToByteArray(
             "0474657374000010"
             + "0001000011940024"
             + "0D613D68656C6C6F"
@@ -89,16 +74,6 @@ public class MdnsResponseTests {
             + "3D31323334353637"
             + "3839300878797A3D"
             + "21242424");
-    // MDNS response w/name "test" & the following text strings:
-    // "a=hello there", "b=1234567890", and "xyz=!@#$"
-    private static final byte[] dataIn_text_2 = HexDump.hexStringToByteArray(
-            "0474657374000010"
-            + "0001000011940024"
-            + "0D613D68656C6C6F"
-            + "2074686572650C62"
-            + "3D31323334353637"
-            + "3839300878797A3D"
-            + "21402324");
 
     private static final int INTERFACE_INDEX = 999;
     private static final int TEST_TTL_MS = 120_000;
@@ -118,52 +93,6 @@ public class MdnsResponseTests {
         }
     }
 
-    // This helper class just wraps the data bytes of a response packet with the contained record
-    // type.
-    // Its only purpose is to make the test code a bit more readable.
-    static class PacketAndRecordClass {
-        public final byte[] packetData;
-        public final Class<?> recordClass;
-
-        public PacketAndRecordClass() {
-            packetData = null;
-            recordClass = null;
-        }
-
-        public PacketAndRecordClass(byte[] data, Class<?> c) {
-            packetData = data;
-            recordClass = c;
-        }
-    }
-
-    // Construct an MdnsResponse with the specified data packets applied.
-    private MdnsResponse makeMdnsResponse(long time, List<PacketAndRecordClass> responseList)
-            throws IOException {
-        MdnsResponse response = new MdnsResponse(time, INTERFACE_INDEX, mNetwork);
-        for (PacketAndRecordClass responseData : responseList) {
-            DatagramPacket packet =
-                    new DatagramPacket(responseData.packetData, responseData.packetData.length);
-            MdnsPacketReader reader = new MdnsPacketReader(packet);
-            String[] name = reader.readLabels();
-            reader.skip(2); // skip record type indication.
-            // Apply the right kind of record to the response.
-            if (responseData.recordClass == MdnsInet4AddressRecord.class) {
-                response.setInet4AddressRecord(new MdnsInet4AddressRecord(name, reader));
-            } else if (responseData.recordClass == MdnsInet6AddressRecord.class) {
-                response.setInet6AddressRecord(new MdnsInet6AddressRecord(name, reader));
-            } else if (responseData.recordClass == MdnsPointerRecord.class) {
-                response.addPointerRecord(new MdnsPointerRecord(name, reader));
-            } else if (responseData.recordClass == MdnsServiceRecord.class) {
-                response.setServiceRecord(new MdnsServiceRecord(name, reader));
-            } else if (responseData.recordClass == MdnsTextRecord.class) {
-                response.setTextRecord(new MdnsTextRecord(name, reader));
-            } else {
-                fail("Unsupported/unexpected MdnsRecord subtype used in test - invalid test!");
-            }
-        }
-        return response;
-    }
-
     private MdnsResponse makeCompleteResponse(int recordsTtlMillis) {
         final MdnsResponse response = new MdnsResponse(/* now= */ 0, INTERFACE_INDEX, mNetwork);
         final String[] hostname = new String[] { "MyHostname" };
@@ -187,7 +116,7 @@ public class MdnsResponseTests {
 
     @Test
     public void getInet4AddressRecord_returnsAddedRecord() throws IOException {
-        DatagramPacket packet = new DatagramPacket(dataIn_ipv4_1, dataIn_ipv4_1.length);
+        DatagramPacket packet = new DatagramPacket(DATAIN_IPV4, DATAIN_IPV4.length);
         MdnsPacketReader reader = new MdnsPacketReader(packet);
         String[] name = reader.readLabels();
         reader.skip(2); // skip record type indication.
@@ -200,7 +129,7 @@ public class MdnsResponseTests {
 
     @Test
     public void getInet6AddressRecord_returnsAddedRecord() throws IOException {
-        DatagramPacket packet = new DatagramPacket(dataIn_ipv6_1, dataIn_ipv6_1.length);
+        DatagramPacket packet = new DatagramPacket(DATAIN_IPV6, DATAIN_IPV6.length);
         MdnsPacketReader reader = new MdnsPacketReader(packet);
         String[] name = reader.readLabels();
         reader.skip(2); // skip record type indication.
@@ -214,7 +143,7 @@ public class MdnsResponseTests {
 
     @Test
     public void getPointerRecords_returnsAddedRecord() throws IOException {
-        DatagramPacket packet = new DatagramPacket(dataIn_ptr_1, dataIn_ptr_1.length);
+        DatagramPacket packet = new DatagramPacket(DATAIN_PTR, DATAIN_PTR.length);
         MdnsPacketReader reader = new MdnsPacketReader(packet);
         String[] name = reader.readLabels();
         reader.skip(2); // skip record type indication.
@@ -230,7 +159,7 @@ public class MdnsResponseTests {
 
     @Test
     public void getServiceRecord_returnsAddedRecord() throws IOException {
-        DatagramPacket packet = new DatagramPacket(dataIn_service_1, dataIn_service_1.length);
+        DatagramPacket packet = new DatagramPacket(DATAIN_SERVICE, DATAIN_SERVICE.length);
         MdnsPacketReader reader = new MdnsPacketReader(packet);
         String[] name = reader.readLabels();
         reader.skip(2); // skip record type indication.
@@ -243,7 +172,7 @@ public class MdnsResponseTests {
 
     @Test
     public void getTextRecord_returnsAddedRecord() throws IOException {
-        DatagramPacket packet = new DatagramPacket(dataIn_text_1, dataIn_text_1.length);
+        DatagramPacket packet = new DatagramPacket(DATAIN_TEXT, DATAIN_TEXT.length);
         MdnsPacketReader reader = new MdnsPacketReader(packet);
         String[] name = reader.readLabels();
         reader.skip(2); // skip record type indication.
@@ -275,88 +204,6 @@ public class MdnsResponseTests {
         assertEquals(mNetwork, response2.getNetwork());
     }
 
-    @Test
-    public void mergeRecordsFrom_indicates_change_on_ipv4_address() throws IOException {
-        MdnsResponse response = makeMdnsResponse(
-                0,
-                Arrays.asList(
-                        new PacketAndRecordClass(dataIn_ipv4_1, MdnsInet4AddressRecord.class)));
-        // Now create a new response that updates the address.
-        MdnsResponse response2 = makeMdnsResponse(
-                100,
-                Arrays.asList(
-                        new PacketAndRecordClass(dataIn_ipv4_2, MdnsInet4AddressRecord.class)));
-        assertTrue(response.mergeRecordsFrom(response2));
-    }
-
-    @Test
-    public void mergeRecordsFrom_indicates_change_on_ipv6_address() throws IOException {
-        MdnsResponse response = makeMdnsResponse(
-                0,
-                Arrays.asList(
-                        new PacketAndRecordClass(dataIn_ipv6_1, MdnsInet6AddressRecord.class)));
-        // Now create a new response that updates the address.
-        MdnsResponse response2 = makeMdnsResponse(
-                100,
-                Arrays.asList(
-                        new PacketAndRecordClass(dataIn_ipv6_2, MdnsInet6AddressRecord.class)));
-        assertTrue(response.mergeRecordsFrom(response2));
-    }
-
-    @Test
-    public void mergeRecordsFrom_indicates_change_on_text() throws IOException {
-        MdnsResponse response = makeMdnsResponse(
-                0,
-                Arrays.asList(new PacketAndRecordClass(dataIn_text_1, MdnsTextRecord.class)));
-        // Now create a new response that updates the address.
-        MdnsResponse response2 = makeMdnsResponse(
-                100,
-                Arrays.asList(new PacketAndRecordClass(dataIn_text_2, MdnsTextRecord.class)));
-        assertTrue(response.mergeRecordsFrom(response2));
-    }
-
-    @Test
-    public void mergeRecordsFrom_indicates_change_on_service() throws IOException {
-        MdnsResponse response = makeMdnsResponse(
-                0,
-                Arrays.asList(new PacketAndRecordClass(dataIn_service_1, MdnsServiceRecord.class)));
-        // Now create a new response that updates the address.
-        MdnsResponse response2 = makeMdnsResponse(
-                100,
-                Arrays.asList(new PacketAndRecordClass(dataIn_service_2, MdnsServiceRecord.class)));
-        assertTrue(response.mergeRecordsFrom(response2));
-    }
-
-    @Test
-    public void mergeRecordsFrom_indicates_change_on_pointer() throws IOException {
-        MdnsResponse response = makeMdnsResponse(
-                0,
-                Arrays.asList(new PacketAndRecordClass(dataIn_ptr_1, MdnsPointerRecord.class)));
-        // Now create a new response that updates the address.
-        MdnsResponse response2 = makeMdnsResponse(
-                100,
-                Arrays.asList(new PacketAndRecordClass(dataIn_ptr_2, MdnsPointerRecord.class)));
-        assertTrue(response.mergeRecordsFrom(response2));
-    }
-
-    @Test
-    @Ignore("MdnsConfigs is not configurable currently.")
-    public void mergeRecordsFrom_indicates_noChange() throws IOException {
-        //MdnsConfigsFlagsImpl.useReducedMergeRecordUpdateEvents.override(true);
-        List<PacketAndRecordClass> recordList =
-                Arrays.asList(
-                        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_text_1, MdnsTextRecord.class));
-        // Create a two identical responses.
-        MdnsResponse response = makeMdnsResponse(0, recordList);
-        MdnsResponse response2 = makeMdnsResponse(100, recordList);
-        // Merging should not indicate any change.
-        assertFalse(response.mergeRecordsFrom(response2));
-    }
-
     @Test
     public void copyConstructor() {
         final MdnsResponse response = makeCompleteResponse(TEST_TTL_MS);