librasn / rasn

A Safe #[no_std] ASN.1 Codec Framework
Other
183 stars 43 forks source link

Feature/jer #187

Closed 6d7a closed 7 months ago

6d7a commented 8 months ago

Adds support for JSON encoding rules

JER-support is implemented as a non-default feature, both in rasn as the feature jer, as well as in rasn-derive as the feature text-based-encodings (changes in rasn-derive will likely be needed for XER handling, too). Encoder and decoder are built upon serde_json.

Closes #188

109

XAMPPRocky commented 8 months ago

Thank you for your PR!

I have two initial bits of feedback, the first is that I don't think this should be a feature. If we want to go for that approach we should do that separately and apply it uniformly to the other codecs.

The second is the use of serde. Since we don't benefit from the generic codec framework of serde, since we are a codec framework ourselves, wouldn't we be better off using a purpose built json library, like jzon?

6d7a commented 8 months ago

jzon looks indeed promising, I overlooked it designing the JER implementation. The API is similar to serde_json so it won't take much effort ro refactor. Seeing that jzon doesn't have any further dependencies I don't see any harm in not making JER a feature. I'll follow up with an update.

6d7a commented 8 months ago

@XAMPPRocky I replaced serde_json with jzon and moved the jer feature into the main codebase. I also made a small change to Codec in order to handle text-based encoding rules. Unfortunately, some fuzz tests fail under specific builds and I'm having a hard time figuring out why. Do you have an idea? Thanks!

XAMPPRocky commented 7 months ago

Thank you for your PR!