diff --git a/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoder.java b/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoder.java index 2f50b80115..c46c98a5d9 100644 --- a/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoder.java +++ b/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoder.java @@ -27,6 +27,8 @@ import io.netty.util.internal.StringUtil; */ public class DefaultDnsRecordDecoder implements DnsRecordDecoder { + static final String ROOT = "."; + /** * Creates a new instance. */ @@ -117,16 +119,22 @@ public class DefaultDnsRecordDecoder implements DnsRecordDecoder { // - https://github.com/netty/netty/issues/5014 // - https://www.ietf.org/rfc/rfc1035.txt , Section 3.1 if (readable == 0) { - return StringUtil.EMPTY_STRING; + return ROOT; } + final StringBuilder name = new StringBuilder(readable << 1); - for (int len = in.readUnsignedByte(); in.isReadable() && len != 0; len = in.readUnsignedByte()) { - boolean pointer = (len & 0xc0) == 0xc0; + while (in.isReadable()) { + final int len = in.readUnsignedByte(); + final boolean pointer = (len & 0xc0) == 0xc0; if (pointer) { if (position == -1) { position = in.readerIndex() + 1; } + if (!in.isReadable()) { + throw new CorruptedFrameException("truncated pointer in a name"); + } + final int next = (len & 0x3f) << 8 | in.readUnsignedByte(); if (next >= end) { throw new CorruptedFrameException("name has an out-of-range pointer"); @@ -138,18 +146,29 @@ public class DefaultDnsRecordDecoder implements DnsRecordDecoder { if (checked >= end) { throw new CorruptedFrameException("name contains a loop."); } - } else { + } else if (len != 0) { + if (!in.isReadable(len)) { + throw new CorruptedFrameException("truncated label in a name"); + } name.append(in.toString(in.readerIndex(), len, CharsetUtil.UTF_8)).append('.'); in.skipBytes(len); + } else { // len == 0 + break; } } + if (position != -1) { in.readerIndex(position); } + if (name.length() == 0) { - return StringUtil.EMPTY_STRING; + return ROOT; } - return name.substring(0, name.length() - 1); + if (name.charAt(name.length() - 1) != '.') { + name.append('.'); + } + + return name.toString(); } } diff --git a/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoder.java b/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoder.java index 4165272d1e..3222566e06 100644 --- a/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoder.java +++ b/codec-dns/src/main/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoder.java @@ -20,6 +20,8 @@ import io.netty.buffer.ByteBufUtil; import io.netty.handler.codec.UnsupportedMessageTypeException; import io.netty.util.internal.StringUtil; +import static io.netty.handler.codec.dns.DefaultDnsRecordDecoder.ROOT; + /** * The default {@link DnsRecordEncoder} implementation. * @@ -77,19 +79,24 @@ public class DefaultDnsRecordEncoder implements DnsRecordEncoder { } protected void encodeName(String name, ByteBuf buf) throws Exception { - String[] parts = StringUtil.split(name, '.'); - for (String part: parts) { - final int partLen = part.length(); - // We always need to write the length even if its 0. - // See: - // - https://github.com/netty/netty/issues/5014 - // - https://www.ietf.org/rfc/rfc1035.txt , Section 3.1 - buf.writeByte(partLen); - if (partLen == 0) { - continue; - } - ByteBufUtil.writeAscii(buf, part); + if (ROOT.equals(name)) { + // Root domain + buf.writeByte(0); + return; } + + final String[] labels = StringUtil.split(name, '.'); + for (String label : labels) { + final int labelLen = label.length(); + if (labelLen == 0) { + // zero-length label means the end of the name. + break; + } + + buf.writeByte(labelLen); + ByteBufUtil.writeAscii(buf, label); + } + buf.writeByte(0); // marks end of name field } } diff --git a/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoderTest.java b/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoderTest.java index a6ee9d7d1b..6004d713a1 100644 --- a/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoderTest.java +++ b/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordDecoderTest.java @@ -24,19 +24,46 @@ import org.junit.Test; public class DefaultDnsRecordDecoderTest { @Test - public void testDecodeEmptyName() { - testDecodeEmptyName0(Unpooled.buffer().writeByte('0')); + public void testDecodeName() { + testDecodeName("netty.io.", Unpooled.wrappedBuffer(new byte[] { + 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0 + })); } @Test - public void testDecodeEmptyNameNonRFC() { - testDecodeEmptyName0(Unpooled.EMPTY_BUFFER); + public void testDecodeNameWithoutTerminator() { + testDecodeName("netty.io.", Unpooled.wrappedBuffer(new byte[] { + 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o' + })); } - private static void testDecodeEmptyName0(ByteBuf buffer) { + @Test + public void testDecodeNameWithExtraTerminator() { + // Should not be decoded as 'netty.io..' + testDecodeName("netty.io.", Unpooled.wrappedBuffer(new byte[] { + 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0, 0 + })); + } + + @Test + public void testDecodeEmptyName() { + testDecodeName(".", Unpooled.buffer().writeByte(0)); + } + + @Test + public void testDecodeEmptyNameFromEmptyBuffer() { + testDecodeName(".", Unpooled.EMPTY_BUFFER); + } + + @Test + public void testDecodeEmptyNameFromExtraZeroes() { + testDecodeName(".", Unpooled.wrappedBuffer(new byte[] { 0, 0 })); + } + + private static void testDecodeName(String expected, ByteBuf buffer) { try { DefaultDnsRecordDecoder decoder = new DefaultDnsRecordDecoder(); - Assert.assertEquals(StringUtil.EMPTY_STRING, decoder.decodeName(buffer)); + Assert.assertEquals(expected, decoder.decodeName(buffer)); } finally { buffer.release(); } diff --git a/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoderTest.java b/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoderTest.java index c1511e0e10..08c0896d51 100644 --- a/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoderTest.java +++ b/codec-dns/src/test/java/io/netty/handler/codec/dns/DefaultDnsRecordEncoderTest.java @@ -16,6 +16,7 @@ package io.netty.handler.codec.dns; import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufUtil; import io.netty.buffer.Unpooled; import io.netty.util.internal.StringUtil; import org.junit.Test; @@ -24,18 +25,42 @@ import static org.junit.Assert.assertEquals; public class DefaultDnsRecordEncoderTest { + @Test + public void testEncodeName() throws Exception { + testEncodeName(new byte[] { 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0 }, "netty.io."); + } + + @Test + public void testEncodeNameWithoutTerminator() throws Exception { + testEncodeName(new byte[] { 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0 }, "netty.io"); + } + + @Test + public void testEncodeNameWithExtraTerminator() throws Exception { + testEncodeName(new byte[] { 5, 'n', 'e', 't', 't', 'y', 2, 'i', 'o', 0 }, "netty.io.."); + } + // Test for https://github.com/netty/netty/issues/5014 @Test public void testEncodeEmptyName() throws Exception { + testEncodeName(new byte[] { 0 }, StringUtil.EMPTY_STRING); + } + + @Test + public void testEncodeRootName() throws Exception { + testEncodeName(new byte[] { 0 }, "."); + } + + private static void testEncodeName(byte[] expected, String name) throws Exception { DefaultDnsRecordEncoder encoder = new DefaultDnsRecordEncoder(); ByteBuf out = Unpooled.buffer(); + ByteBuf expectedBuf = Unpooled.wrappedBuffer(expected); try { - encoder.encodeName(StringUtil.EMPTY_STRING, out); - assertEquals(2, out.readableBytes()); - assertEquals(0, out.readByte()); - assertEquals(0, out.readByte()); + encoder.encodeName(name, out); + assertEquals(expectedBuf, out); } finally { out.release(); + expectedBuf.release(); } } }