Clear inetaddress fields when removing the record

When calling MdnsResponse.setInet4AddressRecord with null, the record
was removed from the records list, but not from the inet4AddressRecord /
inet6AddressRecord fields.

Fix that, and update the MdnsResponseDecoder test that missed the bug
because it modified the same incomplete MdnsResponse multiple times
(so isComplete keeps returning false), instead of using a copy of
a complete MdnsResponse.

This requires using a copy constructor, which will be useful for future
changes, so add it with a test.

Bug: 267570781
Test: atest
Change-Id: I199a8e4bc9a54a142c1fec1acee2ab6b1baa7efb
This commit is contained in:
Remi NGUYEN VAN
2023-03-02 17:46:42 +09:00
parent 016aad904d
commit af4ea5826b
3 changed files with 62 additions and 5 deletions

View File

@@ -16,12 +16,14 @@
package com.android.server.connectivity.mdns; package com.android.server.connectivity.mdns;
import android.annotation.NonNull;
import android.annotation.Nullable; import android.annotation.Nullable;
import android.net.Network; import android.net.Network;
import com.android.internal.annotations.VisibleForTesting; import com.android.internal.annotations.VisibleForTesting;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Iterator; import java.util.Iterator;
import java.util.LinkedList; import java.util.LinkedList;
@@ -48,6 +50,18 @@ public class MdnsResponse {
this.network = network; this.network = network;
} }
public MdnsResponse(@NonNull MdnsResponse base) {
records = new ArrayList<>(base.records);
pointerRecords = new ArrayList<>(base.pointerRecords);
serviceRecord = base.serviceRecord;
textRecord = base.textRecord;
inet4AddressRecord = base.inet4AddressRecord;
inet6AddressRecord = base.inet6AddressRecord;
lastUpdateTime = base.lastUpdateTime;
interfaceIndex = base.interfaceIndex;
network = base.network;
}
// This generic typed helper compares records for equality. // This generic typed helper compares records for equality.
// Returns True if records are the same. // Returns True if records are the same.
private <T> boolean recordsAreSame(T a, T b) { private <T> boolean recordsAreSame(T a, T b) {
@@ -178,6 +192,7 @@ public class MdnsResponse {
} }
if (this.inet4AddressRecord != null) { if (this.inet4AddressRecord != null) {
records.remove(this.inet4AddressRecord); records.remove(this.inet4AddressRecord);
this.inet4AddressRecord = null;
} }
if (newInet4AddressRecord != null && newInet4AddressRecord.getInet4Address() != null) { if (newInet4AddressRecord != null && newInet4AddressRecord.getInet4Address() != null) {
this.inet4AddressRecord = newInet4AddressRecord; this.inet4AddressRecord = newInet4AddressRecord;
@@ -203,6 +218,7 @@ public class MdnsResponse {
} }
if (this.inet6AddressRecord != null) { if (this.inet6AddressRecord != null) {
records.remove(this.inet6AddressRecord); records.remove(this.inet6AddressRecord);
this.inet6AddressRecord = null;
} }
if (newInet6AddressRecord != null && newInet6AddressRecord.getInet6Address() != null) { if (newInet6AddressRecord != null && newInet6AddressRecord.getInet6Address() != null) {
this.inet6AddressRecord = newInet6AddressRecord; this.inet6AddressRecord = newInet6AddressRecord;

View File

@@ -16,6 +16,8 @@
package com.android.server.connectivity.mdns; package com.android.server.connectivity.mdns;
import static android.net.InetAddresses.parseNumericAddress;
import static com.android.server.connectivity.mdns.MdnsResponseDecoder.Clock; import static com.android.server.connectivity.mdns.MdnsResponseDecoder.Clock;
import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2;
@@ -239,25 +241,29 @@ public class MdnsResponseDecoderTests {
@Test @Test
public void testIsComplete() { public void testIsComplete() {
MdnsResponse response = responses.get(0); MdnsResponse response = new MdnsResponse(responses.get(0));
assertTrue(response.isComplete()); assertTrue(response.isComplete());
response.clearPointerRecords(); response.clearPointerRecords();
assertFalse(response.isComplete()); assertFalse(response.isComplete());
response = responses.get(0); response = new MdnsResponse(responses.get(0));
response.setInet4AddressRecord(null); response.setInet4AddressRecord(null);
assertFalse(response.isComplete()); assertFalse(response.isComplete());
response = responses.get(0); response.setInet6AddressRecord(new MdnsInetAddressRecord(new String[] { "testhostname" },
0L /* receiptTimeMillis */, false /* cacheFlush */, 1234L /* ttlMillis */,
parseNumericAddress("2008:db1::123")));
assertTrue(response.isComplete());
response.setInet6AddressRecord(null); response.setInet6AddressRecord(null);
assertFalse(response.isComplete()); assertFalse(response.isComplete());
response = responses.get(0); response = new MdnsResponse(responses.get(0));
response.setServiceRecord(null); response.setServiceRecord(null);
assertFalse(response.isComplete()); assertFalse(response.isComplete());
response = responses.get(0); response = new MdnsResponse(responses.get(0));
response.setTextRecord(null); response.setTextRecord(null);
assertFalse(response.isComplete()); assertFalse(response.isComplete());
} }

View File

@@ -16,6 +16,8 @@
package com.android.server.connectivity.mdns; package com.android.server.connectivity.mdns;
import static android.net.InetAddresses.parseNumericAddress;
import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
@@ -26,6 +28,8 @@ import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static java.util.Collections.emptyList;
import android.net.Network; import android.net.Network;
import com.android.net.module.util.HexDump; import com.android.net.module.util.HexDump;
@@ -330,4 +334,35 @@ public class MdnsResponseTests {
// Merging should not indicate any change. // Merging should not indicate any change.
assertFalse(response.mergeRecordsFrom(response2)); assertFalse(response.mergeRecordsFrom(response2));
} }
@Test
public void copyConstructor() {
final MdnsResponse response = new MdnsResponse(/* now= */ 0, INTERFACE_INDEX, mNetwork);
final String[] hostname = new String[] { "MyHostname" };
final String[] serviceName = new String[] { "MyService", "_type", "_tcp", "local" };
final String[] serviceType = new String[] { "_type", "_tcp", "local" };
response.addPointerRecord(new MdnsPointerRecord(serviceType, 0L /* receiptTimeMillis */,
false /* cacheFlush */, 1234L /* ttlMillis */, serviceName));
response.setServiceRecord(new MdnsServiceRecord(serviceName, 0L /* receiptTimeMillis */,
true /* cacheFlush */, 1234L /* ttlMillis */, 0 /* servicePriority */,
0 /* serviceWeight */, 0 /* servicePort */, hostname));
response.setTextRecord(new MdnsTextRecord(serviceName, 0L /* receiptTimeMillis */,
true /* cacheFlush */, 1234L /* ttlMillis */, emptyList() /* entries */));
response.setInet4AddressRecord(new MdnsInetAddressRecord(
hostname, 0L /* receiptTimeMillis */, true /* cacheFlush */,
1234L /* ttlMillis */, parseNumericAddress("192.0.2.123")));
response.setInet6AddressRecord(new MdnsInetAddressRecord(
hostname, 0L /* receiptTimeMillis */, true /* cacheFlush */,
1234L /* ttlMillis */, parseNumericAddress("2001:db8::123")));
final MdnsResponse copy = new MdnsResponse(response);
assertEquals(response.getInet6AddressRecord(), copy.getInet6AddressRecord());
assertEquals(response.getInet4AddressRecord(), copy.getInet4AddressRecord());
assertEquals(response.getPointerRecords(), copy.getPointerRecords());
assertEquals(response.getServiceRecord(), copy.getServiceRecord());
assertEquals(response.getTextRecord(), copy.getTextRecord());
assertEquals(response.getRecords(), copy.getRecords());
assertEquals(response.getNetwork(), copy.getNetwork());
assertEquals(response.getInterfaceIndex(), copy.getInterfaceIndex());
}
} }