[GH-ISSUE #1202] Design of `ResponseCode::high` #626

New issue

Closed

opened 2026-03-15 23:32:05 +03:00 by kerem · 3 comments

kerem commented

2026-03-15 23:32:05 +03:00

Owner

Originally created by @jonasbb on GitHub (Sep 15, 2020).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1202

Currently, ResponseCode::high is defined as (self) -> u16. For real DNS response codes it only contains 8 bit of information.
You can see that in ResponseCode::from(high: u8, low: u8) -> ResponseCode and in Edns::set_rcode_high(&mut self, rcode_high: u8), which both take the high as u8 instead of u16.

When first introduced the function was broken and returned (). That changes in github.com/bluejekyll/trust-dns@66f65ff6aa (diff-f9f3cd8215) which set the return type to u16.

I think changing the function signature to return u8 would make sense as 8-bit are the only semantically meaningful ones and it would integrate better with the other two functions.

A reason to keep the current return code would be for ResponseCode::Unknown which stores a u16 internally.

Originally created by @jonasbb on GitHub (Sep 15, 2020). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1202 Currently, `ResponseCode::high` is defined as `(self) -> u16`. For real DNS response codes it only contains 8 bit of information. You can see that in `ResponseCode::from(high: u8, low: u8) -> ResponseCode` and in `Edns::set_rcode_high(&mut self, rcode_high: u8)`, which both take the `high` as u8 instead of u16. When first introduced the function was broken and returned `()`. That changes in https://github.com/bluejekyll/trust-dns/commit/66f65ff6aaacb56cfbed80bc67731f83e958e94e#diff-f9f3cd8215b9fd27faa1d1ce3c9d2992R137 which set the return type to u16. I think changing the function signature to return u8 would make sense as 8-bit are the only semantically meaningful ones and it would integrate better with the other two functions. A reason to keep the current return code would be for `ResponseCode::Unknown` which stores a u16 internally.

kerem

2026-03-15 23:32:05 +03:00

closed this issue
added the
crate:proto
label

kerem commented

2026-03-15 23:32:21 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 15, 2020):

Are you seeing issues with the current form? I agree that in almost all uses of this, we're using this as an 8bit value. It looks like we probably need more documentation in a lot of these areas, as it's not always clear that the standard only allows for low value of 4 bits...

One reason not to make this change, is that it's possible someone could send 12bits intentionally, and we would not be able to store that if we change this to a u8.

@bluejekyll commented on GitHub (Sep 15, 2020): Are you seeing issues with the current form? I agree that in almost all uses of this, we're using this as an 8bit value. It looks like we probably need more documentation in a lot of these areas, as it's not always clear that the standard only allows for low value of 4 bits... One reason not to make this change, is that it's possible someone could send 12bits intentionally, and we would not be able to store that if we change this to a u8.

kerem commented

2026-03-15 23:32:26 +03:00

Author

Owner

@jonasbb commented on GitHub (Sep 15, 2020):

Overall it is not really a big problem. It forces casting to u8 basically everywhere you want to use the value, since the consumers (like from and set_rcode_high) only take u8.

One reason not to make this change, is that it's possible someone could send 12bits intentionally, and we would not be able to store that if we change this to a u8.

Sorry, I'm not sure I quite understand what you are referring with the 12 bits to. Are you referring to the 12 meaningful bits returned by the high function or to the 12-bits of response code which can be embedded into a DNS message with EDNS record?

I don't think limiting the high function to u8 is really a problem. The 4 bit which are now not returned anymore can never occur in wire format DNS messages. It seems the only way to set the upper 4 bit of ResponseCode are via the public ResponseCode::Unknown and the From<u16> implementation.

I also wanted to ask about this, since I couldn't find a reason why high returns a u16. As already said, the function was initially broken and got fixed in a refactoring PR which only mentions clippy fixes.
So this might have just been overlooked.

@jonasbb commented on GitHub (Sep 15, 2020): Overall it is not really a big problem. It forces casting to u8 basically everywhere you want to use the value, since the consumers (like `from` and `set_rcode_high`) only take u8. > One reason not to make this change, is that it's possible someone could send 12bits intentionally, and we would not be able to store that if we change this to a u8. Sorry, I'm not sure I quite understand what you are referring with the 12 bits to. Are you referring to the 12 meaningful bits returned by the `high` function or to the 12-bits of response code which can be embedded into a DNS message with EDNS record? I don't think limiting the `high` function to u8 is really a problem. The 4 bit which are now not returned anymore can never occur in wire format DNS messages. It seems the only way to set the upper 4 bit of `ResponseCode` are via the public `ResponseCode::Unknown` and the `From<u16>` implementation. --- I also wanted to ask about this, since I couldn't find a reason why `high` returns a u16. As already said, the function was initially broken and got fixed in a refactoring PR which only mentions clippy fixes. So this might have just been overlooked.

kerem commented

2026-03-15 23:32:31 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 15, 2020):

I was going by my docs there. I had to go back to the RFC to completely refresh my memory on this one:

https://tools.ietf.org/html/rfc6891#section-6.1.3

Forms the upper 8 bits of extended 12-bit RCODE (together with the
      4 bits defined in [RFC1035].  Note that EXTENDED-RCODE value 0
      indicates that an unextended RCODE is in use (values 0 through
      15).

It's been a while since I've looked at this. You're right, the high-order bits are only 8 bits, my documentation there is inaccurate. So yes, we can change that return value to u8. I don't know why it's u16 now, but I'm guessing that was to make some of the bit shifting require fewer casts.

@bluejekyll commented on GitHub (Sep 15, 2020): I was going by my docs there. I had to go back to the RFC to completely refresh my memory on this one: https://tools.ietf.org/html/rfc6891#section-6.1.3 ```text Forms the upper 8 bits of extended 12-bit RCODE (together with the 4 bits defined in [RFC1035]. Note that EXTENDED-RCODE value 0 indicates that an unextended RCODE is in use (values 0 through 15). ``` It's been a while since I've looked at this. You're right, the high-order bits are only 8 bits, my documentation there is inaccurate. So yes, we can change that return value to u8. I don't know why it's u16 now, but I'm guessing that was to make some of the bit shifting require fewer casts.

kerem referenced this issue

2026-03-16 02:13:16 +03:00

[PR #626] [MERGED] upgrade rand from 0.5 to 0.6 #1565