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: correct handling of IPv6 addresses with scope_id in ServiceInfo #1322

Merged
merged 3 commits into from Dec 10, 2023

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented Dec 10, 2023

The scope_id was missing because it should come from the DNSAddress record and not the object itself if the ServiceInfo is pulling from the cache. This also lead to duplicate IPv6 records being returned because the scope id was missing

fixes #1321
fixes #1288

scope_id only works on python 3.9+ since < 3.9 doesn't support it

Copy link

codecov bot commented Dec 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a200842) 99.77% compared to head (fba7272) 99.77%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1322   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          29       29           
  Lines        3087     3104   +17     
  Branches      518      521    +3     
=======================================
+ Hits         3080     3097   +17     
  Misses          5        5           
  Partials        2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bdraco bdraco changed the title Fix handling of IPv6 addresses with scope_id in ServiceInfo fix: correct handling of IPv6 addresses with scope_id in ServiceInfo Dec 10, 2023
@bdraco
Copy link
Member Author

bdraco commented Dec 10, 2023

That test is failing because there is a precision loss with the float to double conversion.
We should make all the floats doubles ..but for another PR

@bdraco bdraco merged commit 1682991 into master Dec 10, 2023
30 of 31 checks passed
@bdraco bdraco deleted the fix_dupe_ipv6 branch December 10, 2023 20:38
@twslankard
Copy link

@bdraco thanks for adding this fix! I was hoping to learn more about this:

scope_id only works on python 3.9+ since < 3.9 doesn't support it

Do you have a reference for this? I'm trying to find out whether it's possible to get the scope id in Python 3.8 some other way and haven't had much luck finding more info yet.

@bdraco
Copy link
Member Author

bdraco commented Feb 3, 2024

https://docs.python.org/3.8/library/ipaddress.html -- scope_id is missing here

https://docs.python.org/3.9/library/ipaddress.html -- scope_id is present here

@twslankard
Copy link

Thanks @bdraco !

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.

Duplicate IPv6 addresses end up in ServiceInfo parsed_scoped_addresses does not return with scope_id
2 participants