[GH-ISSUE #599] How to determine the Query that resolves? #244

Closed
opened 2026-03-07 22:58:56 +03:00 by kerem · 5 comments
Owner

Originally created by @olix0r on GitHub (Nov 2, 2018).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/599

What's the best way to perform proper search-based resolution such that it's possible to know the query that produces a result?

For example, suppose we're resolving an ambiguous name like foo.bar with an /etc/resolv.conf including search a.b c.d e.f

Is there a lookup-like call that produces a result that indicates which of the following names was resolved: foo.bar.a.b., foo.bar.c.d., foo.bar.e.f., or foo.bar.?

Another way to phrase this question: if I wanted to implement dig(1) with trust-dns-resolver, how would I implement something that could take an input like foo.bar and output something like:

; <<>> DiG 9.9.5-9+deb8u15-Debian <<>> +search foo.bar
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 30062
;; flags: qr aa rd ra; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 0

;; QUESTION SECTION:
;foo.bar.c.d.	IN A

;; ANSWER SECTION:
foo.bar.c.d.	30 IN A	10.233.68.175
Originally created by @olix0r on GitHub (Nov 2, 2018). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/599 What's the best way to perform proper search-based resolution such that it's possible to know the query that produces a result? For example, suppose we're resolving an ambiguous name like `foo.bar` with an /etc/resolv.conf including `search a.b c.d e.f` Is there a lookup-like call that produces a result that indicates which of the following names was resolved: `foo.bar.a.b.`, `foo.bar.c.d.`, `foo.bar.e.f.`, or `foo.bar.`? Another way to phrase this question: if I wanted to implement `dig(1)` with trust-dns-resolver, how would I implement something that could take an input like _foo.bar_ and output something like: ``` ; <<>> DiG 9.9.5-9+deb8u15-Debian <<>> +search foo.bar ;; global options: +cmd ;; Got answer: ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 30062 ;; flags: qr aa rd ra; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 0 ;; QUESTION SECTION: ;foo.bar.c.d. IN A ;; ANSWER SECTION: foo.bar.c.d. 30 IN A 10.233.68.175 ```
Author
Owner

@bluejekyll commented on GitHub (Nov 2, 2018):

This is a great question which I haven't pondered before. But it's a perfectly reasonable thing to want. Currently the Query is constructed based on the search path, and/or happy eyeballs ipv4/ipv6. So there are definitely a few options for the query that's sent.

As of now, this isn't returned, but it wouldn't be hard to add Query to the *Lookup* result types. At that point you'd have access to the Query that was constructed for the request and delivered the record. I think at that point you'd have what you're asking for.

<!-- gh-comment-id:435279572 --> @bluejekyll commented on GitHub (Nov 2, 2018): This is a great question which I haven't pondered before. But it's a perfectly reasonable thing to want. Currently the Query is constructed based on the search path, and/or happy eyeballs ipv4/ipv6. So there are definitely a few options for the query that's sent. As of now, this isn't returned, but it wouldn't be hard to add `Query` to the `*Lookup*` result types. At that point you'd have access to the `Query` that was constructed for the request and delivered the record. I think at that point you'd have what you're asking for.
Author
Owner

@olix0r commented on GitHub (Nov 2, 2018):

@bluejekyll thanks for the quick response. I can take a crack at a PR for this if you'd like.

One question about the implementation: how sensitive are you to breaking API compatibility in the Lookup::new_* APIs?

github.com/bluejekyll/trust-dns@75bc87ddd9/crates/resolver/src/lookup.rs (L37-L58)

If we add a required Query to Lookup, then we'll also need to change the new_* fns to take a Query, which is breaking. Alternatively, we could make it an Option<Query> and make it supported with a new set of constructors. This would be a little unfortunate, because we'll always set a Query (within trustdns at least), and so users will likely have to lookup.query().expect("query must be set").

All that said, this library is still an 0.N version; and it seems atypical for users to instantiate Lookup instances directly, so a breaking change probably isn't so disruptive.

Thoughts?

<!-- gh-comment-id:435387318 --> @olix0r commented on GitHub (Nov 2, 2018): @bluejekyll thanks for the quick response. I can take a crack at a PR for this if you'd like. One question about the implementation: how sensitive are you to breaking API compatibility in the `Lookup::new_*` APIs? https://github.com/bluejekyll/trust-dns/blob/75bc87ddd930cc8eaf42701ad5c512ded2c3aeea/crates/resolver/src/lookup.rs#L37-L58 If we add a required `Query` to `Lookup`, then we'll also need to change the `new_*` `fn`s to take a Query, which is breaking. Alternatively, we could make it an `Option<Query>` and make it supported with a new set of constructors. This would be a little unfortunate, because we'll _always_ set a Query (within trustdns at least), and so users will likely have to `lookup.query().expect("query must be set")`. All that said, this library is still an 0.N version; and it seems atypical for users to instantiate `Lookup` instances directly, so a breaking change probably isn't so disruptive. Thoughts?
Author
Owner

@bluejekyll commented on GitHub (Nov 2, 2018):

For the lookups, I’m not concerned about API breakage. In fact, we could possibly make the new Fn private to the crate, except that I’m pretty sure there are some mocked tests where they’re being used.

I would be surprised if anyone outside the trust-dns crates are relying on these constructors at all, though even if they are, it would be good for them to do this as well.

To be clear, I think a strong ownership of the Query is right, and make it non-optional. If you make the change, That would be greatly appreciated!

<!-- gh-comment-id:435389485 --> @bluejekyll commented on GitHub (Nov 2, 2018): For the lookups, I’m not concerned about API breakage. In fact, we could possibly make the `new` Fn private to the crate, except that I’m pretty sure there are some mocked tests where they’re being used. I would be surprised if anyone outside the trust-dns crates are relying on these constructors at all, though even if they are, it would be good for them to do this as well. To be clear, I think a strong ownership of the Query is right, and make it non-optional. If you make the change, That would be greatly appreciated!
Author
Owner

@olix0r commented on GitHub (Nov 2, 2018):

@bluejekyll great, thanks. In linkerd2-proxy, we tend to do something like

impl Thing {
  pub(super) fn new() -> Self {
    ...
  }
}

#[cfg(test)]
impl Thing {
  pub fn for_test() -> Self {
  }
}

so that we can have private constructors but still expose public constructors to tests.

<!-- gh-comment-id:435390332 --> @olix0r commented on GitHub (Nov 2, 2018): @bluejekyll great, thanks. In linkerd2-proxy, we tend to do something like ```rust impl Thing { pub(super) fn new() -> Self { ... } } #[cfg(test)] impl Thing { pub fn for_test() -> Self { } } ``` so that we can have private constructors but still expose public constructors to tests.
Author
Owner

@bluejekyll commented on GitHub (Nov 2, 2018):

Oh. Duh, that’s a great option! We used to do something similar in Java, this is so much more elegant. Not sure why that pattern hasn’t struck me before. Thanks for sharing!

<!-- gh-comment-id:435391158 --> @bluejekyll commented on GitHub (Nov 2, 2018): Oh. Duh, that’s a great option! We used to do something similar in Java, this is so much more elegant. Not sure why that pattern hasn’t struck me before. Thanks for sharing!
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/hickory-dns#244
No description provided.