patch to support mDNS and multicast to servers#148
Conversation
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).
Add support to wait for multiple answers to arrive if and only if the address of the DNS server is a multicast address.
fix misleading text in logging message where the handling has changed.
|
Thanks for creating a PR! I need to read the mDNS spec to see if this is correct, which will take me a while. In the meantime, this doesn't even compile and from a first look you seem to have done a lot of code duplication when parsing the answer(s). Please simplify this, e.g. by extracting the validation and parsing into a method that you can call multiple times. Also, the non-generic response from transaction/multitransaction and later casting is something I'd like to avoid. |
Sorry, something went wrong.
|
OK. I was trying to minimize changes to existing (unicast) logic so I wouldn't break anything. But I can rework it to reduce code redundancy.
Andrew
…________________________________________
From: Ingo Bauersachs <notifications@github.com>
Sent: Wednesday, January 6, 2021 5:19 PM
To: dnsjava/dnsjava
Cc: Andrew Pavlin; Author
Subject: Re: [dnsjava/dnsjava] patch to support mDNS and multicast to servers (#148)
Thanks for creating a PR! I need to read the mDNS spec to see if this is correct, which will take me a while.
In the meantime, this doesn't even compile and from a first look you seem to have done a lot of code duplication when parsing the answer(s). Please simplify this, e.g. by extracting the validation and parsing into a method that you can call multiple times.
Also, the non-generic response from transaction/multitransaction and later casting is something I'd like to avoid. sendrecv could just return a Future<List<byte[]>> where the single transaction always returns a list with a single entry. Distinguishing if answers should be combined or not can still be done based on the server's destination address.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#148 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AESTBTJNLGDIR2OPIHKUNF3SYTOYZANCNFSM4VYCJ7VA>.
|
Sorry, something went wrong.
Refactor the mDNS changes to reduce duplicate code.
refactor most of the duplicated code for error-checking a received query response datagram into a method object.
|
This is bizarre. The code compiles, but something about a format check in the source code in your maven script caused it to be rejected. And I copied the format that was already in those source files.
So I looked at the error from the pull request build, so I am making a third change to my branch of the code base to keep the format-checking Maven plugin from griping.
…________________________________________
From: Ingo Bauersachs <notifications@github.com>
Sent: Wednesday, January 6, 2021 5:19 PM
To: dnsjava/dnsjava
Cc: Andrew Pavlin; Author
Subject: Re: [dnsjava/dnsjava] patch to support mDNS and multicast to servers (#148)
Thanks for creating a PR! I need to read the mDNS spec to see if this is correct, which will take me a while.
In the meantime, this doesn't even compile and from a first look you seem to have done a lot of code duplication when parsing the answer(s). Please simplify this, e.g. by extracting the validation and parsing into a method that you can call multiple times.
Also, the non-generic response from transaction/multitransaction and later casting is something I'd like to avoid. sendrecv could just return a Future<List<byte[]>> where the single transaction always returns a list with a single entry. Distinguishing if answers should be combined or not can still be done based on the server's destination address.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#148 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AESTBTJNLGDIR2OPIHKUNF3SYTOYZANCNFSM4VYCJ7VA>.
|
Sorry, something went wrong.
resolve maven com.coveo:format plugin's complaints
ibauersachs
left a comment
There was a problem hiding this comment.
Have you looked at some details of rfc6762? Specifically:
- port 5353 must not be used as the local port as we're not fully compliant
- Section 3 mandates that any query to
.local.goes to an mDNS server. Not sure how to properly address that, enabling it by default is not an option because .local has been abused by non-mdns setups too much. But I could imagine that theExtendedResolversends .local to the multicast address if the multicast address is one of the resolver addresses. A multicast resolver address could be automatically added if a (new) property is set. - Setting the
unicast-responsebit should be easier to set/strip out (DClass). Not quite sure how repsonses look like. If the bit is still set in the response, it needs to be stripped after validation (or the answers won't be recognized as classIN), otherwise the validation needs to accept responses without the bit set as valid
What's an easy way to live test this code? Should having some Windows/Apple devices in the network suffice?
Sorry, something went wrong.
You can auto-format the code by running the Maven target |
Sorry, something went wrong.
|
Re: reformatting: I didn't want to reformat any lines I didn't change, and cause spurious non-changing source code changes. Such behavior of IDEs (that "always know better than you do how your code should be formatted", especially when multiple developers' IDEs are configured differently) makes it a pain to review code changes, so I am always very careful not to change the styling of other lines of code (not even switching spaces versus tabs). |
Sorry, something went wrong.
I sure did. That's how I came up with the idea of deliberately timing out the resolver so I could wait for multiple answers to come in.
My proposed changes don't specify the port number. The caller is currently required to be smart enough to specify port 5353 when they also specify a multicast address as the "address" of the remote DNS server, since the SimpleResolver constructor I tested with takes InetSocketAddress objects that include the port number. I suppose the constructors taking a String or an InetAddress should check for multicast addresses and switch from DEFAULT_PORT (= 53) to 5353 automatically. Or are you saying I need to add a safety check for when the local port is selected to ensure it doesn't accidentally get 5353 as the port number?
That would have been a larger-impact change that I didn't need to do for my limited use-case, which knew that I was making a ".local" query. Would you like me to make the additional changes to automatically switch to multicast for such queries? Problem is, can I safely assume only IPv6 or only IPv4 for the default multicast address?
I didn't catch that, because my tests all worked correctly with multicast responses. I never received such a response when I did a multicast query in my tests. But I can make such a change to clear that bit in the received responses. Would you like me to do so?
Have a Linux system on the LAN with the avahi-daemon running, then use the avahi-publish command to register some mDNS entries for testing. I suppose Apple devices on the LAN would do the same thing, but you would have less control over what answers you could get. |
Sorry, something went wrong.
Yes, that's what I meant.
I'm not sure how useful it is. I guess we could always add it later if it seems worthwhile.
It depends :-)
I'm not sure it's required, and I couldn't conclude from the RFC without looking at packets.
Okay, I'll look at some of those packets, then I'll know if that bit mentioned above needs attention or not. Thanks. |
Sorry, something went wrong.
|
Okay, so I sent some sample queries on my LAN with the unicode flag masked in, like this: public class Test {
public static void main(String[] args) throws IOException {
SimpleResolver sr = new SimpleResolver(new InetSocketAddress("224.0.0.251", 5353));
Record q = Record.newRecord(Name.fromConstantString("macmini.local."), Type.A, DClass.IN | 0x8000);
Message mq = Message.newQuery(q);
System.out.println(sr.send(mq));
}
}I get a unicast reply from the Mac Mini, but also two multicast responses from avahi-daemons I seem to have running. The answers from the avahi-daemons are thrown away because the validation in The relevant part from the RFC is this sentence (emphasis mine):
Setting the unicast flag in the first hand with masking 0x8000 into Note I'm not sure how to best handle this flag (in questions and answers). Maybe @nresare could chime in here and has an idea? |
Sorry, something went wrong.
of TCP vs. UDP DNS queries and preventing accidental collision of UDP client port with mDNS server.
|
additional changes you requested submitted to mDNS_patch_ka2ddo branch. |
Sorry, something went wrong.
|
and fixing the formatting again.... |
Sorry, something went wrong.
first build failure.
|
and again making the formatter happy (since it only makes one complaint at a time). |
Sorry, something went wrong.
ibauersachs
left a comment
There was a problem hiding this comment.
Thanks for the update. I've left a bunch of comments inline.
The bigger question about how to handle the DClass in questions and responses however is still not clear:
- In questions, the highest bit set in the Class means Unicast
- In answers, the highest bit set in the Class means "Cache flush"
For both cases, this needs to be properly handled, i.e. not only in the SimpleResolver but also in e.g. Message.toString(), Record.getDClass() and probably lots of other locations. Maybe a flag boolean isMulticast; in the Message and Record classes is an option.
and again making the formatter happy (since it only makes one complaint at a time).
You can run the checks locally with mvn verify or make the checks happy by applying the formats to your IDE with the links provided previously.
Sorry, something went wrong.
|
Hello.
Sorry I'm behind on this patch. I hope to get back to it next week.
Andrew
…________________________________________
From: cary-xian ***@***.***>
Sent: Monday, July 26, 2021 8:26 PM
To: dnsjava/dnsjava
Cc: Andrew Pavlin; Author
Subject: Re: [dnsjava/dnsjava] patch to support mDNS and multicast to servers (#148)
dns
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#148 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AESTBTKTWMZGUGRIXCGUBK3TZX4JVANCNFSM4VYCJ7VA>.
|
Sorry, something went wrong.
|
Is this still a WIP? And are you still working on this @ka2ddo? I wouldn't mind giving it a look. |
Sorry, something went wrong.
a601d27 to
378aa24
Compare
May 10, 2026 11:16
changes so that multicast IP addresses can be used for the DNS "server", so that mDNS servers can be queried to support DNS-SD (Service Discovery).