From 504fe0d19adce7a7837b5cb9d1e4616066cf64af Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Wed, 6 Jan 2021 15:46:08 -0500 Subject: [PATCH 01/11] Update SimpleResolver.java add conditional logic to support receiving either a byte array of one answer (existing unicast server behavior), or an ArrayList containing one or more byte arrays (multiple responses to multicast query from multiple mDNS servers). --- .../java/org/xbill/DNS/SimpleResolver.java | 218 +++++++++++++----- 1 file changed, 158 insertions(+), 60 deletions(-) diff --git a/src/main/java/org/xbill/DNS/SimpleResolver.java b/src/main/java/org/xbill/DNS/SimpleResolver.java index 6a71eef57..fd6dc08df 100644 --- a/src/main/java/org/xbill/DNS/SimpleResolver.java +++ b/src/main/java/org/xbill/DNS/SimpleResolver.java @@ -5,8 +5,10 @@ import java.io.IOException; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.SocketTimeoutException; import java.net.UnknownHostException; import java.time.Duration; +import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.concurrent.CompletableFuture; @@ -339,7 +341,7 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { address.getPort()); log.trace("Query:\n{}", query); - CompletableFuture result; + CompletableFuture result; if (tcp) { result = NioTcpClient.sendrecv(localAddress, address, query, out, timeoutValue); } else { @@ -347,74 +349,170 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { } return result.thenComposeAsync( - in -> { + v -> { CompletableFuture f = new CompletableFuture<>(); - // Check that the response is long enough. - if (in.length < Header.LENGTH) { - f.completeExceptionally(new WireParseException("invalid DNS header - too short")); - return f; - } + if (v instanceof byte[]) { + byte[] in = (byte[])v; + // Check that the response is long enough. + if (in.length < Header.LENGTH) { + f.completeExceptionally(new WireParseException("invalid DNS header - too short")); + return f; + } + + // Check that the response ID matches the query ID. We want + // to check this before actually parsing the message, so that + // if there's a malformed response that's not ours, it + // doesn't confuse us. + int id = ((in[0] & 0xFF) << 8) + (in[1] & 0xFF); + if (id != qid) { + f.completeExceptionally( + new WireParseException("invalid message id: expected " + qid + "; got id " + id)); + return f; + } + + Message response; + try { + response = parseMessage(in); + } catch (WireParseException e) { + f.completeExceptionally(e); + return f; + } + + // validate name, class and type (rfc5452#section-9.1) + if (!query.getQuestion().getName().equals(response.getQuestion().getName())) { + f.completeExceptionally( + new WireParseException( + "invalid name in message: expected " + + query.getQuestion().getName() + + "; got " + + response.getQuestion().getName())); + return f; + } + + if (query.getQuestion().getDClass() != response.getQuestion().getDClass()) { + f.completeExceptionally( + new WireParseException( + "invalid class in message: expected " + + DClass.string(query.getQuestion().getDClass()) + + "; got " + + DClass.string(response.getQuestion().getDClass()))); + return f; + } + + if (query.getQuestion().getType() != response.getQuestion().getType()) { + f.completeExceptionally( + new WireParseException( + "invalid type in message: expected " + + Type.string(query.getQuestion().getType()) + + "; got " + + Type.string(response.getQuestion().getType()))); + return f; + } + + verifyTSIG(query, response, in, tsig); + if (!tcp && !ignoreTruncation && response.getHeader().getFlag(Flags.TC)) { + log.debug("Got truncated response for id {}, retrying via TCP", qid); + log.trace("Truncated response: {}", response); + return sendAsync(query, true); + } + + response.setResolver(this); + f.complete(response); + } else if (v instanceof ArrayList) { + @SuppressWarnings("unchecked") + ArrayList list = (ArrayList)v; + Message response = null; + Throwable exc = null; + for (byte[] in : list) { + // Check that the response is long enough. + if (in.length < Header.LENGTH) { + exc = new WireParseException("invalid DNS header - too short"); + continue; + } - // Check that the response ID matches the query ID. We want - // to check this before actually parsing the message, so that - // if there's a malformed response that's not ours, it - // doesn't confuse us. - int id = ((in[0] & 0xFF) << 8) + (in[1] & 0xFF); - if (id != qid) { - f.completeExceptionally( - new WireParseException("invalid message id: expected " + qid + "; got id " + id)); - return f; - } + // Check that the response ID matches the query ID. We want + // to check this before actually parsing the message, so that + // if there's a malformed response that's not ours, it + // doesn't confuse us. + int id = ((in[0] & 0xFF) << 8) + (in[1] & 0xFF); + if (id != qid) { + exc = new WireParseException("invalid message id: expected " + qid + "; got id " + id); + continue; + } - Message response; - try { - response = parseMessage(in); - } catch (WireParseException e) { - f.completeExceptionally(e); - return f; - } + Message r; + try { + r = parseMessage(in); + } catch (WireParseException e) { + exc = e; + continue; + } - // validate name, class and type (rfc5452#section-9.1) - if (!query.getQuestion().getName().equals(response.getQuestion().getName())) { - f.completeExceptionally( - new WireParseException( - "invalid name in message: expected " - + query.getQuestion().getName() - + "; got " - + response.getQuestion().getName())); - return f; - } + // validate name, class and type (rfc5452#section-9.1) + if (!query.getQuestion().getName().equals(response.getQuestion().getName())) { + exc = + new WireParseException( + "invalid name in message: expected " + + query.getQuestion().getName() + + "; got " + + response.getQuestion().getName()); + continue; + } - if (query.getQuestion().getDClass() != response.getQuestion().getDClass()) { - f.completeExceptionally( - new WireParseException( - "invalid class in message: expected " - + DClass.string(query.getQuestion().getDClass()) - + "; got " - + DClass.string(response.getQuestion().getDClass()))); - return f; - } + if (query.getQuestion().getDClass() != response.getQuestion().getDClass()) { + exc = + new WireParseException( + "invalid class in message: expected " + + DClass.string(query.getQuestion().getDClass()) + + "; got " + + DClass.string(response.getQuestion().getDClass())); + continue; + } - if (query.getQuestion().getType() != response.getQuestion().getType()) { - f.completeExceptionally( - new WireParseException( - "invalid type in message: expected " - + Type.string(query.getQuestion().getType()) - + "; got " - + Type.string(response.getQuestion().getType()))); - return f; - } + if (query.getQuestion().getType() != response.getQuestion().getType()) { + exc = + new WireParseException( + "invalid type in message: expected " + + Type.string(query.getQuestion().getType()) + + "; got " + + Type.string(response.getQuestion().getType())); + continue; + } - verifyTSIG(query, response, in, tsig); - if (!tcp && !ignoreTruncation && response.getHeader().getFlag(Flags.TC)) { - log.debug("Got truncated response for id {}, retrying via TCP", qid); - log.trace("Truncated response: {}", response); - return sendAsync(query, true); - } + verifyTSIG(query, response, in, tsig); + if (!tcp && !ignoreTruncation && response.getHeader().getFlag(Flags.TC)) { + log.debug("Got truncated response for id {}, retrying via TCP", qid); + log.trace("Truncated response: {}", response); + continue; + } - response.setResolver(this); - f.complete(response); + if (response == null) { + response = r; + } else { + // not the first answer, append the results to first response + for (Record rec : r.getSection(Section.ANSWER)) { + response.addRecord(rec, Section.ANSWER); + } + for (Record rec : r.getSection(Section.AUTHORITY)) { + response.addRecord(rec, Section.AUTHORITY); + } + for (Record rec : r.getSection(Section.ADDITIONAL)) { + response.addRecord(rec, Section.ADDITIONAL); + } + } + } + + if (response != null) { + response.setResolver(this); + f.complete(response); + } else { + if (exc == null) { + exc = new SocketTimeoutException("Query timed out"); + } + f.completeExceptionally(exc); + } + } return f; }); } From fce0cf8b18dbddc65b8613cdd319dd3c281fa7df Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Wed, 6 Jan 2021 16:01:17 -0500 Subject: [PATCH 02/11] Update NioUdpClient.java Add support to wait for multiple answers to arrive if and only if the address of the DNS server is a multicast address. --- src/main/java/org/xbill/DNS/NioUdpClient.java | 100 +++++++++++++++--- 1 file changed, 86 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/xbill/DNS/NioUdpClient.java b/src/main/java/org/xbill/DNS/NioUdpClient.java index 2c7df7264..8b2627b79 100644 --- a/src/main/java/org/xbill/DNS/NioUdpClient.java +++ b/src/main/java/org/xbill/DNS/NioUdpClient.java @@ -4,6 +4,7 @@ import java.io.EOFException; import java.io.IOException; import java.net.InetSocketAddress; +import java.net.SocketAddress; import java.net.SocketException; import java.net.SocketTimeoutException; import java.nio.ByteBuffer; @@ -12,6 +13,7 @@ import java.nio.channels.Selector; import java.security.SecureRandom; import java.time.Duration; +import java.util.ArrayList; import java.util.Iterator; import java.util.Queue; import java.util.concurrent.CompletableFuture; @@ -71,8 +73,7 @@ private static void checkTransactionTimeouts() { for (Iterator it = pendingTransactions.iterator(); it.hasNext(); ) { Transaction t = it.next(); if (t.endTime - System.nanoTime() < 0) { - t.silentCloseChannel(); - t.f.completeExceptionally(new SocketTimeoutException("Query timed out")); + t.closeTransaction(); it.remove(); } } @@ -81,19 +82,20 @@ private static void checkTransactionTimeouts() { @RequiredArgsConstructor private static class Transaction implements KeyProcessor { private final byte[] data; - private final int max; + final int max; private final long endTime; private final DatagramChannel channel; - private final CompletableFuture f; + private final SocketAddress remoteSocketAddress; + final CompletableFuture f; void send() throws IOException { ByteBuffer buffer = ByteBuffer.wrap(data); verboseLog( "UDP write", channel.socket().getLocalSocketAddress(), - channel.socket().getRemoteSocketAddress(), + remoteSocketAddress, data); - int n = channel.send(buffer, channel.socket().getRemoteSocketAddress()); + int n = channel.send(buffer, remoteSocketAddress); if (n <= 0) { throw new EOFException(); } @@ -109,10 +111,12 @@ public void processReadyKey(SelectionKey key) { DatagramChannel channel = (DatagramChannel) key.channel(); ByteBuffer buffer = ByteBuffer.allocate(max); + SocketAddress source; int read; try { - read = channel.read(buffer); - if (read <= 0) { + source = channel.receive(buffer); + read = buffer.position(); + if (read <= 0 || source == null) { throw new EOFException(); } } catch (IOException e) { @@ -128,14 +132,14 @@ public void processReadyKey(SelectionKey key) { verboseLog( "UDP read", channel.socket().getLocalSocketAddress(), - channel.socket().getRemoteSocketAddress(), + remoteSocketAddress, data); silentCloseChannel(); f.complete(data); pendingTransactions.remove(this); } - private void silentCloseChannel() { + void silentCloseChannel() { try { channel.disconnect(); channel.close(); @@ -143,11 +147,73 @@ private void silentCloseChannel() { // ignore, we either already have everything we need or can't do anything } } + + void closeTransaction() { + silentCloseChannel(); + f.completeExceptionally(new SocketTimeoutException("Query timed out")); + } + } + + private static class MultiAnswerTransaction extends Transaction { + MultiAnswerTransaction(byte[] query, int max, long endTime, DatagramChannel channel, + SocketAddress remoteSocketAddress, + CompletableFuture f) { + super(query, max, endTime, channel, remoteSocketAddress, f); + } + + public void processReadyKey(SelectionKey key) { + if (!key.isReadable()) { + silentCloseChannel(); + f.completeExceptionally(new EOFException("channel not readable")); + pendingTransactions.remove(this); + return; + } + + DatagramChannel channel = (DatagramChannel) key.channel(); + ByteBuffer buffer = ByteBuffer.allocate(max); + SocketAddress source; + int read; + try { + source = channel.receive(buffer); + read = buffer.position(); + if (read <= 0 || source == null) { + return; // ignore this datagram + } + } catch (IOException e) { + silentCloseChannel(); + f.completeExceptionally(e); + pendingTransactions.remove(this); + return; + } + + buffer.flip(); + byte[] data = new byte[read]; + System.arraycopy(buffer.array(), 0, data, 0, read); + verboseLog( + "UDP read", + channel.socket().getLocalSocketAddress(), + source, + data); + answers.add(data); + } + + private ArrayList answers = new ArrayList<>(); + + @Override + void closeTransaction() { + if (answers.size() > 0) { + silentCloseChannel(); + f.complete(answers); + } else { + // we failed, no answers + super.closeTransaction(); + } + } } - static CompletableFuture sendrecv( + static CompletableFuture sendrecv( InetSocketAddress local, InetSocketAddress remote, byte[] data, int max, Duration timeout) { - CompletableFuture f = new CompletableFuture<>(); + CompletableFuture f = new CompletableFuture<>(); try { final Selector selector = selector(); DatagramChannel channel = DatagramChannel.open(); @@ -185,9 +251,15 @@ static CompletableFuture sendrecv( } } - channel.connect(remote); long endTime = System.nanoTime() + timeout.toNanos(); - Transaction t = new Transaction(data, max, endTime, channel, f); + Transaction t; + if (!remote.getAddress().isMulticastAddress()) { + channel.connect(remote); + t = new Transaction(data, max, endTime, channel, f); + } else { + // stop this a little before the timeout so we can report what answers we did get + t = new MultiAnswerTransaction(data, max, endTime - 1000000000L, channel, f); + } pendingTransactions.add(t); registrationQueue.add(t); selector.wakeup(); From 544182bcbcddade31b496410279b20e61dece6d6 Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Wed, 6 Jan 2021 16:07:49 -0500 Subject: [PATCH 03/11] Update SimpleResolver.java fix misleading text in logging message where the handling has changed. --- src/main/java/org/xbill/DNS/SimpleResolver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/xbill/DNS/SimpleResolver.java b/src/main/java/org/xbill/DNS/SimpleResolver.java index fd6dc08df..86560287c 100644 --- a/src/main/java/org/xbill/DNS/SimpleResolver.java +++ b/src/main/java/org/xbill/DNS/SimpleResolver.java @@ -482,7 +482,7 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { verifyTSIG(query, response, in, tsig); if (!tcp && !ignoreTruncation && response.getHeader().getFlag(Flags.TC)) { - log.debug("Got truncated response for id {}, retrying via TCP", qid); + log.debug("Got truncated response for id {}, discarding", qid); log.trace("Truncated response: {}", response); continue; } From 855b7be395ffb5e6a65ef29e78ca2f1d0cfef1b3 Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Thu, 7 Jan 2021 09:16:33 -0500 Subject: [PATCH 04/11] reduce duplicate code in mDNS support changes Refactor the mDNS changes to reduce duplicate code. --- src/main/java/org/xbill/DNS/NioUdpClient.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/xbill/DNS/NioUdpClient.java b/src/main/java/org/xbill/DNS/NioUdpClient.java index 8b2627b79..930a9b29d 100644 --- a/src/main/java/org/xbill/DNS/NioUdpClient.java +++ b/src/main/java/org/xbill/DNS/NioUdpClient.java @@ -14,7 +14,9 @@ import java.security.SecureRandom; import java.time.Duration; import java.util.ArrayList; +import java.util.Collections; import java.util.Iterator; +import java.util.List; import java.util.Queue; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentLinkedQueue; @@ -86,7 +88,7 @@ private static class Transaction implements KeyProcessor { private final long endTime; private final DatagramChannel channel; private final SocketAddress remoteSocketAddress; - final CompletableFuture f; + final CompletableFuture> f; void send() throws IOException { ByteBuffer buffer = ByteBuffer.wrap(data); @@ -132,10 +134,10 @@ public void processReadyKey(SelectionKey key) { verboseLog( "UDP read", channel.socket().getLocalSocketAddress(), - remoteSocketAddress, + source, data); silentCloseChannel(); - f.complete(data); + f.complete(Collections.singletonList(data)); pendingTransactions.remove(this); } @@ -147,7 +149,7 @@ void silentCloseChannel() { // ignore, we either already have everything we need or can't do anything } } - + void closeTransaction() { silentCloseChannel(); f.completeExceptionally(new SocketTimeoutException("Query timed out")); @@ -157,7 +159,7 @@ void closeTransaction() { private static class MultiAnswerTransaction extends Transaction { MultiAnswerTransaction(byte[] query, int max, long endTime, DatagramChannel channel, SocketAddress remoteSocketAddress, - CompletableFuture f) { + CompletableFuture> f) { super(query, max, endTime, channel, remoteSocketAddress, f); } @@ -211,9 +213,9 @@ void closeTransaction() { } } - static CompletableFuture sendrecv( + static CompletableFuture> sendrecv( InetSocketAddress local, InetSocketAddress remote, byte[] data, int max, Duration timeout) { - CompletableFuture f = new CompletableFuture<>(); + CompletableFuture> f = new CompletableFuture<>(); try { final Selector selector = selector(); DatagramChannel channel = DatagramChannel.open(); @@ -255,10 +257,10 @@ static CompletableFuture sendrecv( Transaction t; if (!remote.getAddress().isMulticastAddress()) { channel.connect(remote); - t = new Transaction(data, max, endTime, channel, f); + t = new Transaction(data, max, endTime, channel, remote, f); } else { // stop this a little before the timeout so we can report what answers we did get - t = new MultiAnswerTransaction(data, max, endTime - 1000000000L, channel, f); + t = new MultiAnswerTransaction(data, max, endTime - 1000000000L, channel, remote, f); } pendingTransactions.add(t); registrationQueue.add(t); From 1cf8fb85193f86b483ba51785c7fec7de1cec1a8 Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Thu, 7 Jan 2021 09:21:39 -0500 Subject: [PATCH 05/11] reduce duplicated code in mDNS changes refactor most of the duplicated code for error-checking a received query response datagram into a method object. --- .../java/org/xbill/DNS/SimpleResolver.java | 200 ++++++++---------- 1 file changed, 89 insertions(+), 111 deletions(-) diff --git a/src/main/java/org/xbill/DNS/SimpleResolver.java b/src/main/java/org/xbill/DNS/SimpleResolver.java index 86560287c..45c09e0f5 100644 --- a/src/main/java/org/xbill/DNS/SimpleResolver.java +++ b/src/main/java/org/xbill/DNS/SimpleResolver.java @@ -352,61 +352,12 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { v -> { CompletableFuture f = new CompletableFuture<>(); - if (v instanceof byte[]) { + Verify1Response verify1Response = new Verify1Response(query, qid); + if (v instanceof byte[]) { byte[] in = (byte[])v; - // Check that the response is long enough. - if (in.length < Header.LENGTH) { - f.completeExceptionally(new WireParseException("invalid DNS header - too short")); - return f; - } - - // Check that the response ID matches the query ID. We want - // to check this before actually parsing the message, so that - // if there's a malformed response that's not ours, it - // doesn't confuse us. - int id = ((in[0] & 0xFF) << 8) + (in[1] & 0xFF); - if (id != qid) { - f.completeExceptionally( - new WireParseException("invalid message id: expected " + qid + "; got id " + id)); - return f; - } - - Message response; - try { - response = parseMessage(in); - } catch (WireParseException e) { - f.completeExceptionally(e); - return f; - } - - // validate name, class and type (rfc5452#section-9.1) - if (!query.getQuestion().getName().equals(response.getQuestion().getName())) { - f.completeExceptionally( - new WireParseException( - "invalid name in message: expected " - + query.getQuestion().getName() - + "; got " - + response.getQuestion().getName())); - return f; - } - - if (query.getQuestion().getDClass() != response.getQuestion().getDClass()) { - f.completeExceptionally( - new WireParseException( - "invalid class in message: expected " - + DClass.string(query.getQuestion().getDClass()) - + "; got " - + DClass.string(response.getQuestion().getDClass()))); - return f; - } - - if (query.getQuestion().getType() != response.getQuestion().getType()) { - f.completeExceptionally( - new WireParseException( - "invalid type in message: expected " - + Type.string(query.getQuestion().getType()) - + "; got " - + Type.string(response.getQuestion().getType()))); + Message response = verify1Response.parse(in); + if (response == null) { + f.completeExceptionally(verify1Response.getExc()); return f; } @@ -419,71 +370,22 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { response.setResolver(this); f.complete(response); - } else if (v instanceof ArrayList) { + } else if (v instanceof ArrayList) { @SuppressWarnings("unchecked") ArrayList list = (ArrayList)v; Message response = null; Throwable exc = null; for (byte[] in : list) { - // Check that the response is long enough. - if (in.length < Header.LENGTH) { - exc = new WireParseException("invalid DNS header - too short"); - continue; - } - - // Check that the response ID matches the query ID. We want - // to check this before actually parsing the message, so that - // if there's a malformed response that's not ours, it - // doesn't confuse us. - int id = ((in[0] & 0xFF) << 8) + (in[1] & 0xFF); - if (id != qid) { - exc = new WireParseException("invalid message id: expected " + qid + "; got id " + id); - continue; - } - - Message r; - try { - r = parseMessage(in); - } catch (WireParseException e) { - exc = e; - continue; - } - - // validate name, class and type (rfc5452#section-9.1) - if (!query.getQuestion().getName().equals(response.getQuestion().getName())) { - exc = - new WireParseException( - "invalid name in message: expected " - + query.getQuestion().getName() - + "; got " - + response.getQuestion().getName()); - continue; - } - - if (query.getQuestion().getDClass() != response.getQuestion().getDClass()) { - exc = - new WireParseException( - "invalid class in message: expected " - + DClass.string(query.getQuestion().getDClass()) - + "; got " - + DClass.string(response.getQuestion().getDClass())); + Message r = verify1Response.parse(in); + if (r == null) { + exc = verify1Response.getExc(); continue; } - if (query.getQuestion().getType() != response.getQuestion().getType()) { - exc = - new WireParseException( - "invalid type in message: expected " - + Type.string(query.getQuestion().getType()) - + "; got " - + Type.string(response.getQuestion().getType())); - continue; - } - - verifyTSIG(query, response, in, tsig); - if (!tcp && !ignoreTruncation && response.getHeader().getFlag(Flags.TC)) { - log.debug("Got truncated response for id {}, discarding", qid); - log.trace("Truncated response: {}", response); + verifyTSIG(query, r, in, tsig); + if (!tcp && !ignoreTruncation && r.getHeader().getFlag(Flags.TC)) { + log.debug("Got truncated response for id {}, discarding`", qid); + log.trace("Truncated response: {}", r); continue; } @@ -542,4 +444,80 @@ private Message sendAXFR(Message query) throws IOException { public String toString() { return "SimpleResolver [" + address + "]"; } + + private class Verify1Response { + private Message query; + private int qid; + private Throwable exc; + + public Verify1Response(Message query, int qid) { + this.query = query; + this.qid = qid; + } + + public Throwable getExc() { + return exc; + } + + public Message parse(byte[] in) { + exc = null; + + // Check that the response is long enough. + if (in.length < Header.LENGTH) { + exc = new WireParseException("invalid DNS header - too short"); + return null; + } + + // Check that the response ID matches the query ID. We want + // to check this before actually parsing the message, so that + // if there's a malformed response that's not ours, it + // doesn't confuse us. + int id = ((in[0] & 0xFF) << 8) + (in[1] & 0xFF); + if (id != qid) { + exc = new WireParseException("invalid message id: expected " + qid + "; got id " + id); + return null; + } + + Message r; + try { + r = parseMessage(in); + } catch (WireParseException e) { + exc = e; + return null; + } + + // validate name, class and type (rfc5452#section-9.1) + if (!query.getQuestion().getName().equals(r.getQuestion().getName())) { + exc = + new WireParseException( + "invalid name in message: expected " + + query.getQuestion().getName() + + "; got " + + r.getQuestion().getName()); + return null; + } + + if (query.getQuestion().getDClass() != r.getQuestion().getDClass()) { + exc = + new WireParseException( + "invalid class in message: expected " + + DClass.string(query.getQuestion().getDClass()) + + "; got " + + DClass.string(r.getQuestion().getDClass())); + return null; + } + + if (query.getQuestion().getType() != r.getQuestion().getType()) { + exc = + new WireParseException( + "invalid type in message: expected " + + Type.string(query.getQuestion().getType()) + + "; got " + + Type.string(r.getQuestion().getType())); + return null; + } + + return r; + } + } } From cc916e5a6872ace64216fcf793542169a16e2e24 Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Thu, 7 Jan 2021 09:44:08 -0500 Subject: [PATCH 06/11] resolve format complaints in mDNS-modified code resolve maven com.coveo:format plugin's complaints --- src/main/java/org/xbill/DNS/NioUdpClient.java | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/xbill/DNS/NioUdpClient.java b/src/main/java/org/xbill/DNS/NioUdpClient.java index 930a9b29d..3cb15a87e 100644 --- a/src/main/java/org/xbill/DNS/NioUdpClient.java +++ b/src/main/java/org/xbill/DNS/NioUdpClient.java @@ -92,11 +92,7 @@ private static class Transaction implements KeyProcessor { void send() throws IOException { ByteBuffer buffer = ByteBuffer.wrap(data); - verboseLog( - "UDP write", - channel.socket().getLocalSocketAddress(), - remoteSocketAddress, - data); + verboseLog("UDP write", channel.socket().getLocalSocketAddress(), remoteSocketAddress, data); int n = channel.send(buffer, remoteSocketAddress); if (n <= 0) { throw new EOFException(); @@ -131,11 +127,7 @@ public void processReadyKey(SelectionKey key) { buffer.flip(); byte[] data = new byte[read]; System.arraycopy(buffer.array(), 0, data, 0, read); - verboseLog( - "UDP read", - channel.socket().getLocalSocketAddress(), - source, - data); + verboseLog("UDP read", channel.socket().getLocalSocketAddress(), source, data); silentCloseChannel(); f.complete(Collections.singletonList(data)); pendingTransactions.remove(this); @@ -157,13 +149,17 @@ void closeTransaction() { } private static class MultiAnswerTransaction extends Transaction { - MultiAnswerTransaction(byte[] query, int max, long endTime, DatagramChannel channel, - SocketAddress remoteSocketAddress, - CompletableFuture> f) { - super(query, max, endTime, channel, remoteSocketAddress, f); - } + MultiAnswerTransaction( + byte[] query, + int max, + long endTime, + DatagramChannel channel, + SocketAddress remoteSocketAddress, + CompletableFuture> f) { + super(query, max, endTime, channel, remoteSocketAddress, f); + } - public void processReadyKey(SelectionKey key) { + public void processReadyKey(SelectionKey key) { if (!key.isReadable()) { silentCloseChannel(); f.completeExceptionally(new EOFException("channel not readable")); @@ -191,11 +187,7 @@ public void processReadyKey(SelectionKey key) { buffer.flip(); byte[] data = new byte[read]; System.arraycopy(buffer.array(), 0, data, 0, read); - verboseLog( - "UDP read", - channel.socket().getLocalSocketAddress(), - source, - data); + verboseLog("UDP read", channel.socket().getLocalSocketAddress(), source, data); answers.add(data); } From e838df6ce8a049e310bd4d97725c4d0f285ba76c Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Thu, 7 Jan 2021 09:45:25 -0500 Subject: [PATCH 07/11] resolve format complaints in mDNS changes --- .../java/org/xbill/DNS/SimpleResolver.java | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/xbill/DNS/SimpleResolver.java b/src/main/java/org/xbill/DNS/SimpleResolver.java index 45c09e0f5..28f25456b 100644 --- a/src/main/java/org/xbill/DNS/SimpleResolver.java +++ b/src/main/java/org/xbill/DNS/SimpleResolver.java @@ -354,7 +354,7 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { Verify1Response verify1Response = new Verify1Response(query, qid); if (v instanceof byte[]) { - byte[] in = (byte[])v; + byte[] in = (byte[]) v; Message response = verify1Response.parse(in); if (response == null) { f.completeExceptionally(verify1Response.getExc()); @@ -372,7 +372,7 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { f.complete(response); } else if (v instanceof ArrayList) { @SuppressWarnings("unchecked") - ArrayList list = (ArrayList)v; + ArrayList list = (ArrayList) v; Message response = null; Throwable exc = null; for (byte[] in : list) { @@ -489,31 +489,31 @@ public Message parse(byte[] in) { // validate name, class and type (rfc5452#section-9.1) if (!query.getQuestion().getName().equals(r.getQuestion().getName())) { exc = - new WireParseException( - "invalid name in message: expected " - + query.getQuestion().getName() - + "; got " - + r.getQuestion().getName()); + new WireParseException( + "invalid name in message: expected " + + query.getQuestion().getName() + + "; got " + + r.getQuestion().getName()); return null; } if (query.getQuestion().getDClass() != r.getQuestion().getDClass()) { exc = - new WireParseException( - "invalid class in message: expected " - + DClass.string(query.getQuestion().getDClass()) - + "; got " - + DClass.string(r.getQuestion().getDClass())); + new WireParseException( + "invalid class in message: expected " + + DClass.string(query.getQuestion().getDClass()) + + "; got " + + DClass.string(r.getQuestion().getDClass())); return null; } if (query.getQuestion().getType() != r.getQuestion().getType()) { exc = - new WireParseException( - "invalid type in message: expected " - + Type.string(query.getQuestion().getType()) - + "; got " - + Type.string(r.getQuestion().getType())); + new WireParseException( + "invalid type in message: expected " + + Type.string(query.getQuestion().getType()) + + "; got " + + Type.string(r.getQuestion().getType())); return null; } From 92d98d500d30505f61938b883149fbc9369e075d Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Sat, 13 Mar 2021 16:19:48 -0500 Subject: [PATCH 08/11] resolve issues reported by dnsjava over regarding consistent behavior of TCP vs. UDP DNS queries and preventing accidental collision of UDP client port with mDNS server. --- src/main/java/org/xbill/DNS/NioTcpClient.java | 13 ++++--- src/main/java/org/xbill/DNS/NioUdpClient.java | 3 ++ .../java/org/xbill/DNS/SimpleResolver.java | 34 +++++++------------ .../java/org/xbill/DNS/NioTcpClientTest.java | 2 +- 4 files changed, 22 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/xbill/DNS/NioTcpClient.java b/src/main/java/org/xbill/DNS/NioTcpClient.java index 9eef1bbb3..6bd9bef9c 100644 --- a/src/main/java/org/xbill/DNS/NioTcpClient.java +++ b/src/main/java/org/xbill/DNS/NioTcpClient.java @@ -10,9 +10,7 @@ import java.nio.channels.Selector; import java.nio.channels.SocketChannel; import java.time.Duration; -import java.util.Iterator; -import java.util.Map; -import java.util.Queue; +import java.util.*; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; @@ -74,7 +72,8 @@ private static class Transaction { private final byte[] queryData; private final long endTime; private final SocketChannel channel; - private final CompletableFuture f; + private final CompletableFuture> f; + private boolean sendDone; void send() throws IOException { @@ -206,7 +205,7 @@ private void processRead() { int id = ((data[0] & 0xFF) << 8) + (data[1] & 0xFF); int qid = t.query.getHeader().getID(); if (id == qid) { - t.f.complete(data); + t.f.complete(Collections.singletonList(data)); it.remove(); return; } @@ -235,13 +234,13 @@ private static class ChannelKey { final InetSocketAddress remote; } - static CompletableFuture sendrecv( + static CompletableFuture> sendrecv( InetSocketAddress local, InetSocketAddress remote, Message query, byte[] data, Duration timeout) { - CompletableFuture f = new CompletableFuture<>(); + CompletableFuture> f = new CompletableFuture<>(); try { final Selector selector = selector(); long endTime = System.nanoTime() + timeout.toNanos(); diff --git a/src/main/java/org/xbill/DNS/NioUdpClient.java b/src/main/java/org/xbill/DNS/NioUdpClient.java index 3cb15a87e..3f3ffa2ca 100644 --- a/src/main/java/org/xbill/DNS/NioUdpClient.java +++ b/src/main/java/org/xbill/DNS/NioUdpClient.java @@ -229,6 +229,9 @@ static CompletableFuture> sendrecv( addr = new InetSocketAddress(local.getAddress(), port); } + if (addr.getPort() == SimpleResolver.RESERVED_MDNS_PORT) { + continue; // can't use the mDNS server port, try again + } channel.bind(addr); bound = true; diff --git a/src/main/java/org/xbill/DNS/SimpleResolver.java b/src/main/java/org/xbill/DNS/SimpleResolver.java index 28f25456b..5da7b56bc 100644 --- a/src/main/java/org/xbill/DNS/SimpleResolver.java +++ b/src/main/java/org/xbill/DNS/SimpleResolver.java @@ -30,6 +30,9 @@ public class SimpleResolver implements Resolver { /** The default port to send queries to */ public static final int DEFAULT_PORT = 53; + /** The port we can't use as a client because it's reserved for mDNS servers. */ + public static final int RESERVED_MDNS_PORT = 5353; + /** The default EDNS payload size */ public static final int DEFAULT_EDNS_PAYLOADSIZE = 1280; @@ -341,7 +344,7 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { address.getPort()); log.trace("Query:\n{}", query); - CompletableFuture result; + CompletableFuture> result; if (tcp) { result = NioTcpClient.sendrecv(localAddress, address, query, out, timeoutValue); } else { @@ -353,29 +356,10 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { CompletableFuture f = new CompletableFuture<>(); Verify1Response verify1Response = new Verify1Response(query, qid); - if (v instanceof byte[]) { - byte[] in = (byte[]) v; - Message response = verify1Response.parse(in); - if (response == null) { - f.completeExceptionally(verify1Response.getExc()); - return f; - } - - verifyTSIG(query, response, in, tsig); - if (!tcp && !ignoreTruncation && response.getHeader().getFlag(Flags.TC)) { - log.debug("Got truncated response for id {}, retrying via TCP", qid); - log.trace("Truncated response: {}", response); - return sendAsync(query, true); - } - - response.setResolver(this); - f.complete(response); - } else if (v instanceof ArrayList) { @SuppressWarnings("unchecked") - ArrayList list = (ArrayList) v; Message response = null; Throwable exc = null; - for (byte[] in : list) { + for (byte[] in : v) { Message r = verify1Response.parse(in); if (r == null) { exc = verify1Response.getExc(); @@ -414,7 +398,6 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { } f.completeExceptionally(exc); } - } return f; }); } @@ -455,6 +438,13 @@ public Verify1Response(Message query, int qid) { this.qid = qid; } + /** + * Keep the exception that caused this to fail. We don't throw the exception + * immediately because, when using mDNS, we might get both failing and successful + * answers, in which case we just want to discard the bad answers and keep the + * good ones; the exception only gets thrown if only bad answers are received. + * @return the Throwable reporting why this parse failed. + */ public Throwable getExc() { return exc; } diff --git a/src/test/java/org/xbill/DNS/NioTcpClientTest.java b/src/test/java/org/xbill/DNS/NioTcpClientTest.java index 1b2e009ed..c99d936a4 100644 --- a/src/test/java/org/xbill/DNS/NioTcpClientTest.java +++ b/src/test/java/org/xbill/DNS/NioTcpClientTest.java @@ -67,7 +67,7 @@ void testResponseStream() throws InterruptedException, IOException { .thenAccept( d -> { try { - clientReceivedAnswers[jj] = new Message(d); + clientReceivedAnswers[jj] = new Message(d.get(0)); cdl2.countDown(); } catch (IOException e) { fail(e); From 80aa3cdd17ed01000bff6432abd4e0404ff7a24e Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Sat, 13 Mar 2021 16:44:39 -0500 Subject: [PATCH 09/11] a few more bugfixes to the mDNS changes per dnsjava author's request. --- src/main/java/org/xbill/DNS/DClass.java | 3 ++ .../java/org/xbill/DNS/SimpleResolver.java | 41 +++++++++---------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/xbill/DNS/DClass.java b/src/main/java/org/xbill/DNS/DClass.java index 42f5c09b7..3d09fc3a9 100644 --- a/src/main/java/org/xbill/DNS/DClass.java +++ b/src/main/java/org/xbill/DNS/DClass.java @@ -31,6 +31,9 @@ public final class DClass { /** Matches any class */ public static final int ANY = 255; + /** Indicates on mDNS that querier will accept unicast replies from a multicast request. */ + public static final int UNICAST_RESPONSE = 0x8000; + private static class DClassMnemonic extends Mnemonic { public DClassMnemonic() { super("DClass", CASE_UPPER); diff --git a/src/main/java/org/xbill/DNS/SimpleResolver.java b/src/main/java/org/xbill/DNS/SimpleResolver.java index 5da7b56bc..8722503f0 100644 --- a/src/main/java/org/xbill/DNS/SimpleResolver.java +++ b/src/main/java/org/xbill/DNS/SimpleResolver.java @@ -8,7 +8,6 @@ import java.net.SocketTimeoutException; import java.net.UnknownHostException; import java.time.Duration; -import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.concurrent.CompletableFuture; @@ -355,14 +354,14 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { v -> { CompletableFuture f = new CompletableFuture<>(); - Verify1Response verify1Response = new Verify1Response(query, qid); + VerifyOneResponse verifyOneResponse = new VerifyOneResponse(query, qid); @SuppressWarnings("unchecked") Message response = null; - Throwable exc = null; + Throwable responseFailureException = null; for (byte[] in : v) { - Message r = verify1Response.parse(in); + Message r = verifyOneResponse.parse(in); if (r == null) { - exc = verify1Response.getExc(); + responseFailureException = verifyOneResponse.getFailureReason(); continue; } @@ -393,10 +392,10 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { response.setResolver(this); f.complete(response); } else { - if (exc == null) { - exc = new SocketTimeoutException("Query timed out"); + if (responseFailureException == null) { + responseFailureException = new SocketTimeoutException("Query timed out"); } - f.completeExceptionally(exc); + f.completeExceptionally(responseFailureException); } return f; }); @@ -428,12 +427,12 @@ public String toString() { return "SimpleResolver [" + address + "]"; } - private class Verify1Response { + private class VerifyOneResponse { private Message query; private int qid; - private Throwable exc; + private Throwable failureReasonException; - public Verify1Response(Message query, int qid) { + public VerifyOneResponse(Message query, int qid) { this.query = query; this.qid = qid; } @@ -445,16 +444,16 @@ public Verify1Response(Message query, int qid) { * good ones; the exception only gets thrown if only bad answers are received. * @return the Throwable reporting why this parse failed. */ - public Throwable getExc() { - return exc; + public Throwable getFailureReason() { + return failureReasonException; } public Message parse(byte[] in) { - exc = null; + failureReasonException = null; // Check that the response is long enough. if (in.length < Header.LENGTH) { - exc = new WireParseException("invalid DNS header - too short"); + failureReasonException = new WireParseException("invalid DNS header - too short"); return null; } @@ -464,7 +463,7 @@ public Message parse(byte[] in) { // doesn't confuse us. int id = ((in[0] & 0xFF) << 8) + (in[1] & 0xFF); if (id != qid) { - exc = new WireParseException("invalid message id: expected " + qid + "; got id " + id); + failureReasonException = new WireParseException("invalid message id: expected " + qid + "; got id " + id); return null; } @@ -472,13 +471,13 @@ public Message parse(byte[] in) { try { r = parseMessage(in); } catch (WireParseException e) { - exc = e; + failureReasonException = e; return null; } // validate name, class and type (rfc5452#section-9.1) if (!query.getQuestion().getName().equals(r.getQuestion().getName())) { - exc = + failureReasonException = new WireParseException( "invalid name in message: expected " + query.getQuestion().getName() @@ -487,8 +486,8 @@ public Message parse(byte[] in) { return null; } - if (query.getQuestion().getDClass() != r.getQuestion().getDClass()) { - exc = + if ((query.getQuestion().getDClass() & ~DClass.UNICAST_RESPONSE) != (r.getQuestion().getDClass() & ~DClass.UNICAST_RESPONSE)) { + failureReasonException = new WireParseException( "invalid class in message: expected " + DClass.string(query.getQuestion().getDClass()) @@ -498,7 +497,7 @@ public Message parse(byte[] in) { } if (query.getQuestion().getType() != r.getQuestion().getType()) { - exc = + failureReasonException = new WireParseException( "invalid type in message: expected " + Type.string(query.getQuestion().getType()) From 31f26216f5627117cc3b4e398381095dc626be1e Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Sat, 13 Mar 2021 16:50:58 -0500 Subject: [PATCH 10/11] make the format checker happy (again!). --- .../java/org/xbill/DNS/SimpleResolver.java | 87 ++++++++++--------- 1 file changed, 45 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/xbill/DNS/SimpleResolver.java b/src/main/java/org/xbill/DNS/SimpleResolver.java index 8722503f0..eccddac6c 100644 --- a/src/main/java/org/xbill/DNS/SimpleResolver.java +++ b/src/main/java/org/xbill/DNS/SimpleResolver.java @@ -355,48 +355,48 @@ CompletableFuture sendAsync(Message query, boolean forceTcp) { CompletableFuture f = new CompletableFuture<>(); VerifyOneResponse verifyOneResponse = new VerifyOneResponse(query, qid); - @SuppressWarnings("unchecked") - Message response = null; - Throwable responseFailureException = null; - for (byte[] in : v) { - Message r = verifyOneResponse.parse(in); - if (r == null) { - responseFailureException = verifyOneResponse.getFailureReason(); - continue; - } - - verifyTSIG(query, r, in, tsig); - if (!tcp && !ignoreTruncation && r.getHeader().getFlag(Flags.TC)) { - log.debug("Got truncated response for id {}, discarding`", qid); - log.trace("Truncated response: {}", r); - continue; - } + @SuppressWarnings("unchecked") + Message response = null; + Throwable responseFailureException = null; + for (byte[] in : v) { + Message r = verifyOneResponse.parse(in); + if (r == null) { + responseFailureException = verifyOneResponse.getFailureReason(); + continue; + } - if (response == null) { - response = r; - } else { - // not the first answer, append the results to first response - for (Record rec : r.getSection(Section.ANSWER)) { - response.addRecord(rec, Section.ANSWER); - } - for (Record rec : r.getSection(Section.AUTHORITY)) { - response.addRecord(rec, Section.AUTHORITY); - } - for (Record rec : r.getSection(Section.ADDITIONAL)) { - response.addRecord(rec, Section.ADDITIONAL); - } - } + verifyTSIG(query, r, in, tsig); + if (!tcp && !ignoreTruncation && r.getHeader().getFlag(Flags.TC)) { + log.debug("Got truncated response for id {}, discarding`", qid); + log.trace("Truncated response: {}", r); + continue; } - if (response != null) { - response.setResolver(this); - f.complete(response); + if (response == null) { + response = r; } else { - if (responseFailureException == null) { - responseFailureException = new SocketTimeoutException("Query timed out"); + // not the first answer, append the results to first response + for (Record rec : r.getSection(Section.ANSWER)) { + response.addRecord(rec, Section.ANSWER); + } + for (Record rec : r.getSection(Section.AUTHORITY)) { + response.addRecord(rec, Section.AUTHORITY); + } + for (Record rec : r.getSection(Section.ADDITIONAL)) { + response.addRecord(rec, Section.ADDITIONAL); } - f.completeExceptionally(responseFailureException); } + } + + if (response != null) { + response.setResolver(this); + f.complete(response); + } else { + if (responseFailureException == null) { + responseFailureException = new SocketTimeoutException("Query timed out"); + } + f.completeExceptionally(responseFailureException); + } return f; }); } @@ -438,10 +438,11 @@ public VerifyOneResponse(Message query, int qid) { } /** - * Keep the exception that caused this to fail. We don't throw the exception - * immediately because, when using mDNS, we might get both failing and successful - * answers, in which case we just want to discard the bad answers and keep the - * good ones; the exception only gets thrown if only bad answers are received. + * Keep the exception that caused this to fail. We don't throw the exception immediately + * because, when using mDNS, we might get both failing and successful answers, in which case we + * just want to discard the bad answers and keep the good ones; the exception only gets thrown + * if only bad answers are received. + * * @return the Throwable reporting why this parse failed. */ public Throwable getFailureReason() { @@ -463,7 +464,8 @@ public Message parse(byte[] in) { // doesn't confuse us. int id = ((in[0] & 0xFF) << 8) + (in[1] & 0xFF); if (id != qid) { - failureReasonException = new WireParseException("invalid message id: expected " + qid + "; got id " + id); + failureReasonException = + new WireParseException("invalid message id: expected " + qid + "; got id " + id); return null; } @@ -486,7 +488,8 @@ public Message parse(byte[] in) { return null; } - if ((query.getQuestion().getDClass() & ~DClass.UNICAST_RESPONSE) != (r.getQuestion().getDClass() & ~DClass.UNICAST_RESPONSE)) { + if ((query.getQuestion().getDClass() & ~DClass.UNICAST_RESPONSE) + != (r.getQuestion().getDClass() & ~DClass.UNICAST_RESPONSE)) { failureReasonException = new WireParseException( "invalid class in message: expected " From c1ce27b8e2ec3546a0e420d0fbf01d6827373365 Mon Sep 17 00:00:00 2001 From: Andrew Pavlin Date: Sat, 13 Mar 2021 16:57:44 -0500 Subject: [PATCH 11/11] more formatting issues that the online checker didn't report on the first build failure. --- src/main/java/org/xbill/DNS/NioTcpClient.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/xbill/DNS/NioTcpClient.java b/src/main/java/org/xbill/DNS/NioTcpClient.java index 6bd9bef9c..76f014677 100644 --- a/src/main/java/org/xbill/DNS/NioTcpClient.java +++ b/src/main/java/org/xbill/DNS/NioTcpClient.java @@ -10,7 +10,11 @@ import java.nio.channels.Selector; import java.nio.channels.SocketChannel; import java.time.Duration; -import java.util.*; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Queue; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue;