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

fix PTR expiration preventing later service resolution #140

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

Mornix
Copy link
Contributor

@Mornix Mornix commented Oct 22, 2023

Hi,

I ran across this issue while using mdns-sd when browsing for a specific service domain. The bug requires the following message pattern to manifest:

  1. Remote end sends an announcement (PTR+SRV+TXT) -- mdns-sd correctly emits ServiceFound and ServiceResolved
  2. Remote end sends an invalidation for only the PTR (setting TTL to zero) -- mdns-sd correctly emits ServiceRemoved
  3. Remote end sends an announcement (PTR+SRV+TXT) before any of the SRV, TXT, A, or AAAA records expire -- mdns-sd emits ServiceFound but does not emit ServiceResolved

From my understanding of the code, it looks like it assumes that a new SRV or TXT (or another non-PTR) record will be received after a new PTR relying on that to trigger the block responsible for resolving the service.

I've added a test (in the place of least resistance) that reproduces this behaviour.

@Mornix Mornix changed the title fix PTR expiration from preventing later service resolution fix PTR expiration preventing later service resolution Oct 22, 2023
Copy link
Owner

@keepsimple1 keepsimple1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! That's a good fix for a subtle case!

@keepsimple1 keepsimple1 merged commit 3d075a7 into keepsimple1:main Oct 23, 2023
3 checks passed
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.

None yet

2 participants