windy1 / zeroconf-rs

zeroconf is a cross-platform library that wraps underlying ZeroConf/mDNS implementations such as Bonjour or Avahi, providing an easy and idiomatic way to both register and browse services.
MIT License
77 stars 25 forks source link

Fix incorrect port numbers being registered for services on macOS. #16

Closed nsabovic closed 3 years ago

nsabovic commented 3 years ago

Per https://developer.apple.com/documentation/dnssd/1804733-dnsserviceregister, DNSServiceRegister() takes the port number in network byte order. This fixes the bug with wrong ports being registered on macOS.

windy1 commented 3 years ago

Thanks for this, you are of course correct. However, while TCP/IP's byte order is big-endian there could conceivably be other protocols with a little-endian byte order. I'm wondering if perhaps an additional interface on the API level is required to specify the byte order of the port with a default of big-endian.

e.g.

enum ByteOrder {
    LittleEndian,
    BigEndian,
}

// ...
service.set_port_byte_order(ByteOrder::BigEndian);
// ...

And then swap the bytes as necessary in BonjourMdnsService before passing it to the Bonjour wrapper.

nsabovic commented 3 years ago

@windy1 excellent!

The issue at hand is not the endiannes of the protocol but simply the function DNSServiceRegister() which is silly and wants byte-swapped integer. I don't think there's any benefit of doing all that work and introducing port byte order, I'd always prefer to specify it as an integer.

windy1 commented 3 years ago

@nsabovic While in practice I'm sure you are right. I believe that theoretically "network byte order" (as it's called in the docs) refers to the byte order that the protocol defines. (see: Endianness notes on network order). From what I can tell 99.9% of the time big-endian is the de facto standard for network protocols; however, there is no reason why a little-endian protocol could not exist.

I have an alternative idea on how to implement this in such a way where big-endian is the default (so a normal user would not have to ever worry about this) while also making the byte order configurable on the backend. I'm going to take a look myself and see where I land on this.

In the meantime, do you have a reliable way to reproduce / test this issue?

nsabovic commented 3 years ago

Yup, I tested this on my mac. We have an app where registering port 12345 (0x3039) results in port 14640 (0x3930) being registered, and this fixes it.