starred/hickory-dns

Fork 0

mirror of https://github.com/hickory-dns/hickory-dns.git synced 2026-04-25 11:15:54 +03:00

[GH-ISSUE #320] Ability to create invalid records #146

New issue

Closed

opened 2026-03-07 22:27:53 +03:00 by kerem · 4 comments

kerem commented

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

Owner

Originally created by @vorner on GitHub (Dec 18, 2017).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/320

Hello

I noticed it is possible to create invalid record. Eg, this compiles (and runs), but produces invalid data when serialized (claims to be A, but has too much bytes).

let mut record = Record::with(name, RecordType::A, 3600);
record.set_rdata(RData::AAAA("db8::1".parse().unwrap());

Does it make sense to be able to create a record with different rdata than its type? Does it make sense to create an „empty“ record? Wouldn't it be better to be able to create only valid records (directly fill them with rdata, derive the record type from the rdata, etc).

Originally created by @vorner on GitHub (Dec 18, 2017). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/320 Hello I noticed it is possible to create invalid record. Eg, this compiles (and runs), but produces invalid data when serialized (claims to be A, but has too much bytes). ```rust let mut record = Record::with(name, RecordType::A, 3600); record.set_rdata(RData::AAAA("db8::1".parse().unwrap()); ``` Does it make sense to be able to create a record with different rdata than its type? Does it make sense to create an „empty“ record? Wouldn't it be better to be able to create only valid records (directly fill them with rdata, derive the record type from the rdata, etc).

kerem

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

closed this issue
added the
bug

crate:proto

cleanup
labels

kerem commented

2026-03-07 22:28:09 +03:00

Author

Owner

@bluejekyll commented on GitHub (Dec 18, 2017):

Does it make sense to be able to create a record with different rdata than its type?

There was a point where I was reusing common RData (like Name) across different RecordTypes, where this would have been valid. I've since moved away from that and started explicitly using separate RData types, with single values as the actual data section of the RData type. So there should be (and I think there is now) a 1-1 relationship between RecordType and RData. We could hide RecordType entirely I think at this point. RData does have a method to get the RecordType from it.

Does it make sense to create an „empty“ record?

Depends on what you mean by empty. There are cases where the Null RData type is valid, but they are mostly edge conditions.

Wouldn't it be better to be able to create only valid records?

Yes. We should definitely add some validation on the RData to restrict it to RecordType. At one point I toyed with trying to make a generic type parameter to catch this at compile time, but I don't think it's easily done without adding more intermediate types. So yes, I think we could drop RecordType from the Record constructor all together, possibly each from the Record itself, and fix locations as necessary.

@bluejekyll commented on GitHub (Dec 18, 2017): > Does it make sense to be able to create a record with different rdata than its type? There was a point where I was reusing common RData (like Name) across different RecordTypes, where this would have been valid. I've since moved away from that and started explicitly using separate RData types, with single values as the actual data section of the RData type. So there should be (and I think there is now) a 1-1 relationship between RecordType and RData. We could hide RecordType entirely I think at this point. RData does have a method to get the RecordType from it. > Does it make sense to create an „empty“ record? Depends on what you mean by empty. There are cases where the `Null` RData type is valid, but they are mostly edge conditions. > Wouldn't it be better to be able to create only valid records? Yes. We should definitely add some validation on the RData to restrict it to RecordType. At one point I toyed with trying to make a generic type parameter to catch this at compile time, but I don't think it's easily done without adding more intermediate types. So yes, I think we could drop RecordType from the Record constructor all together, possibly each from the Record itself, and fix locations as necessary.

kerem commented

2026-03-07 22:28:15 +03:00

Author

Owner

@vorner commented on GitHub (Dec 18, 2017):

Depends on what you mean by empty. There are cases where the Null RData type is valid, but they are mostly edge conditions.

I meant whatever is created by the new() constructor. Yes, there are some defaults (but why empty name, for example?).

What I was thinking would be to have to pass all the information at construction time. That way it wouldn't be easy to create „broken“ ones. But maybe I'm missing some edge case.

@vorner commented on GitHub (Dec 18, 2017): > Depends on what you mean by empty. There are cases where the Null RData type is valid, but they are mostly edge conditions. I meant whatever is created by the `new()` constructor. Yes, there are some defaults (but why empty name, for example?). What I was thinking would be to have to pass all the information at construction time. That way it wouldn't be easy to create „broken“ ones. But maybe I'm missing some edge case.

kerem commented

2026-03-07 22:28:20 +03:00

Author

Owner

@bluejekyll commented on GitHub (Dec 18, 2017):

What I was thinking would be to have to pass all the information at construction time. That way it wouldn't be easy to create „broken“ ones. But maybe I'm missing some edge case.

Yes, I think this would be a good solution. If I were writing this today, I might opt to do that. When I wrote it originally, I don't think all the potential use cases were obvious to me at the time, so I opted for more flexibility. I'm all for restricting this more now, and requiring new Records to be created if you want to change shape...

@bluejekyll commented on GitHub (Dec 18, 2017): > What I was thinking would be to have to pass all the information at construction time. That way it wouldn't be easy to create „broken“ ones. But maybe I'm missing some edge case. Yes, I think this would be a good solution. If I were writing this today, I might opt to do that. When I wrote it originally, I don't think all the potential use cases were obvious to me at the time, so I opted for more flexibility. I'm all for restricting this more now, and requiring new Records to be created if you want to change shape...

kerem commented

2026-03-07 22:28:25 +03:00

Author

Owner

@bluejekyll commented on GitHub (Apr 9, 2019):

This was fixed in #674

@bluejekyll commented on GitHub (Apr 9, 2019): This was fixed in #674

kerem referenced this issue

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

[PR #146] [MERGED] v0.10.3 #1266