search

librespot-org / libmdns

mDNS Responder library for building discoverable LAN services in Rust
https://crates.io/crates/libmdns
MIT License
71 stars 34 forks source link

service type enumeration support #38

Closed elbe0046 closed 2 years ago

elbe0046 commented 2 years ago

I'm curious to see if there is any interest in landing support for service type enumeration. @willstott101 would such a change align with what you envision for this library? If the underlying idea seems palatable I'm happy to make changes to what is in this PR if you prefer a different approach / form.

For example, if gating this functionality behind a service-type-enumeration (or similar) feature would be preferred I can certainly do that.

For reference, service type enumeration is detailed in the DNS-Based Service Discovery RFC (https://www.rfc-editor.org/rfc/rfc6763#section-9):

In general, a normal client is not interested in finding every service on the network, just the services that the client knows how to use.

However, for problem diagnosis and network management tools, it may be useful for network administrators to find the list of advertised service types on the network, even if those Service Names are just opaque identifiers and not particularly informative in isolation.

For this purpose, a special meta-query is defined. A DNS query for PTR records with the name "_services._dns-sd._udp." yields a set of PTR records, where the rdata of each PTR record is the two- label name, plus the same domain, e.g., "_http._tcp.". Including the domain in the PTR rdata allows for slightly better name compression in Unicast DNS responses, but only the first two labels are relevant for the purposes of service type enumeration. These two-label service types can then be used to construct subsequent Service Instance Enumeration PTR queries, in this or others, to discover instances of that service type.

These changes aim to add support for such service type enumeration by implementing the described response to a PTR record query on "_services._dns-sd._udp.". This allows for utilities such as avahi-browse to discover services using libmdns without needing to query for a specific service type.

Thanks in advance for your time!

willstott101 commented 2 years ago

Thanks! Absolutely interested in including this.

If you're willing I'd love to see an integration test that exercises this functionality with a third-party client. Maybe using the python zeroconf lib, as with the existing tests, provided it supports service type enumeration.

elbe0046 commented 2 years ago

If you're willing I'd love to see an integration test that exercises this functionality with a third-party client. Maybe using the python zeroconf lib, as with the existing tests, provided it supports service type enumeration.

That's a great idea, I should have done so to begin with. I added a commit with an initial pass at such a test: https://github.com/librespot-org/libmdns/pull/38/commits/6b2ffba3216bf891559f16f7d5423a02de716118.

If you prefer that structured differently (multiple files, etc.) just let me know what you have in mind and I'll update.

I took a couple of liberties as well in the tests, any of them can be reverted:

Thanks again!

willstott101 commented 2 years ago

Thanks again for your patience. Just had a moment to take another look at this. I see that you imported ZeroconfServiceTypes, but then aren't using it for the actual test. I did some testing and it seems to work with that class.

Ideally with the integration test we'll be checking against as high-level a library as possible to ensure we aren't doing something differently than another mDNS client might be as it queries our services.

Thanks for your work on this, I'll release as a new patch release shortly.

elbe0046 commented 2 years ago

Thanks again for your patience. Just had a moment to take another look at this.

Not at all, thank you for all of your time and for working with me to land this change!

I see that you imported ZeroconfServiceTypes, but then aren't using it for the actual test. I did some testing and it seems to work with that class.

Ah you're right, I neglected to remove it when I switched approaches, sorry about that.

Ideally with the integration test we'll be checking against as high-level a library as possible to ensure we aren't doing something differently than another mDNS client might be as it queries our services.

If there's any followup work you'd like for me to do here not covered by that test in zeroconf_test.py, I'd be happy to do that. I'm a bit less familiar with the python / zeroconf side of things, so if you have a recommendation for a higher-level library / API any recommendations you have would be greatly appreciated.

Once again, thank you so much!

willstott101 commented 2 years ago

No problem, thanks for all your contributions. All I mean by high-level API is using ZeroconfServiceTypes really.

Let's say we go the string "_services._dns-sd._udp.local." wrong, by specifying it in the test too we're not really testing integration with this other library, just that this library can query against us when we do it our way.

I switched the test back to using ZeroconfServiceTypes myself: https://github.com/librespot-org/libmdns/commit/10ceb7b23087a31510f2b3d4243d323b94422021 which seems to work fine :smiley: