-
Notifications
You must be signed in to change notification settings - Fork 108
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
Tighten up string type representations to prevent illegal values #214
Tighten up string type representations to prevent illegal values #214
Conversation
This looks okay so far. One thought is that I think maybe the |
Maybe we could provide both? I think the PrintableString("BR".into()); to PrintableString("BR".try_into()?) If we only provide PrintableString(String::from("BR").try_into()?) |
Having both would also solve this one.
I see your point but I think it's okay because users are used to the standard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a quick review pass. So far it's looking good. Thanks for picking this up
I've updated the PR with the missing methods, and integrate it with subject alternative names.
The PR is still a draft because I would like to add more tests but I haven't found a good way yet. |
8450995
to
c352ad8
Compare
I rebased on top of #213. I've also cleanup the git history a bit |
c352ad8
to
8ff55b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thank you
@Tudyx If you rebase I think this is ready for merge. Thank you! |
d53eb3a
to
b427983
Compare
That's should be it |
Thanks again. I appreciate you wrangling this tech debt to unblock the feature work you were originally interested in doing. You went the extra mile 🏆 |
Fix #181
Mainly inspired by the der and the asn1_rs crates.