fazilsheik
/
android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix cache flush always causing response change 

When the cache flush bit is set on host address records in a response,
known records are cleared and replaced with the contents of the packet.
However even if the response contains the same records as before, this
caused it to be marked as modified, and response modified callbacks are
sent for every incoming packet.

Instead, ensure that the response is only marked as modified if a
previously unknown address record is added, or if a previously known
address record is removed.

The issue wasn't found by the existing testDecodeWithNoChange test,
because it used a service record for testhost2 instead of testhost1, so
the address records were ignored.

Original change: https://r.android.com/2619297

Bug: 285997766
Bug: 285084489
Test: atest CtsNetTest FrameworksNetTests

Change-Id: I178ed8398afe4354c09bdb3fa5c221377385417e
Merged-In: Ic0f19adf5d9bde7cdab766e49cf677b319a2219b

Former-commit-id: e4cbda29ef
This commit is contained in:
Yuyang Huang 2023-06-08 09:55:23 +00:00 committed by Remi NGUYEN VAN
parent b4d33fa927
commit e32015b74e
4 changed files with 90 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

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

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

@ -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;
@ -139,8 +140,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:
@ -166,7 +170,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();
@ -262,7 +266,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 {
@ -270,12 +278,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);
}

54
tests/unit/java/com/android/server/connectivity/mdns/MdnsResponseDecoderTests.java
View File

@ -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;
@ -406,6 +407,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(
@ -426,6 +452,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(
@ -493,11 +542,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()));

12
tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java
View File

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