Privatize EADItem details, increase label range to i16 #100
Conversation
chrysn
force-pushed
the
api-enhancements
branch
from
f4216a8 to
57a284a
Compare
consts/src/lib.rs
Outdated
| fn new(label: u16, is_critical: bool, value: Option<&[u8]>) -> Result<Self, EADNewError> { | ||
| let mut label: i16 = label | ||
| .try_into() | ||
| .map_err(|_| EADNewError::InexpressibleLabel)?; |
There was a problem hiding this comment.
I think ? (and early returns in general?) is currently not supported by cargo hax into fstar, @W95Psp could you please confirm?
There was a problem hiding this comment.
No, that should be fine, ? and early returns are supported (they simply produce a bit heavyweight F*)
There was a problem hiding this comment.
Great, thanks for the clarification!
consts/src/lib.rs
Outdated
| @@ -88,30 +88,71 @@ mod common { | |||
|
|
|||
| #[derive(Debug)] | |||
| pub struct EADItem { | |||
| pub label: u8, | |||
| pub is_critical: bool, | |||
| pub(crate) label: i16, | |||
There was a problem hiding this comment.
If we are modifying label to support larger ranges, I think we could go ahead and use i32 to support the full specified range for EAD labels (0 to 65536).
There was a problem hiding this comment.
I thought there was no limit in EDHOC -22, but indeed the registry range implies that it's u16 / i17.
The choice between u16+bool and i32 is an implementation detail since the first commit, I think I'd go with the u16+bool version in this case (seeing the CBOR is done manually in edhoc.rs'sencode_ead_item); this can be revisited if CBOR in here ever gets outsourced to a CBOR library.
|
Thanks for this PR @chrysn. Left a few comments in the code. I assume you were waiting for a feedback before adjusting other impacted areas of the library. |
BREAKING CHANGE: EADItem's constructor changed, and its fields became private.
chrysn
force-pushed
the
api-enhancements
branch
from
57a284a to
0160855
Compare
This increases the usable size of EAD labels to +-65535. It also adds a bit of CBOR processing, to the point where it may make sense to use a proper CBOR library instead; that is likely most easily achieved after completion of [105]. [105]: openwsn-berkeley#105
chrysn
force-pushed
the
api-enhancements
branch
from
0160855 to
6f730cb
Compare
While it has practical uses (I'd like to utilize the library with https://datatracker.ietf.org/doc/draft-amsuess-core-edhoc-grease/, although that'll need the u16 or practically i32 points), its main purpose is to check whether my understanding of the code is good enough to pass CI.
I'd still ask for reviews towards merging because