starred/hickory-dns

Fork 0

mirror of https://github.com/hickory-dns/hickory-dns.git synced 2026-04-25 03:05:51 +03:00

[GH-ISSUE #370] Passing TCP or UDP client as an argument #170

New issue

Open

opened 2026-03-07 22:37:46 +03:00 by kerem · 10 comments

kerem commented

2026-03-07 22:37:46 +03:00

Owner

Originally created by @msehnout on GitHub (Apr 3, 2018).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/370

How can I specify a function, that takes either UDP or TCP client? I have a SyncClient with UdpClientConnection written like this:

fn support_udp_answers(client: SyncClient<trust_dns::udp::UdpClientConnection>) -> TestResult {
    let name = Name::from_str("www.example.com.").unwrap();
    if let Ok(_) = client.query(&name, DNSClass::IN, RecordType::A) {
        TestResult::Success
    } else {
        TestResult::Fail
    }
}

fn main() {
    let address = "8.8.8.8:53".parse().unwrap();
    let conn = UdpClientConnection::new(address).unwrap();
    let client = SyncClient::new(conn);
    println!("Support UDP answers: {:?}", support_udp_answers(client));
}

but I would like to it accept either UdpClientConnection or TcpClientConnection. I tried to use Client as a trait bound, but I failed to compile my code. Can you help me with this?

Originally created by @msehnout on GitHub (Apr 3, 2018). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/370 How can I specify a function, that takes either UDP or TCP client? I have a `SyncClient` with `UdpClientConnection` written like this: ```rust fn support_udp_answers(client: SyncClient<trust_dns::udp::UdpClientConnection>) -> TestResult { let name = Name::from_str("www.example.com.").unwrap(); if let Ok(_) = client.query(&name, DNSClass::IN, RecordType::A) { TestResult::Success } else { TestResult::Fail } } fn main() { let address = "8.8.8.8:53".parse().unwrap(); let conn = UdpClientConnection::new(address).unwrap(); let client = SyncClient::new(conn); println!("Support UDP answers: {:?}", support_udp_answers(client)); } ``` but I would like to it accept either `UdpClientConnection` or `TcpClientConnection`. I tried to use Client as a trait bound, but I failed to compile my code. Can you help me with this?

kerem added the

docs

label

2026-03-07 22:37:46 +03:00

kerem commented

2026-03-07 22:37:57 +03:00

Author

Owner

@msehnout commented on GitHub (Apr 3, 2018):

edit: I found your blog post: http://bluejekyll.github.io/blog/rust/2017/08/06/type-parameters.html and managed to do it, but it was tough :-)

fn support_simple_answers<CC>(client: SyncClient<CC>) -> TestResult 
  where CC: ClientConnection,
        <CC as ClientConnection>::MessageStream: Stream<Item=Vec<u8>, Error=io::Error> + 'static
{
    let name = Name::from_str("www.example.com.").unwrap();
    if let Ok(_) = client.query(&name, DNSClass::IN, RecordType::A) {
        TestResult::Success
    } else {
        TestResult::Fail
    }
}

fn main() {
    let address = "8.8.8.8:53".parse().unwrap();
    
    let connection = UdpClientConnection::new(address).unwrap();
    let client = SyncClient::new(connection);
    println!("Support UDP answers: {:?}", support_simple_answers(client));

    let connection = TcpClientConnection::new(address).unwrap();
    let client = SyncClient::new(connection);
    println!("Support TCP answers: {:?}", support_simple_answers(client));
}

@msehnout commented on GitHub (Apr 3, 2018): edit: I found your blog post: http://bluejekyll.github.io/blog/rust/2017/08/06/type-parameters.html and managed to do it, but it was tough :-) ```rust fn support_simple_answers<CC>(client: SyncClient<CC>) -> TestResult where CC: ClientConnection, <CC as ClientConnection>::MessageStream: Stream<Item=Vec<u8>, Error=io::Error> + 'static { let name = Name::from_str("www.example.com.").unwrap(); if let Ok(_) = client.query(&name, DNSClass::IN, RecordType::A) { TestResult::Success } else { TestResult::Fail } } fn main() { let address = "8.8.8.8:53".parse().unwrap(); let connection = UdpClientConnection::new(address).unwrap(); let client = SyncClient::new(connection); println!("Support UDP answers: {:?}", support_simple_answers(client)); let connection = TcpClientConnection::new(address).unwrap(); let client = SyncClient::new(connection); println!("Support TCP answers: {:?}", support_simple_answers(client)); } ```

kerem commented

2026-03-07 22:38:02 +03:00

Author

Owner

@bluejekyll commented on GitHub (Apr 3, 2018):

Glad you worked through it! Yes these interfaces need to be simplified. The traits are confusing at the moment, and their type parameters as well.

Out of curiosity, did you look at the Resolver which does this implicitly?

@bluejekyll commented on GitHub (Apr 3, 2018): Glad you worked through it! Yes these interfaces need to be simplified. The traits are confusing at the moment, and their type parameters as well. Out of curiosity, did you look at the Resolver which does this implicitly?

kerem commented

2026-03-07 22:38:07 +03:00

Author

Owner

@msehnout commented on GitHub (Apr 4, 2018):

I looked into the code of the Resolver for inspiration :-) , but I need direct control over the underlying protocol. I'd like to implement some test as defined in RFC 8027

