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

Parse IP addresses from JSON-like strings to allow better IPv6 address handling #514

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

uzlonewolf
Copy link
Contributor

This will allow quoted "::" and "[::]" style IPv6 addresses. Combined with a small change to stubby this will close both #358 and stubby#304

@wtoorop
Copy link
Contributor

wtoorop commented Dec 6, 2021

Thanks @uzlonewolf , but I think this approach might be a bit too generic. Any text string looking like an IP address (even as rdata field in for example a TXT RR) will be converted to an IP dict with this PR.
Having that said, I do like to approach of an IP address nested in a string and I do think this would indeed be the correct way to address getdnsapi/stubby#304 . So I'm thinking maybe we can adapt getdns_context_set_upstream_recursive_servers() to accept strings in the list and convert it to addresses. Likewise for the other settings that accept IP address dicts (getdns_context_set_dns_root_servers() and getdns_context_set_listen_addresses()). WDYT?

@uzlonewolf
Copy link
Contributor Author

Yes, I do like that idea a lot better. I'll try to modify this PR to do that in the next day or 2.

@wtoorop
Copy link
Contributor

wtoorop commented Dec 6, 2021 via email

@uzlonewolf
Copy link
Contributor Author

uzlonewolf commented Dec 7, 2021

Well, after spending a couple hours on this and successfully making getdns_context_set_listen_addresses() accept an IP string, I'm still not happy with the results. It's most likely going to be a breaking change to anyone using the API and passing in an IP address as a binary string. It's probably fine for the listen addresses since those most likely include a port number and thus get passed as a dict, however the getdns_context_set_..._servers() functions are likely to have been used with binary strings.

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.

2 participants