Skip to content
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

Add support for RELATIVE-OID #48

Merged
merged 2 commits into from
Feb 15, 2024
Merged

Conversation

russhousley
Copy link

Add support for RELATIVE-OID

Copy link

@CBonnell CBonnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I have not previously contributed to pyasn1, I use the library extensively and have a decent understanding of the library internals by implementing a Rust-based "pedantic DER decode" function as part of the cbonnell/pyasn1-fasder library.

I reviewed this PR and believe it is ready. Specifically, the following items are addressed by the PR:

  1. Creation of a class (RelativeOID) to represent RELATIVE OID values
  2. Creation of the appropriate encoder and decoder classes to serialize/deserialize RELATIVE OID instances. Since the encoding of RELATIVE OIDs is identical for BER, CER, and DER, only the BER encoder and decoder are required. The native encoder and decoder is also added.
  3. Registration of the unambiguous typeId in the TYPE_MAP and as well as registration of the the RELATIVE OID tagSet in the TAG_MAP in the encoder and decoder
  4. Correct enforcement of encoding requirements as prescribed in X.690. Namely, the prohibition on leading 0x80 values in RELATIVE OID subcomponents is enforced.
  5. A comprehensive set of unit tests is included.
  6. Appropriate documentation for the newly supported type is added.

I hope this review is useful in determining suitability for merging this PR. Additionally, I'd be happy to answer any questions about this review.

Copy link

@droideck droideck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@russhousley Finished reviewing. And it looks really great! Thank you!

@CBonnell And, thank you so much for an excellent and detailed review. It helped a lot!

@droideck
Copy link

@russhousley, feel free to squash and write the commit message how you want, and we can merge it then. :)

@russhousley
Copy link
Author

This is the first time I have done squash. I hope I did it properly.

@droideck droideck merged commit d17e0e1 into pyasn1:main Feb 15, 2024
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants