Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow advertising services on port 0 #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akdor1154
Copy link

@akdor1154 akdor1154 commented Nov 26, 2023

hey, thanks for maintaining this!

Here's a small (maybe annoying) fix:

https://datatracker.ietf.org/doc/html/rfc2782 allows for srv records with port numbers >= 0.

More specifically, https://www.rfc-editor.org/rfc/rfc6763.html states

If a device does not implement the flagship protocol, then it instead creates a placeholder SRV record (priority=0, weight=0, port=0, target host = host name of device) with that name. If, when it attempts to create this SRV record, it finds that a record with the same name already exists, then it knows that this name is already taken by some other entity implementing at least one of the protocols from the fleet, and it must choose another. If no SRV record already exists, then the act of creating it stakes a claim to that name so that future devices in the same protocol fleet will detect a conflict when they try to use it.

In my case, I'm trying to implement an IPP Everywhere server, the spec for which states

Printers that support DNS-SD MUST advertise the "_printer._tcp" (LPD) service over mDNS in order to conform to the Flagship Naming requirements as defined in [RFC6763]. For example, a Printer named "Example Printer" would advertise the service instance name "Example Printer._printer._tcp.local." with a port number of 0 to indicate that the LPD protocol is not actually supported.

So.. any chance of removing the port != 0 check? :)

Cheers again,
Jarrad

@akdor1154
Copy link
Author

just reading through other PRs and now understand the internal nature of this library, I'd also missed that in my readme skimming. Accept/close as you see fit :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant