keepsimple1 / mdns-sd

Rust library for mDNS based Service Discovery
Apache License 2.0
96 stars 37 forks source link

Added the link-local feature to if_addrs in Cargo.toml #126

Closed hgomersall closed 1 year ago

hgomersall commented 1 year ago

if_addrs disables link-local addresses on windows if the "link-local" feature is not enabled. See: https://github.com/messense/if-addrs/blob/947c6342681b047b48b5f53eb75049881d2dfa20/src/sockaddr.rs#L63C21-L63C21

Currently, the behaviour on Linux is to allow link-local IPv4 addresses, but IPv6 and windows is behind the feature gate.

This PR trivially adds the link-local feature to the if-addrs package.

It might be desirable to reflect the feature gating of link-local addresses instead of accepting this PR as-is.

hgomersall commented 1 year ago

Agreed. Would you mind add that (e.g. in [features] , add link-local = ["if-addrs/link-local"] and enable it in default ) ?

It seems odd to me that under Linux this flag would be ignored entirely on IPv4. It's probably why the issue with if-addrs wasn't picked up sooner because AFAICT it impacts both browse and register - that is, link-local addresses are completely removed from the mdns-sd process except for Linux IPv4.

Is it desired behaviour that the flag will be ignored with IPv4 under linux (as per if-addrs)?

keepsimple1 commented 1 year ago

It seems odd to me that under Linux this flag would be ignored entirely on IPv4. It's probably why the issue with if-addrs wasn't picked up sooner because AFAICT it impacts both browse and register - that is, link-local addresses are completely removed from the mdns-sd process except for Linux IPv4.

Is it desired behaviour that the flag will be ignored with IPv4 under linux (as per if-addrs)?

I see what you mean. It seems inconsistent to me. Probably only the authors of if-addrs knows about the reason / history behind this behavior.

Given that, I think we can just apply the change you have now. It will be a user visible change on Windows, so we will bump up the minor version in the next release. I think It's a good change as it means more consistent behavior between Linux/macOS and Windows.

For adding the feature gate in our crate, we can do it later if needed. It should be okay as the feature gate will be enabled by default, so it would not cause backward compatibility issue.