From 33bfef3b54008c6715acf8326885e20443acf6f1 Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Wed, 23 Jan 2019 21:53:13 +0800 Subject: [PATCH] Minor changes to the async DNS query JAVA API 1. refine the naming in DnsPacket and add more comment 2. add comment in DnsResolver Test: built, flashed, booted atest DnsResolverTest DnsPacketTest Change-Id: Ib482d079d6823fd1d9bff163427b7aad38374199 --- core/java/android/net/DnsPacket.java | 77 +++++++++++-------- core/java/android/net/DnsResolver.java | 13 ++-- tests/net/java/android/net/DnsPacketTest.java | 60 +++++++-------- 3 files changed, 81 insertions(+), 69 deletions(-) diff --git a/core/java/android/net/DnsPacket.java b/core/java/android/net/DnsPacket.java index 458fb340b196f..0ac02b1b7b37f 100644 --- a/core/java/android/net/DnsPacket.java +++ b/core/java/android/net/DnsPacket.java @@ -28,7 +28,6 @@ import java.text.DecimalFormat; import java.text.FieldPosition; import java.util.ArrayList; import java.util.List; -import java.util.StringJoiner; /** * Defines basic data for DNS protocol based on RFC 1035. @@ -42,7 +41,7 @@ public abstract class DnsPacket { public final int id; public final int flags; public final int rcode; - private final int[] mSectionCount; + private final int[] mRecordCount; /** * Create a new DnsHeader from a positioned ByteBuffer. @@ -52,27 +51,32 @@ public abstract class DnsPacket { * When this constructor returns, the reading position of the ByteBuffer has been * advanced to the end of the DNS header record. * This is meant to chain with other methods reading a DNS response in sequence. - * */ DnsHeader(@NonNull ByteBuffer buf) throws BufferUnderflowException { id = BitUtils.uint16(buf.getShort()); flags = BitUtils.uint16(buf.getShort()); rcode = flags & 0xF; - mSectionCount = new int[NUM_SECTIONS]; + mRecordCount = new int[NUM_SECTIONS]; for (int i = 0; i < NUM_SECTIONS; ++i) { - mSectionCount[i] = BitUtils.uint16(buf.getShort()); + mRecordCount[i] = BitUtils.uint16(buf.getShort()); } } /** - * Get section count by section type. + * Get record count by type. */ - public int getSectionCount(int sectionType) { - return mSectionCount[sectionType]; + public int getRecordCount(int type) { + return mRecordCount[type]; } } - public class DnsSection { + /** + * It's used both for DNS questions and DNS resource records. + * + * DNS questions (No TTL/RDATA) + * DNS resource records (With TTL/RDATA) + */ + public class DnsRecord { private static final int MAXNAMESIZE = 255; private static final int MAXLABELSIZE = 63; private static final int MAXLABELCOUNT = 128; @@ -81,57 +85,57 @@ public abstract class DnsPacket { private final DecimalFormat byteFormat = new DecimalFormat(); private final FieldPosition pos = new FieldPosition(0); - private static final String TAG = "DnsSection"; + private static final String TAG = "DnsRecord"; public final String dName; public final int nsType; public final int nsClass; public final long ttl; - private final byte[] mRR; + private final byte[] mRdata; /** - * Create a new DnsSection from a positioned ByteBuffer. + * Create a new DnsRecord from a positioned ByteBuffer. * - * The ByteBuffer must be in network byte order (which is the default). - * Reads the passed ByteBuffer from its current position and decodes a DNS section. + * @param ByteBuffer input of record, must be in network byte order + * (which is the default). + * Reads the passed ByteBuffer from its current position and decodes a DNS record. * When this constructor returns, the reading position of the ByteBuffer has been * advanced to the end of the DNS header record. * This is meant to chain with other methods reading a DNS response in sequence. - * */ - DnsSection(int sectionType, @NonNull ByteBuffer buf) + DnsRecord(int recordType, @NonNull ByteBuffer buf) throws BufferUnderflowException, ParseException { dName = parseName(buf, 0 /* Parse depth */); if (dName.length() > MAXNAMESIZE) { - throw new ParseException("Parse name fail, name size is too long"); + throw new ParseException( + "Parse name fail, name size is too long: " + dName.length()); } nsType = BitUtils.uint16(buf.getShort()); nsClass = BitUtils.uint16(buf.getShort()); - if (sectionType != QDSECTION) { + if (recordType != QDSECTION) { ttl = BitUtils.uint32(buf.getInt()); final int length = BitUtils.uint16(buf.getShort()); - mRR = new byte[length]; - buf.get(mRR); + mRdata = new byte[length]; + buf.get(mRdata); } else { ttl = 0; - mRR = null; + mRdata = null; } } /** - * Get a copy of rr. + * Get a copy of rdata. */ - @Nullable public byte[] getRR() { - return (mRR == null) ? null : mRR.clone(); + @Nullable + public byte[] getRR() { + return (mRdata == null) ? null : mRdata.clone(); } /** * Convert label from {@code byte[]} to {@code String} * - * It follows the same converting rule as native layer. - * (See ns_name.c in libc) - * + * Follows the same conversion rules of the native code (ns_name.c in libc) */ private String labelToString(@NonNull byte[] label) { final StringBuffer sb = new StringBuffer(); @@ -139,13 +143,16 @@ public abstract class DnsPacket { int b = BitUtils.uint8(label[i]); // Control characters and non-ASCII characters. if (b <= 0x20 || b >= 0x7f) { + // Append the byte as an escaped decimal number, e.g., "\19" for 0x13. sb.append('\\'); byteFormat.format(b, sb, pos); } else if (b == '"' || b == '.' || b == ';' || b == '\\' || b == '(' || b == ')' || b == '@' || b == '$') { + // Append the byte as an escaped character, e.g., "\:" for 0x3a. sb.append('\\'); sb.append((char) b); } else { + // Append the byte as a character, e.g., "a" for 0x61. sb.append((char) b); } } @@ -154,7 +161,9 @@ public abstract class DnsPacket { private String parseName(@NonNull ByteBuffer buf, int depth) throws BufferUnderflowException, ParseException { - if (depth > MAXLABELCOUNT) throw new ParseException("Parse name fails, too many labels"); + if (depth > MAXLABELCOUNT) { + throw new ParseException("Failed to parse name, too many labels"); + } final int len = BitUtils.uint8(buf.get()); final int mask = len & NAME_COMPRESSION; if (0 == len) { @@ -194,7 +203,7 @@ public abstract class DnsPacket { private static final String TAG = DnsPacket.class.getSimpleName(); protected final DnsHeader mHeader; - protected final List[] mSections; + protected final List[] mRecords; public static class ParseException extends Exception { public ParseException(String msg) { @@ -216,18 +225,18 @@ public abstract class DnsPacket { throw new ParseException("Parse Header fail, bad input data", e); } - mSections = new ArrayList[NUM_SECTIONS]; + mRecords = new ArrayList[NUM_SECTIONS]; for (int i = 0; i < NUM_SECTIONS; ++i) { - final int count = mHeader.getSectionCount(i); + final int count = mHeader.getRecordCount(i); if (count > 0) { - mSections[i] = new ArrayList(count); + mRecords[i] = new ArrayList(count); } for (int j = 0; j < count; ++j) { try { - mSections[i].add(new DnsSection(i, buffer)); + mRecords[i].add(new DnsRecord(i, buffer)); } catch (BufferUnderflowException e) { - throw new ParseException("Parse section fail", e); + throw new ParseException("Parse record fail", e); } } } diff --git a/core/java/android/net/DnsResolver.java b/core/java/android/net/DnsResolver.java index 6d54264cd89f4..d3bc3e66fbeee 100644 --- a/core/java/android/net/DnsResolver.java +++ b/core/java/android/net/DnsResolver.java @@ -43,6 +43,9 @@ import java.util.function.Consumer; /** * Dns resolver class for asynchronous dns querying * + * Note that if a client sends a query with more than 1 record in the question section but + * the remote dns server does not support this, it may not respond at all, leading to a timeout. + * */ public final class DnsResolver { private static final String TAG = "DnsResolver"; @@ -226,19 +229,19 @@ public final class DnsResolver { if (mHeader.rcode != 0) { throw new ParseException("Response error, rcode:" + mHeader.rcode); } - if (mHeader.getSectionCount(ANSECTION) == 0) { + if (mHeader.getRecordCount(ANSECTION) == 0) { throw new ParseException("No available answer"); } - if (mHeader.getSectionCount(QDSECTION) == 0) { + if (mHeader.getRecordCount(QDSECTION) == 0) { throw new ParseException("No question found"); } - // Assume only one question per answer packet. (RFC1035) - mQueryType = mSections[QDSECTION].get(0).nsType; + // Expect only one question in question section. + mQueryType = mRecords[QDSECTION].get(0).nsType; } public @NonNull List getAddresses() { final List results = new ArrayList(); - for (final DnsSection ansSec : mSections[ANSECTION]) { + for (final DnsRecord ansSec : mRecords[ANSECTION]) { // Only support A and AAAA, also ignore answers if query type != answer type. int nsType = ansSec.nsType; if (nsType != mQueryType || (nsType != TYPE_A && nsType != TYPE_AAAA)) { diff --git a/tests/net/java/android/net/DnsPacketTest.java b/tests/net/java/android/net/DnsPacketTest.java index 91ff6b3a7e063..9ede2b85af008 100644 --- a/tests/net/java/android/net/DnsPacketTest.java +++ b/tests/net/java/android/net/DnsPacketTest.java @@ -36,19 +36,19 @@ public class DnsPacketTest { int qCount, int aCount, int nsCount, int arCount) { assertEquals(header.id, id); assertEquals(header.flags, flag); - assertEquals(header.getSectionCount(DnsPacket.QDSECTION), qCount); - assertEquals(header.getSectionCount(DnsPacket.ANSECTION), aCount); - assertEquals(header.getSectionCount(DnsPacket.NSSECTION), nsCount); - assertEquals(header.getSectionCount(DnsPacket.ARSECTION), arCount); + assertEquals(header.getRecordCount(DnsPacket.QDSECTION), qCount); + assertEquals(header.getRecordCount(DnsPacket.ANSECTION), aCount); + assertEquals(header.getRecordCount(DnsPacket.NSSECTION), nsCount); + assertEquals(header.getRecordCount(DnsPacket.ARSECTION), arCount); } - private void assertSectionParses(DnsPacket.DnsSection section, String dname, + private void assertRecordParses(DnsPacket.DnsRecord record, String dname, int dtype, int dclass, int ttl, byte[] rr) { - assertEquals(section.dName, dname); - assertEquals(section.nsType, dtype); - assertEquals(section.nsClass, dclass); - assertEquals(section.ttl, ttl); - assertTrue(Arrays.equals(section.getRR(), rr)); + assertEquals(record.dName, dname); + assertEquals(record.nsType, dtype); + assertEquals(record.nsClass, dclass); + assertEquals(record.ttl, ttl); + assertTrue(Arrays.equals(record.getRR(), rr)); } class TestDnsPacket extends DnsPacket { @@ -59,8 +59,8 @@ public class DnsPacketTest { public DnsHeader getHeader() { return mHeader; } - public List getSectionList(int secType) { - return mSections[secType]; + public List getRecordList(int secType) { + return mRecords[secType]; } } @@ -101,16 +101,16 @@ public class DnsPacketTest { // Header part assertHeaderParses(packet.getHeader(), 0x5566, 0x8180, 1, 1, 0, 0); - // Section part - List qdSectionList = - packet.getSectionList(DnsPacket.QDSECTION); - assertEquals(qdSectionList.size(), 1); - assertSectionParses(qdSectionList.get(0), "www.google.com", 1, 1, 0, null); + // Record part + List qdRecordList = + packet.getRecordList(DnsPacket.QDSECTION); + assertEquals(qdRecordList.size(), 1); + assertRecordParses(qdRecordList.get(0), "www.google.com", 1, 1, 0, null); - List anSectionList = - packet.getSectionList(DnsPacket.ANSECTION); - assertEquals(anSectionList.size(), 1); - assertSectionParses(anSectionList.get(0), "www.google.com", 1, 1, 0x12b, + List anRecordList = + packet.getRecordList(DnsPacket.ANSECTION); + assertEquals(anRecordList.size(), 1); + assertRecordParses(anRecordList.get(0), "www.google.com", 1, 1, 0x12b, new byte[]{ (byte) 0xac, (byte) 0xd9, (byte) 0xa1, (byte) 0x84 }); } @@ -143,16 +143,16 @@ public class DnsPacketTest { // Header part assertHeaderParses(packet.getHeader(), 0x7722, 0x8180, 1, 1, 0, 0); - // Section part - List qdSectionList = - packet.getSectionList(DnsPacket.QDSECTION); - assertEquals(qdSectionList.size(), 1); - assertSectionParses(qdSectionList.get(0), "www.google.com", 28, 1, 0, null); + // Record part + List qdRecordList = + packet.getRecordList(DnsPacket.QDSECTION); + assertEquals(qdRecordList.size(), 1); + assertRecordParses(qdRecordList.get(0), "www.google.com", 28, 1, 0, null); - List anSectionList = - packet.getSectionList(DnsPacket.ANSECTION); - assertEquals(anSectionList.size(), 1); - assertSectionParses(anSectionList.get(0), "www.google.com", 28, 1, 0x37, + List anRecordList = + packet.getRecordList(DnsPacket.ANSECTION); + assertEquals(anRecordList.size(), 1); + assertRecordParses(anRecordList.get(0), "www.google.com", 28, 1, 0x37, new byte[]{ 0x24, 0x04, 0x68, 0x00, 0x40, 0x05, 0x08, 0x0d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x04 }); }