@msehnout commented on GitHub (Apr 4, 2018): I looked into the code of the Resolver for inspiration :-) , but I need direct control over the underlying protocol. I'd like to implement some test as defined in [RFC 8027](https://tools.ietf.org/html/rfc8027)

kerem commented

2026-03-07 22:38:12 +03:00

Author

Owner

@bluejekyll commented on GitHub (Apr 4, 2018):

Oh, that would be a cool tool. Yeah, that's a good use for the client. At some point it would be great to clean the library up and make some of these interfaces simpler, or at least more straight-forward idiomatic Rust. Let me know if you run into any issues.

@bluejekyll commented on GitHub (Apr 4, 2018): Oh, that would be a cool tool. Yeah, that's a good use for the client. At some point it would be great to clean the library up and make some of these interfaces simpler, or at least more straight-forward idiomatic Rust. Let me know if you run into any issues.

kerem commented

2026-03-07 22:38:17 +03:00

Author

Owner

@msehnout commented on GitHub (Apr 7, 2018):

I tried to implement the third test, EDNS0 support. I had to create the DNS message by hand and then send it manually. It took me some time to find out where to put the message to get response back. Eventually I copied&pasted some code from the resolver and client crates a managed to implement the test.

One issue I had, is that the DnsHandle trait is available only from the trust-dns-proto crate, which from my understanding is not supposed to be used directly.
Import: https://github.com/msehnout/dns-roadblock-tests/blob/master/src/main.rs#L19
Usage: https://github.com/msehnout/dns-roadblock-tests/blob/master/src/main.rs#L100

The second thing I don't understand is this part:
https://github.com/msehnout/dns-roadblock-tests/blob/master/src/main.rs#L98

    let (stream, stream_handle) = conn.new_stream(handle).unwrap();
    let mut client_handle = ClientFuture::new(stream, stream_handle, handle, None);
    let response_future = client_handle.send(msg);

What is the purpose of creating stream and stream_handle? And why do I pass the handle to the ClientFuture, when eventually I pass the response_future to the reactor?

@msehnout commented on GitHub (Apr 7, 2018): I tried to implement the third test, EDNS0 support. I had to create the DNS message by hand and then send it manually. It took me some time to find out where to put the message to get response back. Eventually I copied&pasted some code from the resolver and client crates a managed to implement the test. One issue I had, is that the DnsHandle trait is available only from the trust-dns-proto crate, which from my understanding is not supposed to be used directly. Import: https://github.com/msehnout/dns-roadblock-tests/blob/master/src/main.rs#L19 Usage: https://github.com/msehnout/dns-roadblock-tests/blob/master/src/main.rs#L100 The second thing I don't understand is this part: https://github.com/msehnout/dns-roadblock-tests/blob/master/src/main.rs#L98 ```rust let (stream, stream_handle) = conn.new_stream(handle).unwrap(); let mut client_handle = ClientFuture::new(stream, stream_handle, handle, None); let response_future = client_handle.send(msg); ``` What is the purpose of creating stream and stream_handle? And why do I pass the handle to the ClientFuture, when eventually I pass the response_future to the reactor?

kerem commented

2026-03-07 22:38:22 +03:00

Author

Owner

@bluejekyll commented on GitHub (Apr 7, 2018):

We can clean up the new_stream() interface so that this is a little easier for external implementations like this. A single combined type as the result would do that. I haven't spent much time cleaning things like this up. The reason there are two, is that DNS supports out of order message sending and delivery. So we can't have a simple high-level interface over the stream for sending/receiving. If you want to file an issue for cleaning up that interface please do. The stream is the i/o processor, whereas the handle is the thing which you use to send messages to the stream. This shouldn't be something your library cares about, so simplifying that to a single type makes a lot of sense.

If DnsHandle needs to be public to support your use-case, I'm fine with us doing that. I was generally trying to steer people away from some of this complexity. Also, for anything where you needed to directly pull in trust_dns_proto, we should reexport that in the trust-dns crate.

@bluejekyll commented on GitHub (Apr 7, 2018): We can clean up the `new_stream()` interface so that this is a little easier for external implementations like this. A single combined type as the result would do that. I haven't spent much time cleaning things like this up. The reason there are two, is that DNS supports out of order message sending and delivery. So we can't have a simple high-level interface over the stream for sending/receiving. If you want to file an issue for cleaning up that interface please do. The `stream` is the i/o processor, whereas the `handle` is the thing which you use to send messages to the `stream`. This shouldn't be something your library cares about, so simplifying that to a single type makes a lot of sense. If `DnsHandle` needs to be public to support your use-case, I'm fine with us doing that. I was generally trying to steer people away from some of this complexity. Also, for anything where you needed to directly pull in trust_dns_proto, we should reexport that in the `trust-dns` crate.

kerem commented

2026-03-07 22:38:27 +03:00

Author

Owner

@msehnout commented on GitHub (Apr 12, 2018):

