8000 Add New Hint in AddrInfo to send back QueryId in getaddrinfo by arnnav · Pull Request #961 · c-ares/c-ares · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add New Hint in AddrInfo to send back QueryId in getaddrinfo #961

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add New Hint in AddrInfo to send back QueryId in getaddrinfo #961

wants to merge 2 commits into from

Conversation

arnnav
Copy link
@arnnav arnnav commented Jan 16, 2025

Adding a new hint in ares_addrinfo_hints which will enable sending back query id as part of the ares_addrinfo in the call back of getaddrinfo .

@bradh352
Copy link
Member

a few comments here.

  1. You can't modify the hints structure since a newer c-ares version would try to read out of bounds if the app was originally linked against an older version. This breaks ABI. Adding a new ai_flag would accomplish the same thing without ABI breakage.
  2. qid is 16 bit, so shouldn't use int ... though that may not be the best thing to use in the first place.
  3. maybe it would be better to optionally attach the returned dnsrec entries to the addrinfo result, that would contain the qid obviously as well as other metadata not typically present that might be desirable (such as the 'additional' records that may be attached)
  4. @bagder recently asked (on the mailing list) about adding the ability to know if the result was served out of /etc/hosts vs a query, so if requesting the full record be attached and there wasn't one, that would indicate it was served via /etc/hosts.

@arnnav
Copy link
Author
arnnav commented Jan 21, 2025

Thanks @bradh352 for the comments. If I understand correctly, your comments are suggesting the following:

  1. No modification to the ares_addrinfo_hints structure.
  2. Do not make qid part of the ares_addrinfo structure.
  3. Update the ares_addrinfo structure to have the dnsrec entires (dnsrec_a, dnsrec_aaaa). Pass that back as part of the addrinfo result.
  4. Based on the family specified in the ares_addrinfo_hints, the existence of dnsrec entries can determine if the requests were served out of /etc/hosts or a query.

I will go ahead and make these changes.
Please let me know if I missed something or understood incorrectly.
Thanks!

@bradh352
Copy link
Member

I think the missing thing is adding a new ai_flag to indicate if the dnsrec_a and dnsrec_aaaa should be attached to the answer or not. Don't know if it makes sense to always attach ... I'd need to look at it closer I guess, I don't recall if it would be necessary to duplicate the response into the structure or if we can just attach it to the structure as a way to delay its destruction.

@arnnav
Copy link
Author
arnnav commented Jan 21, 2025

I think adding a new ai_flag can be beneficial, if the app is not interested in the dnsrec entries then c-ares should not be passing that information back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@arnnav @bradh352
0