[automerge] Avoid going over max service name length when renaming NSD services 2p: 34b5eeae9f

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/23023746 Bug: 265865456 Change-Id: I2bb115cb4712e6aaf0a082785ec272061dc68cf0 Merged-In: I077d8abdb91071db62b9618d9918e3a12682aaf4
2023-05-08 08:22:43 +00:00
parent b7a509ee94 34b5eeae9f
commit 5572568b81
6 changed files with 83 additions and 25 deletions
--- a/service-t/src/com/android/server/NsdService.java
+++ b/service-t/src/com/android/server/NsdService.java
@@ -23,6 +23,7 @@ import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;
 import static android.provider.DeviceConfig.NAMESPACE_TETHERING;

 import static com.android.modules.utils.build.SdkLevel.isAtLeastU;
+import static com.android.server.connectivity.mdns.MdnsRecord.MAX_LABEL_LENGTH;

 import android.annotation.NonNull;
 import android.annotation.Nullable;
@@ -73,6 +74,7 @@ import com.android.server.connectivity.mdns.MdnsServiceBrowserListener;
 import com.android.server.connectivity.mdns.MdnsServiceInfo;
 import com.android.server.connectivity.mdns.MdnsSocketClientBase;
 import com.android.server.connectivity.mdns.MdnsSocketProvider;
+import com.android.server.connectivity.mdns.util.MdnsUtils;

 import java.io.FileDescriptor;
 import java.io.PrintWriter;
@@ -81,11 +83,6 @@ import java.net.InetAddress;
 import java.net.NetworkInterface;
 import java.net.SocketException;
 import java.net.UnknownHostException;