Eventually I switched to using DnsHandle as an argument instead of the SyncClient. I would also like to ask about the message composition together with Edns, am I using it correctly here?
https://github.com/msehnout/dns-roadblock-tests/blob/master/src/main.rs#L52

    // Create a query
    let query = Query::query(TESTING_SERVER.clone(), RecordType::A);
    // Create an EDNS struct
    let mut edns = Edns::new();
    let v = vec![];
    edns.set_option(EdnsOption::from((EdnsCode::Zero, &v[..])));
    // Finally, assemble a message
    let mut msg = Message::new();
    msg.add_query(query);
    msg.set_edns(edns);

Because when I run those tests against Google public DNS, Cloudflare DNS and my local Unbound resolver, the local resolver always return with "Refused"

@msehnout commented on GitHub (Apr 12, 2018): Eventually I switched to using `DnsHandle` as an argument instead of the `SyncClient`. I would also like to ask about the message composition together with Edns, am I using it correctly here? https://github.com/msehnout/dns-roadblock-tests/blob/master/src/main.rs#L52 ```rust // Create a query let query = Query::query(TESTING_SERVER.clone(), RecordType::A); // Create an EDNS struct let mut edns = Edns::new(); let v = vec![]; edns.set_option(EdnsOption::from((EdnsCode::Zero, &v[..]))); // Finally, assemble a message let mut msg = Message::new(); msg.add_query(query); msg.set_edns(edns); ``` Because when I run those tests against Google public DNS, Cloudflare DNS and my local Unbound resolver, the local resolver always return with "Refused"

kerem commented

2026-03-07 22:38:32 +03:00

Author

Owner

@bluejekyll commented on GitHub (Apr 12, 2018):

That construction looks ok. I think the issue is the EdnsCode::Zero usage, we may want to disallow use of that parameter. Here's a list of the valid edns codes: https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-11, Zero is reserved, and as far as I know not used for anything.

The upstream resolvers may be rejecting the DNS message b/c of that.

@bluejekyll commented on GitHub (Apr 12, 2018): That construction looks ok. I think the issue is the `EdnsCode::Zero` usage, we may want to disallow use of that parameter. Here's a list of the valid edns codes: https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-11, Zero is reserved, and as far as I know not used for anything. The upstream resolvers may be rejecting the DNS message b/c of that.

kerem commented

2026-03-07 22:38:37 +03:00

Author

Owner

@bluejekyll commented on GitHub (Apr 13, 2018):

Reading you're question again. I wonder if there's some confusion here. EDNS(0) is the name of the current RFC on EDNS, it does not mean an EDNS with an associated EdnsOption of the code EdnsCode::Zero, it is actually referring to the edns.version(), in EDNS(0), where the version is 0, 0 specifying that the library is in full compliance with the RFC.

https://tools.ietf.org/html/rfc6891

@bluejekyll commented on GitHub (Apr 13, 2018): Reading you're question again. I wonder if there's some confusion here. `EDNS(0)` is the name of the current RFC on `EDNS`, it does not mean an `EDNS` with an associated `EdnsOption` of the code `EdnsCode::Zero`, it is actually referring to the `edns.version()`, in `EDNS(0)`, where the version is `0`, `0` specifying that the library is in full compliance with the RFC. https://tools.ietf.org/html/rfc6891

kerem commented

2026-03-07 22:38:42 +03:00

Author

Owner

@msehnout commented on GitHub (Apr 13, 2018):

Yes, it seems, that I did not understand the test definition:
https://tools.ietf.org/html/rfc8027#section-3.1.3

Test: Send a request to the resolver under test for an A record for a
known existing domain, such as good-a.test.example.com, with an EDNS0
OPT record in the additional section.

So I guess, this line:

edns.set_option(EdnsOption::from((EdnsCode::Zero, &v[..])));

should by replaced with this:

edns.set_version(0);

Resulting in this message:

    // Create a query
    let query = Query::query(TESTING_SERVER.clone(), RecordType::A);
    // Create an EDNS struct
    let mut edns = Edns::new();
    edns.set_version(0);
    // Finally, assemble a message
    let mut msg = Message::new();
    msg.add_query(query);
    msg.set_edns(edns);

@msehnout commented on GitHub (Apr 13, 2018): Yes, it seems, that I did not understand the test definition: https://tools.ietf.org/html/rfc8027#section-3.1.3 ``` Test: Send a request to the resolver under test for an A record for a known existing domain, such as good-a.test.example.com, with an EDNS0 OPT record in the additional section. ``` So I guess, this line: ```rust edns.set_option(EdnsOption::from((EdnsCode::Zero, &v[..]))); ``` should by replaced with this: ```rust edns.set_version(0); ``` Resulting in this message: ```rust // Create a query let query = Query::query(TESTING_SERVER.clone(), RecordType::A); // Create an EDNS struct let mut edns = Edns::new(); edns.set_version(0); // Finally, assemble a message let mut msg = Message::new(); msg.add_query(query); msg.set_edns(edns); ```

kerem referenced this issue

2026-03-16 01:57:48 +03:00

[PR #170] [MERGED] C0.11.0 r0.3.0 #1278