-
Notifications
You must be signed in to change notification settings - Fork 82
feat: simple type support #145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Nice! I was was alerted to this only now. |
e3effa4 to
837e2b5
Compare
Signed-off-by: beltram <[email protected]>
|
Alright, there was some missing pieces to make this work (especially when the SimpleType is in a Mapping or an Array for instance). This works now and the fuzzer is happy. I agree with your point @cabo: however since this library was erroring on unassigned values it could subtedly break downstream consumers ; so this would require a new minor release. @haraldh any thought on this ^ and on merging this ? |
|
@npmccallum your call |
|
@npmccallum Any progress on this? |
|
@npmccallum @haraldh It sure would be nice if this functionality was incorporated. Then we would be able to use ciborium unmodified to implement SD-CWT. |
|
@rohanmahy I'm not against this change. I do want to make you aware that I'm going to soon commit a new replacement for ciborium-ll (called ciborium-atom) that will fix a number of deficiencies. This will include better simple type support. But I'm not going to hold up your contribution until then. You're currently serializing the simple type as an enum. I think a newtype would be a better option here. https://docs.rs/serde/latest/serde/de/trait.Deserializer.html#tymethod.deserialize_newtype_struct You can use the rename attribute to rename the struct to a magic name. But this should simplify the code significantly. I'll try to do a more thorough review this weekend. |
Hi,
I've crated this PR to add support for CBOR Simple types as defined in RFC 8949 Section 3.3.
I've roughly used the same approach you used for tags. I chose to fail in case a reserved simple type was used (0..=19) or (24..=31) and I think this is what causes the fuzzer to fail. He also seems not to differentiate one-byte and two-byte variants of simple types which causes some other errors in the fuzzer. I would like some pointers from your side to understand what happens under the hood and how to fix that.
Additionally, I wanted to take advantage of this PR to introduce a new variant
Value::Undefinedinstead ofValue::Simple(23). I think that's fine since the enum in#[non_exhaustive]. Let me know if that sounds reasonable to you.Any help getting this PR fixed and merged will be very appreciated :)
Thank you
Closes #60