-import java.nio.ByteBuffer;
-import java.nio.CharBuffer;
-import java.nio.charset.Charset;
-import java.nio.charset.CharsetEncoder;
-import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -108,8 +105,6 @@ public class NsdService extends INsdManager.Stub {
     */
    private static final String MDNS_DISCOVERY_MANAGER_VERSION = "mdns_discovery_manager_version";
    private static final String LOCAL_DOMAIN_NAME = "local";
-    // Max label length as per RFC 1034/1035
-    private static final int MAX_LABEL_LENGTH = 63;

    /**
     * Enable advertising using the Java MdnsAdvertiser, instead of the legacy mdnsresponder
@@ -570,18 +565,7 @@ public class NsdService extends INsdManager.Stub {
             */
            @NonNull
            private String truncateServiceName(@NonNull String originalName) {
-                // UTF-8 is at most 4 bytes per character; return early in the common case where
-                // the name can't possibly be over the limit given its string length.
-                if (originalName.length() <= MAX_LABEL_LENGTH / 4) return originalName;
-
-                final Charset utf8 = StandardCharsets.UTF_8;
-                final CharsetEncoder encoder = utf8.newEncoder();
-                final ByteBuffer out = ByteBuffer.allocate(MAX_LABEL_LENGTH);
-                // encode will write as many characters as possible to the out buffer, and just
-                // return an overflow code if there were too many characters (no need to check the
-                // return code here, this method truncates the name on purpose).
-                encoder.encode(CharBuffer.wrap(originalName), out, true /* endOfInput */);
-                return new String(out.array(), 0, out.position(), utf8);
+                return MdnsUtils.truncateServiceName(originalName, MAX_LABEL_LENGTH);
            }

            private void stopDiscoveryManagerRequest(ClientRequest request, int clientId, int id,
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsAdvertiser.java
@@ -16,6 +16,8 @@

 package com.android.server.connectivity.mdns;

+import static com.android.server.connectivity.mdns.MdnsRecord.MAX_LABEL_LENGTH;
+
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.net.LinkAddress;
@@ -29,6 +31,7 @@ import android.util.SparseArray;

 import com.android.internal.annotations.VisibleForTesting;
 import com.android.net.module.util.SharedLog;
+import com.android.server.connectivity.mdns.util.MdnsUtils;

 import java.io.PrintWriter;
 import java.util.List;
@@ -359,7 +362,7 @@ public class MdnsAdvertiser {
            // "Name (2)", then "Name (3)" etc.
            // TODO: use a hidden method in NsdServiceInfo once MdnsAdvertiser is moved to service-t
            final NsdServiceInfo newInfo = new NsdServiceInfo();
-            newInfo.setServiceName(mOriginalName + " (" + (mConflictCount + renameCount + 1) + ")");
+            newInfo.setServiceName(getUpdatedServiceName(renameCount));
            newInfo.setServiceType(mServiceInfo.getServiceType());
            for (Map.Entry<String, byte[]> attr : mServiceInfo.getAttributes().entrySet()) {
                newInfo.setAttribute(attr.getKey(),
@@ -372,6 +375,13 @@ public class MdnsAdvertiser {
            return newInfo;
        }

+        private String getUpdatedServiceName(int renameCount) {
+            final String suffix = " (" + (mConflictCount + renameCount + 1) + ")";
+            final String truncatedServiceName = MdnsUtils.truncateServiceName(mOriginalName,
+                    MAX_LABEL_LENGTH - suffix.length());
+            return truncatedServiceName + suffix;
+        }
+
        @NonNull
        public NsdServiceInfo getServiceInfo() {
            return mServiceInfo;
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsRecord.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsRecord.java
@@ -46,6 +46,8 @@ public abstract class MdnsRecord {

    public static final long RECEIPT_TIME_NOT_SENT = 0L;
    public static final int CLASS_ANY = 0x00ff;
+    /** Max label length as per RFC 1034/1035 */
+    public static final int MAX_LABEL_LENGTH = 63;

    /** Status indicating that the record is current. */
    public static final int STATUS_OK = 0;
--- a/service-t/src/com/android/server/connectivity/mdns/util/MdnsUtils.java
+++ b/service-t/src/com/android/server/connectivity/mdns/util/MdnsUtils.java
@@ -21,6 +21,12 @@ import android.annotation.Nullable;
 import android.net.Network;
 import android.os.Handler;

+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.Charset;
+import java.nio.charset.CharsetEncoder;
+import java.nio.charset.StandardCharsets;
+
 /**
 * Mdns utility functions.
 */
@@ -73,4 +79,22 @@ public class MdnsUtils {
            @Nullable Network currentNetwork) {
        return targetNetwork == null || targetNetwork.equals(currentNetwork);
    }
-}
+
+    /**
+     * Truncate a service name to up to maxLength UTF-8 bytes.
+     */
+    public static String truncateServiceName(@NonNull String originalName, int maxLength) {
+        // UTF-8 is at most 4 bytes per character; return early in the common case where
+        // the name can't possibly be over the limit given its string length.
+        if (originalName.length() <= maxLength / 4) return originalName;
+
+        final Charset utf8 = StandardCharsets.UTF_8;
+        final CharsetEncoder encoder = utf8.newEncoder();
+        final ByteBuffer out = ByteBuffer.allocate(maxLength);
+        // encode will write as many characters as possible to the out buffer, and just
+        // return an overflow code if there were too many characters (no need to check the
+        // return code here, this method truncates the name on purpose).
+        encoder.encode(CharBuffer.wrap(originalName), out, true /* endOfInput */);
+        return new String(out.array(), 0, out.position(), utf8);
+    }
+}
--- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsAdvertiserTest.kt
@@ -47,6 +47,8 @@ import org.mockito.Mockito.verify

 private const val SERVICE_ID_1 = 1
 private const val SERVICE_ID_2 = 2
+private const val LONG_SERVICE_ID_1 = 3
+private const val LONG_SERVICE_ID_2 = 4
 private const val TIMEOUT_MS = 10_000L
 private val TEST_ADDR = parseNumericAddress("2001:db8::123")
 private val TEST_LINKADDR = LinkAddress(TEST_ADDR, 64 /* prefixLength */)
@@ -56,16 +58,30 @@ private val TEST_HOSTNAME = arrayOf("Android_test", "local")

 private val SERVICE_1 = NsdServiceInfo("TestServiceName", "_advertisertest._tcp").apply {
    port = 12345
-    host = TEST_ADDR
+    hostAddresses = listOf(TEST_ADDR)
    network = TEST_NETWORK_1
 }

+private val LONG_SERVICE_1 =
+    NsdServiceInfo("a".repeat(48) + "TestServiceName", "_longadvertisertest._tcp").apply {
+    port = 12345
+    hostAddresses = listOf(TEST_ADDR)
+    network = TEST_NETWORK_1
+    }
+
 private val ALL_NETWORKS_SERVICE = NsdServiceInfo("TestServiceName", "_advertisertest._tcp").apply {
    port = 12345
-    host = TEST_ADDR
+    hostAddresses = listOf(TEST_ADDR)
    network = null
 }

+private val LONG_ALL_NETWORKS_SERVICE =
+    NsdServiceInfo("a".repeat(48) + "TestServiceName", "_longadvertisertest._tcp").apply {
+        port = 12345
+        hostAddresses = listOf(TEST_ADDR)
+        network = null
+    }
+
@RunWith(DevSdkIgnoreRunner::class)
@IgnoreUpTo(Build.VERSION_CODES.S_V2)
 class MdnsAdvertiserTest {
@@ -191,6 +207,9 @@ class MdnsAdvertiserTest {
        verify(socketProvider).requestSocket(eq(null), allNetSocketCbCaptor.capture())
        val allNetSocketCb = allNetSocketCbCaptor.value

+        postSync { advertiser.addService(LONG_SERVICE_ID_1, LONG_SERVICE_1) }
+        postSync { advertiser.addService(LONG_SERVICE_ID_2, LONG_ALL_NETWORKS_SERVICE) }
+
        // Callbacks for matching network and all networks both get the socket
        postSync {
            oneNetSocketCb.onSocketCreated(TEST_NETWORK_1, mockSocket1, listOf(TEST_LINKADDR))
@@ -200,10 +219,18 @@ class MdnsAdvertiserTest {
        val expectedRenamed = NsdServiceInfo(
                "${ALL_NETWORKS_SERVICE.serviceName} (2)", ALL_NETWORKS_SERVICE.serviceType).apply {
            port = ALL_NETWORKS_SERVICE.port
-            host = ALL_NETWORKS_SERVICE.host
+            hostAddresses = ALL_NETWORKS_SERVICE.hostAddresses
            network = ALL_NETWORKS_SERVICE.network
        }

+        val expectedLongRenamed = NsdServiceInfo(
+            "${LONG_ALL_NETWORKS_SERVICE.serviceName.dropLast(4)} (2)",
+            LONG_ALL_NETWORKS_SERVICE.serviceType).apply {
+            port = LONG_ALL_NETWORKS_SERVICE.port
+            hostAddresses = LONG_ALL_NETWORKS_SERVICE.hostAddresses
+            network = LONG_ALL_NETWORKS_SERVICE.network
+        }
+
        val intAdvCbCaptor = ArgumentCaptor.forClass(MdnsInterfaceAdvertiser.Callback::class.java)
        verify(mockDeps).makeAdvertiser(eq(mockSocket1), eq(listOf(TEST_LINKADDR)),
                eq(thread.looper), any(), intAdvCbCaptor.capture(), eq(TEST_HOSTNAME)
@@ -212,6 +239,10 @@ class MdnsAdvertiserTest {
                argThat { it.matches(SERVICE_1) })
        verify(mockInterfaceAdvertiser1).addService(eq(SERVICE_ID_2),
                argThat { it.matches(expectedRenamed) })
+        verify(mockInterfaceAdvertiser1).addService(eq(LONG_SERVICE_ID_1),
+                argThat { it.matches(LONG_SERVICE_1) })
+        verify(mockInterfaceAdvertiser1).addService(eq(LONG_SERVICE_ID_2),
+            argThat { it.matches(expectedLongRenamed) })

        doReturn(false).`when`(mockInterfaceAdvertiser1).isProbing(SERVICE_ID_1)
        postSync { intAdvCbCaptor.value.onRegisterServiceSucceeded(
@@ -251,7 +282,7 @@ private fun NsdServiceInfo.matches(other: NsdServiceInfo): Boolean {
    return Objects.equals(serviceName, other.serviceName) &&
            Objects.equals(serviceType, other.serviceType) &&
            Objects.equals(attributes, other.attributes) &&
-            Objects.equals(host, other.host) &&
+            Objects.equals(hostAddresses, other.hostAddresses) &&
            port == other.port &&
            Objects.equals(network, other.network)
 }
--- a/tests/unit/java/com/android/server/connectivity/mdns/util/MdnsUtilsTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/mdns/util/MdnsUtilsTest.kt
@@ -19,6 +19,7 @@ package com.android.server.connectivity.mdns.util
 import android.os.Build
 import com.android.server.connectivity.mdns.util.MdnsUtils.equalsIgnoreDnsCase
 import com.android.server.connectivity.mdns.util.MdnsUtils.toDnsLowerCase
+import com.android.server.connectivity.mdns.util.MdnsUtils.truncateServiceName
 import com.android.testutils.DevSdkIgnoreRule
 import com.android.testutils.DevSdkIgnoreRunner
 import org.junit.Assert.assertEquals
@@ -65,4 +66,10 @@ class MdnsUtilsTest {
                "Test: >\ud83c\udff4\udb40\udc67\udb40\udc62\udb40" +
                        "\udc77\udb40\udc6c\udb40\udc73\udb40\udc7f<"))
    }
+
+    @Test
+    fun testTruncateServiceName() {
+        assertEquals(truncateServiceName("测试abcde", 7), "测试a")
+        assertEquals(truncateServiceName("测试abcde", 100), "测试abcde")
+    }
 }