Merge "Do not send empty TXT records" into main
diff --git a/service-t/src/com/android/server/NsdService.java b/service-t/src/com/android/server/NsdService.java
index 419ec3a..5f672e7 100644
--- a/service-t/src/com/android/server/NsdService.java
+++ b/service-t/src/com/android/server/NsdService.java
@@ -1936,6 +1936,8 @@
mContext, MdnsFeatureFlags.NSD_AGGRESSIVE_QUERY_MODE))
.setIsQueryWithKnownAnswerEnabled(mDeps.isFeatureEnabled(
mContext, MdnsFeatureFlags.NSD_QUERY_WITH_KNOWN_ANSWER))
+ .setAvoidAdvertisingEmptyTxtRecords(mDeps.isTetheringFeatureNotChickenedOut(
+ mContext, MdnsFeatureFlags.NSD_AVOID_ADVERTISING_EMPTY_TXT_RECORDS))
.setOverrideProvider(flag -> mDeps.isFeatureEnabled(
mContext, FORCE_ENABLE_FLAG_FOR_TEST_PREFIX + flag))
.build();
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsFeatureFlags.java b/service-t/src/com/android/server/connectivity/mdns/MdnsFeatureFlags.java
index c264f25..709dc79 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsFeatureFlags.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsFeatureFlags.java
@@ -67,6 +67,12 @@
*/
public static final String NSD_QUERY_WITH_KNOWN_ANSWER = "nsd_query_with_known_answer";
+ /**
+ * A feature flag to avoid advertising empty TXT records, as per RFC 6763 6.1.
+ */
+ public static final String NSD_AVOID_ADVERTISING_EMPTY_TXT_RECORDS =
+ "nsd_avoid_advertising_empty_txt_records";
+
// Flag for offload feature
public final boolean mIsMdnsOffloadFeatureEnabled;
@@ -91,6 +97,9 @@
// Flag for query with known-answer
public final boolean mIsQueryWithKnownAnswerEnabled;
+ // Flag for avoiding advertising empty TXT records
+ public final boolean mAvoidAdvertisingEmptyTxtRecords;
+
@Nullable
private final FlagOverrideProvider mOverrideProvider;
@@ -142,6 +151,15 @@
}
/**
+ * Indicates whether {@link #NSD_AVOID_ADVERTISING_EMPTY_TXT_RECORDS} is enabled, including for
+ * testing.
+ */
+ public boolean avoidAdvertisingEmptyTxtRecords() {
+ return mAvoidAdvertisingEmptyTxtRecords
+ || isForceEnabledForTest(NSD_AVOID_ADVERTISING_EMPTY_TXT_RECORDS);
+ }
+
+ /**
* The constructor for {@link MdnsFeatureFlags}.
*/
public MdnsFeatureFlags(boolean isOffloadFeatureEnabled,
@@ -152,6 +170,7 @@
boolean isUnicastReplyEnabled,
boolean isAggressiveQueryModeEnabled,
boolean isQueryWithKnownAnswerEnabled,
+ boolean avoidAdvertisingEmptyTxtRecords,
@Nullable FlagOverrideProvider overrideProvider) {
mIsMdnsOffloadFeatureEnabled = isOffloadFeatureEnabled;
mIncludeInetAddressRecordsInProbing = includeInetAddressRecordsInProbing;
@@ -161,6 +180,7 @@
mIsUnicastReplyEnabled = isUnicastReplyEnabled;
mIsAggressiveQueryModeEnabled = isAggressiveQueryModeEnabled;
mIsQueryWithKnownAnswerEnabled = isQueryWithKnownAnswerEnabled;
+ mAvoidAdvertisingEmptyTxtRecords = avoidAdvertisingEmptyTxtRecords;
mOverrideProvider = overrideProvider;
}
@@ -181,6 +201,7 @@
private boolean mIsUnicastReplyEnabled;
private boolean mIsAggressiveQueryModeEnabled;
private boolean mIsQueryWithKnownAnswerEnabled;
+ private boolean mAvoidAdvertisingEmptyTxtRecords;
private FlagOverrideProvider mOverrideProvider;
/**
@@ -195,6 +216,7 @@
mIsUnicastReplyEnabled = true; // Default enabled.
mIsAggressiveQueryModeEnabled = false;
mIsQueryWithKnownAnswerEnabled = false;
+ mAvoidAdvertisingEmptyTxtRecords = true; // Default enabled.
mOverrideProvider = null;
}
@@ -291,6 +313,16 @@
}
/**
+ * Set whether to avoid advertising empty TXT records.
+ *
+ * @see #NSD_AVOID_ADVERTISING_EMPTY_TXT_RECORDS
+ */
+ public Builder setAvoidAdvertisingEmptyTxtRecords(boolean avoidAdvertisingEmptyTxtRecords) {
+ mAvoidAdvertisingEmptyTxtRecords = avoidAdvertisingEmptyTxtRecords;
+ return this;
+ }
+
+ /**
* Builds a {@link MdnsFeatureFlags} with the arguments supplied to this builder.
*/
public MdnsFeatureFlags build() {
@@ -302,6 +334,7 @@
mIsUnicastReplyEnabled,
mIsAggressiveQueryModeEnabled,
mIsQueryWithKnownAnswerEnabled,
+ mAvoidAdvertisingEmptyTxtRecords,
mOverrideProvider);
}
}
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java
index 0e84764..ebd95c9 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsRecordRepository.java
@@ -227,11 +227,13 @@
/**
* Create a ServiceRegistration with only update the subType.
*/
- ServiceRegistration withSubtypes(@NonNull Set<String> newSubtypes) {
+ ServiceRegistration withSubtypes(@NonNull Set<String> newSubtypes,
+ boolean avoidEmptyTxtRecords) {
NsdServiceInfo newServiceInfo = new NsdServiceInfo(serviceInfo);
newServiceInfo.setSubtypes(newSubtypes);
return new ServiceRegistration(srvRecord.record.getServiceHost(), newServiceInfo,
- repliedServiceCount, sentPacketCount, exiting, isProbing, ttl);
+ repliedServiceCount, sentPacketCount, exiting, isProbing, ttl,
+ avoidEmptyTxtRecords);
}
/**
@@ -239,7 +241,7 @@
*/
ServiceRegistration(@NonNull String[] deviceHostname, @NonNull NsdServiceInfo serviceInfo,
int repliedServiceCount, int sentPacketCount, boolean exiting, boolean isProbing,
- @Nullable Duration ttl) {
+ @Nullable Duration ttl, boolean avoidEmptyTxtRecords) {
this.serviceInfo = serviceInfo;
final long nonNameRecordsTtlMillis;
@@ -310,7 +312,8 @@
// Service name is verified unique after probing
true /* cacheFlush */,
nonNameRecordsTtlMillis,
- attrsToTextEntries(serviceInfo.getAttributes())),
+ attrsToTextEntries(
+ serviceInfo.getAttributes(), avoidEmptyTxtRecords)),
false /* sharedName */);
allRecords.addAll(ptrRecords);
@@ -393,9 +396,10 @@
* @param serviceInfo Service to advertise
*/
ServiceRegistration(@NonNull String[] deviceHostname, @NonNull NsdServiceInfo serviceInfo,
- int repliedServiceCount, int sentPacketCount, @Nullable Duration ttl) {
+ int repliedServiceCount, int sentPacketCount, @Nullable Duration ttl,
+ boolean avoidEmptyTxtRecords) {
this(deviceHostname, serviceInfo,repliedServiceCount, sentPacketCount,
- false /* exiting */, true /* isProbing */, ttl);
+ false /* exiting */, true /* isProbing */, ttl, avoidEmptyTxtRecords);
}
void setProbing(boolean probing) {
@@ -446,7 +450,7 @@
"Service ID must already exist for an update request: " + serviceId);
}
final ServiceRegistration updatedRegistration = existingRegistration.withSubtypes(
- subtypes);
+ subtypes, mMdnsFeatureFlags.avoidAdvertisingEmptyTxtRecords());
mServices.put(serviceId, updatedRegistration);
}
@@ -477,7 +481,8 @@
final ServiceRegistration registration = new ServiceRegistration(
mDeviceHostname, serviceInfo, NO_PACKET /* repliedServiceCount */,
- NO_PACKET /* sentPacketCount */, ttl);
+ NO_PACKET /* sentPacketCount */, ttl,
+ mMdnsFeatureFlags.avoidAdvertisingEmptyTxtRecords());
mServices.put(serviceId, registration);
// Remove existing exiting service
@@ -548,8 +553,17 @@
return new MdnsProber.ProbingInfo(serviceId, probingRecords);
}
- private static List<MdnsServiceInfo.TextEntry> attrsToTextEntries(Map<String, byte[]> attrs) {
- final List<MdnsServiceInfo.TextEntry> out = new ArrayList<>(attrs.size());
+ private static List<MdnsServiceInfo.TextEntry> attrsToTextEntries(Map<String, byte[]> attrs,
+ boolean avoidEmptyTxtRecords) {
+ final List<MdnsServiceInfo.TextEntry> out = new ArrayList<>(
+ attrs.size() == 0 ? 1 : attrs.size());
+ if (avoidEmptyTxtRecords && attrs.size() == 0) {
+ // As per RFC6763 6.1, empty TXT records are not allowed, but records containing a
+ // single empty String must be treated as equivalent.
+ out.add(new MdnsServiceInfo.TextEntry("", (byte[]) null));
+ return out;
+ }
+
for (Map.Entry<String, byte[]> attr : attrs.entrySet()) {
out.add(new MdnsServiceInfo.TextEntry(attr.getKey(), attr.getValue()));
}
@@ -1403,7 +1417,8 @@
if (existing == null) return null;
final ServiceRegistration newService = new ServiceRegistration(mDeviceHostname, newInfo,
- existing.repliedServiceCount, existing.sentPacketCount, existing.ttl);
+ existing.repliedServiceCount, existing.sentPacketCount, existing.ttl,
+ mMdnsFeatureFlags.avoidAdvertisingEmptyTxtRecords());
mServices.put(serviceId, newService);
return makeProbingInfo(serviceId, newService);
}
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceInfo.java b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceInfo.java
index 1ec9e39..3fb92bb 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceInfo.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceInfo.java
@@ -355,6 +355,13 @@
/** Represents a DNS TXT key-value pair defined by RFC 6763. */
public static final class TextEntry implements Parcelable {
+ /**
+ * The value to use for attributes with no value.
+ *
+ * <p>As per RFC6763 P.16, attributes may have no value, which is different from having an
+ * empty value (which would be an empty byte array).
+ */
+ public static final byte[] VALUE_NONE = null;
public static final Parcelable.Creator<TextEntry> CREATOR =
new Parcelable.Creator<TextEntry>() {
@Override
@@ -389,7 +396,7 @@
// 2. If there is no '=' in a DNS-SD TXT record string, then it is a
// boolean attribute, simply identified as being present, with no value.
if (delimitPos < 0) {
- return new TextEntry(new String(textBytes, US_ASCII), (byte[]) null);
+ return new TextEntry(new String(textBytes, US_ASCII), VALUE_NONE);
} else if (delimitPos == 0) {
return null;
}
@@ -400,13 +407,13 @@
/** Creates a new {@link TextEntry} with given key and value of a UTF-8 string. */
public TextEntry(String key, String value) {
- this(key, value == null ? null : value.getBytes(UTF_8));
+ this(key, value == null ? VALUE_NONE : value.getBytes(UTF_8));
}
/** Creates a new {@link TextEntry} with given key and value of a byte array. */
public TextEntry(String key, byte[] value) {
this.key = key;
- this.value = value == null ? null : value.clone();
+ this.value = value == VALUE_NONE ? VALUE_NONE : value.clone();
}
private TextEntry(Parcel in) {
@@ -419,22 +426,26 @@
}
public byte[] getValue() {
- return value == null ? null : value.clone();
+ return value == VALUE_NONE ? VALUE_NONE : value.clone();
}
/** Converts this {@link TextEntry} instance to '=' separated byte array. */
public byte[] toBytes() {
final byte[] keyBytes = key.getBytes(US_ASCII);
- if (value == null) {
+ if (value == VALUE_NONE) {
return keyBytes;
}
return ByteUtils.concat(keyBytes, new byte[]{'='}, value);
}
+ public boolean isEmpty() {
+ return TextUtils.isEmpty(key) && (value == VALUE_NONE || value.length == 0);
+ }
+
/** Converts this {@link TextEntry} instance to '=' separated string. */
@Override
public String toString() {
- if (value == null) {
+ if (value == VALUE_NONE) {
return key;
}
return key + "=" + new String(value, UTF_8);
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsTextRecord.java b/service-t/src/com/android/server/connectivity/mdns/MdnsTextRecord.java
index 92cf324..77d1d7a 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsTextRecord.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsTextRecord.java
@@ -89,6 +89,13 @@
}
}
+ private boolean isEmpty() {
+ return entries == null || entries.size() == 0
+ // RFC6763 6.1 indicates that a TXT record with a single zero byte is equivalent to
+ // an empty record.
+ || (entries.size() == 1 && entries.get(0).isEmpty());
+ }
+
@Override
public String toString() {
StringBuilder sb = new StringBuilder();
@@ -105,7 +112,7 @@
@Override
public int hashCode() {
- return (super.hashCode() * 31) + Objects.hash(entries);
+ return (super.hashCode() * 31) + (isEmpty() ? 0 : Objects.hash(entries));
}
@Override
@@ -116,7 +123,19 @@
if (!(other instanceof MdnsTextRecord)) {
return false;
}
-
- return super.equals(other) && Objects.equals(entries, ((MdnsTextRecord) other).entries);
+ if (!super.equals(other)) {
+ return false;
+ }
+ // As per RFC6763 6.1: DNS-SD clients MUST treat the following as equivalent:
+ // - A TXT record containing a single zero byte.
+ // - An empty (zero-length) TXT record. (This is not strictly legal, but should one be
+ // received, it should be interpreted as the same as a single empty string.)
+ // - No TXT record
+ // Ensure that empty TXT records are considered equal, so that they are not considered
+ // conflicting for example.
+ if (isEmpty() && ((MdnsTextRecord) other).isEmpty()) {
+ return true;
+ }
+ return Objects.equals(entries, ((MdnsTextRecord) other).entries);
}
}
\ No newline at end of file
diff --git a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt
index be80787..5c1099d 100644
--- a/tests/cts/net/src/android/net/cts/NsdManagerTest.kt
+++ b/tests/cts/net/src/android/net/cts/NsdManagerTest.kt
@@ -105,7 +105,6 @@
import com.android.testutils.TestableNetworkAgent
import com.android.testutils.TestableNetworkAgent.CallbackEntry.OnNetworkCreated
import com.android.testutils.TestableNetworkCallback
-import com.android.testutils.assertContainsExactly
import com.android.testutils.assertEmpty
import com.android.testutils.filters.CtsNetTestCasesMaxTargetSdk30
import com.android.testutils.filters.CtsNetTestCasesMaxTargetSdk33
@@ -128,6 +127,7 @@
import java.util.Random
import java.util.concurrent.Executor
import kotlin.math.min
+import kotlin.test.assertContentEquals
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertNotNull
@@ -2471,6 +2471,44 @@
}
}
+ @Test
+ fun testAdvertiseServiceWithNoAttributes_TxtRecordIstNotEmpty() {
+ deviceConfigRule.setConfig(
+ NAMESPACE_TETHERING,
+ "test_nsd_avoid_advertising_empty_txt_records",
+ "1"
+ )
+ val packetReader = TapPacketReader(
+ Handler(handlerThread.looper),
+ testNetwork1.iface.fileDescriptor.fileDescriptor,
+ 1500 /* maxPacketSize */
+ )
+ packetReader.startAsyncForTest()
+ handlerThread.waitForIdle(TIMEOUT_MS)
+
+ // Test behavior described in RFC6763 6.1: empty TXT records are not allowed, but TXT
+ // records with a zero length string are equivalent.
+ val si = makeTestServiceInfo(testNetwork1.network)
+ // Register service on testNetwork1
+ val registrationRecord = NsdRegistrationRecord()
+ registerService(registrationRecord, si)
+
+ tryTest {
+ val announcement =
+ packetReader.pollForReply("$serviceName.$serviceType.local", DnsResolver.TYPE_TXT)
+ assertNotNull(announcement)
+ val txtRecords = announcement.records[ANSECTION].filter {
+ it.nsType == DnsResolver.TYPE_TXT
+ }
+ assertEquals(1, txtRecords.size)
+ // The TXT record should contain as single zero
+ assertContentEquals(byteArrayOf(0), txtRecords[0].rr)
+ } cleanup {
+ nsdManager.unregisterService(registrationRecord)
+ registrationRecord.expectCallback<ServiceUnregistered>()
+ }
+ }
+
private fun hasServiceTypeClientsForNetwork(clients: List<String>, network: Network): Boolean {
return clients.any { client -> client.substring(
client.indexOf("network=") + "network=".length,
diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
index 2cb97c9..8c44abd 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordRepositoryTest.kt
@@ -172,11 +172,13 @@
private fun makeFlags(
includeInetAddressesInProbing: Boolean = false,
isKnownAnswerSuppressionEnabled: Boolean = false,
- unicastReplyEnabled: Boolean = true
+ unicastReplyEnabled: Boolean = true,
+ avoidAdvertisingEmptyTxtRecords: Boolean = true
) = MdnsFeatureFlags.Builder()
.setIncludeInetAddressRecordsInProbing(includeInetAddressesInProbing)
.setIsKnownAnswerSuppressionEnabled(isKnownAnswerSuppressionEnabled)
.setIsUnicastReplyEnabled(unicastReplyEnabled)
+ .setAvoidAdvertisingEmptyTxtRecords(avoidAdvertisingEmptyTxtRecords)
.build()
@Test
@@ -1721,6 +1723,30 @@
}
@Test
+ fun testGetConflictingServices_ZeroLengthTxtRecord_NoConflict() {
+ val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
+ repository.addServiceAndFinishProbing(TEST_SERVICE_ID_1, TEST_SERVICE_1)
+
+ val packet = MdnsPacket(
+ 0 /* flags */,
+ emptyList() /* questions */,
+ listOf(
+ MdnsTextRecord(
+ arrayOf("MyOtherTestService", "_testservice", "_tcp", "local"),
+ 0L /* receiptTimeMillis */,
+ true /* cacheFlush */,
+ SHORT_TTL,
+ listOf(TextEntry("", null as ByteArray?))
+ ),
+ ) /* answers */,
+ emptyList() /* authorityRecords */,
+ emptyList() /* additionalRecords */
+ )
+
+ assertEquals(emptyMap(), repository.getConflictingServices(packet))
+ }
+
+ @Test
fun testGetServiceRepliedRequestsCount() {
val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
repository.initWithService(TEST_SERVICE_ID_1, TEST_SERVICE_1)
@@ -2168,6 +2194,46 @@
assertEquals(knownAnswers, reply.knownAnswers)
}
+ private fun doAddServiceWithEmptyTxtRecordTest(flags: MdnsFeatureFlags): MdnsTextRecord {
+ val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, flags)
+ repository.addServiceAndFinishProbing(TEST_SERVICE_ID_1, TEST_SERVICE_1)
+
+ val questions = listOf(MdnsTextRecord(
+ arrayOf("MyTestService", "_testservice", "_tcp", "local"),
+ true /* isUnicast */
+ ))
+ val query = MdnsPacket(
+ 0 /* flags */,
+ questions,
+ emptyList() /* answers */,
+ emptyList() /* authorityRecords */,
+ emptyList() /* additionalRecords */
+ )
+ val src = InetSocketAddress(parseNumericAddress("192.0.2.123"), 5353)
+ val reply = repository.getReply(query, src)
+
+ assertNotNull(reply)
+ assertEquals(1, reply.answers.size)
+ assertTrue(reply.answers[0] is MdnsTextRecord)
+ return reply.answers[0] as MdnsTextRecord
+ }
+
+ @Test
+ fun testAddService_AvoidEmptyTxtRecords_HasTxtRecordWithEmptyString() {
+ val answerRecord = doAddServiceWithEmptyTxtRecordTest(makeFlags())
+ assertEquals(1, answerRecord.entries.size)
+ assertEquals(0, answerRecord.entries[0].key.length)
+ assertNull(answerRecord.entries[0].value)
+ }
+
+ @Test
+ fun testAddService_UsesEmptyTxtRecords_HasEmptyTxtRecord() {
+ val answerRecord = doAddServiceWithEmptyTxtRecordTest(makeFlags(
+ avoidAdvertisingEmptyTxtRecords = false
+ ))
+ assertEquals(0, answerRecord.entries.size)
+ }
+
@Test
fun testRestartProbingForHostname() {
val repository = MdnsRecordRepository(thread.looper, deps, TEST_HOSTNAME, makeFlags())
diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordTests.java b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordTests.java
index 63548c1..784c502 100644
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordTests.java
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsRecordTests.java
@@ -28,6 +28,8 @@
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
+import static java.util.Collections.emptyList;
+
import android.util.Log;
import com.android.net.module.util.HexDump;
@@ -372,6 +374,30 @@
assertEquals(dataInText, dataOutText);
}
+ private static MdnsTextRecord makeTextRecordWithEntries(List<TextEntry> entries) {
+ return new MdnsTextRecord(new String[] { "test", "record" }, 0L /* receiptTimeMillis */,
+ true /* cacheFlush */, 120_000L /* ttlMillis */, entries);
+ }
+
+ @Test
+ public void testTextRecord_EmptyRecordsAreEquivalent() {
+ final MdnsTextRecord record1 = makeTextRecordWithEntries(emptyList());
+ final MdnsTextRecord record2 = makeTextRecordWithEntries(
+ List.of(new TextEntry("", (byte[]) null)));
+ final MdnsTextRecord record3 = makeTextRecordWithEntries(
+ List.of(new TextEntry(null, (byte[]) null)));
+ final MdnsTextRecord nonEmptyRecord = makeTextRecordWithEntries(
+ List.of(new TextEntry("a", (byte[]) null)));
+
+ assertEquals(record1, record1);
+ assertEquals(record1, record2);
+ assertEquals(record1, record3);
+
+ assertNotEquals(nonEmptyRecord, record1);
+ assertNotEquals(nonEmptyRecord, record2);
+ assertNotEquals(nonEmptyRecord, record3);
+ }
+
private static String toHex(MdnsRecord record) throws IOException {
MdnsPacketWriter writer = new MdnsPacketWriter(MAX_PACKET_SIZE);
record.write(writer, record.getReceiptTime());