From 8f8c78d56f843fc054ee0555a01084b456fe5fe9 Mon Sep 17 00:00:00 2001 From: Handa Wang Date: Thu, 22 May 2025 21:38:55 -0700 Subject: [PATCH] Truncate the additional records if the packet is too large When the packet is too large to be sent it may cause the socket to throw an error. This commit truncates the additional records in this case. Bug: 407609792 Test: CI Flag: EXEMPT bugfix Change-Id: I77a979d7f8799f05cbcb14857061a18aecfef657 --- .../connectivity/mdns/MdnsPacketWriter.java | 5 +- .../connectivity/mdns/MdnsReplySender.java | 37 ++++++++++- .../connectivity/mdns/MdnsReplySenderTest.kt | 64 +++++++++++++++++++ 3 files changed, 103 insertions(+), 3 deletions(-) diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsPacketWriter.java b/service-t/src/com/android/server/connectivity/mdns/MdnsPacketWriter.java index cf788be0d1..1d227adc8d 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsPacketWriter.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsPacketWriter.java @@ -19,6 +19,7 @@ package com.android.server.connectivity.mdns; import com.android.net.module.util.DnsUtils; import com.android.server.connectivity.mdns.MdnsServiceInfo.TextEntry; +import java.io.EOFException; import java.io.IOException; import java.net.DatagramPacket; import java.net.SocketAddress; @@ -241,9 +242,9 @@ public class MdnsPacketWriter { } // Checks if the remaining space in the packet is at least |count|. - private void checkRemaining(int count) throws IOException { + private void checkRemaining(int count) throws EOFException { if (getRemaining() < count) { - throw new IOException(); + throw new EOFException(); } } diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java b/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java index 4708cb62ac..f41d6be8d2 100644 --- a/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java +++ b/service-t/src/com/android/server/connectivity/mdns/MdnsReplySender.java @@ -33,6 +33,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.net.module.util.SharedLog; import com.android.server.connectivity.mdns.util.MdnsUtils; +import java.io.EOFException; import java.io.IOException; import java.net.DatagramPacket; import java.net.Inet4Address; @@ -100,6 +101,14 @@ public class MdnsReplySender { public void removeMessages(@NonNull Handler handler, int what, @NonNull Object object) { handler.removeMessages(what, object); } + + /** + * @see MdnsUtils#createRawDnsPacket(byte[], MdnsPacket) + */ + public byte[] createRawDnsPacket(@NonNull byte[] buffer, @NonNull MdnsPacket packet) + throws IOException { + return MdnsUtils.createRawDnsPacket(buffer, packet); + } } public MdnsReplySender(@NonNull Looper looper, @NonNull MdnsInterfaceSocket socket, @@ -210,11 +219,37 @@ public class MdnsReplySender { // Skip sending if the socket has not joined the v4/v6 group (there was no address) return PACKET_NOT_SENT; } - final byte[] outBuffer = MdnsUtils.createRawDnsPacket(mPacketCreationBuffer, packet); + final byte[] outBuffer = createRawDnsPacket(packet); mSocket.send(new DatagramPacket(outBuffer, 0, outBuffer.length, destination)); return PACKET_SENT; } + private byte[] createRawDnsPacket(MdnsPacket packet) + throws IOException { + try { + return mDependencies.createRawDnsPacket(mPacketCreationBuffer, packet); + } catch (EOFException e) { + // Try truncating the additional records if the packet is too large. + final MdnsPacket packetWithoutAdditionalRecords = + new MdnsPacket( + packet.transactionId, + packet.flags, + packet.questions, + packet.answers, + packet.authorityRecords, + Collections.emptyList() /* additionalRecords */); + final byte[] outBuffer = + mDependencies.createRawDnsPacket( + mPacketCreationBuffer, packetWithoutAdditionalRecords); + // TODO: The packet may still be too large after truncating the additional records. We + // may need to further optimize this. + mSharedLog.e( + "The message is too large to fit in a single packet. The additional records are" + + " not included."); + return outBuffer; + } + } + /** * Cancel all pending sends. */ diff --git a/tests/unit/java/com/android/server/connectivity/mdns/MdnsReplySenderTest.kt b/tests/unit/java/com/android/server/connectivity/mdns/MdnsReplySenderTest.kt index 9bd0530108..53e5e28eb2 100644 --- a/tests/unit/java/com/android/server/connectivity/mdns/MdnsReplySenderTest.kt +++ b/tests/unit/java/com/android/server/connectivity/mdns/MdnsReplySenderTest.kt @@ -28,11 +28,13 @@ import com.android.server.connectivity.mdns.MdnsConstants.IPV6_SOCKET_ADDR import com.android.server.connectivity.mdns.MdnsReplySender.getReplyDestination import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo import com.android.testutils.DevSdkIgnoreRunner +import java.io.IOException import java.net.DatagramPacket import java.net.InetSocketAddress import java.util.concurrent.CompletableFuture import java.util.concurrent.TimeUnit import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import org.junit.After import org.junit.Before import org.junit.Test @@ -41,9 +43,12 @@ import org.mockito.ArgumentCaptor import org.mockito.Mockito.any import org.mockito.Mockito.anyLong import org.mockito.Mockito.argThat +import org.mockito.Mockito.doCallRealMethod import org.mockito.Mockito.doReturn +import org.mockito.Mockito.doThrow import org.mockito.Mockito.eq import org.mockito.Mockito.mock +import org.mockito.Mockito.never import org.mockito.Mockito.timeout import org.mockito.Mockito.times import org.mockito.Mockito.verify @@ -107,6 +112,23 @@ class MdnsReplySenderTest { MdnsNsecRecord(otherHostname, 0L /* receiptTimeMillis */, true /* cacheFlush */, SHORT_TTL, otherHostname /* nextDomain */, intArrayOf(MdnsRecord.TYPE_A, MdnsRecord.TYPE_AAAA))) + private val manyServiceNames = (1..30).map { + arrayOf("MyTestService", "_testservice", "_tcp", "local") + } + private val manyAnswers = manyServiceNames.map { + MdnsPointerRecord(it, 0L /* receiptTimeMillis */, false /* cacheFlush */, LONG_TTL, it) + } + private val manyAdditionalAnswers = manyServiceNames.flatMap { + listOf( + MdnsTextRecord(it, 0L /* receiptTimeMillis */, true /* cacheFlush */, LONG_TTL, + emptyList()), + MdnsServiceRecord(it, 0L /* receiptTimeMillis */, true /* cacheFlush */, SHORT_TTL, + 0 /* servicePriority */, 0 /* serviceWeight */, TEST_PORT, it), + MdnsNsecRecord(it, 0L /* receiptTimeMillis */, true /* cacheFlush */, LONG_TTL, + it /* nextDomain */, + intArrayOf(MdnsRecord.TYPE_TXT, MdnsRecord.TYPE_SRV)) + ) + } private val thread = HandlerThread(MdnsReplySenderTest::class.simpleName) private val socket = mock(MdnsInterfaceSocket::class.java) private val buffer = ByteArray(1500) @@ -119,6 +141,7 @@ class MdnsReplySenderTest { thread.start() doReturn(true).`when`(socket).hasJoinedIpv4() doReturn(true).`when`(socket).hasJoinedIpv6() + doCallRealMethod().`when`(deps).createRawDnsPacket(any(), any()) } @After @@ -162,6 +185,47 @@ class MdnsReplySenderTest { verify(socket).send(argThat{ it.socketAddress.equals(IPV4_SOCKET_ADDR) }) } + @Test + fun testSendNow_CreateRawDnsPacketThrowsGeneralIOException_PacketIsNotSent() { + val replySender = createSender(enableKAS = false) + val packet = MdnsPacket(0x8400, + emptyList() /* questions */, + answers, + emptyList() /* authorityRecords */, + additionalAnswers) + doThrow(IOException()).`when`(deps).createRawDnsPacket(any(), any()) + val future = CompletableFuture() + handler.post { + assertFailsWith { + replySender.sendNow(packet, IPV4_SOCKET_ADDR) + } + future.complete(null) + } + future.get(DEFAULT_TIMEOUT_MS, TimeUnit.MILLISECONDS) + verify(socket, never()).send(any()) + } + + @Test + fun testSendNow_PacketIsTooLarge_AdditionalRecordsAreTruncated() { + val replySender = createSender(enableKAS = false) + // The datagram is 2059 bytes which exceeds the 1500 byte limit. + val packet = MdnsPacket(0x8400, + emptyList() /* questions */, + manyAnswers, + emptyList() /* authorityRecords */, + manyAdditionalAnswers) + sendNow(replySender, packet, IPV4_SOCKET_ADDR) + verify(socket).send(argThat{ + val packet = MdnsPacket.parse(MdnsPacketReader(it)) + + packet.questions.isEmpty() && + packet.answers.equals(manyAnswers) && + packet.authorityRecords.isEmpty() && + packet.additionalRecords.isEmpty() && + it.socketAddress.equals(IPV4_SOCKET_ADDR) + }) + } + private fun verifyMessageQueued( sender: MdnsReplySender, replies: List