Description
After writing a custom extension (#1490), I wanted to parse it from a certificate. The AsExtension
trait has no decoding-related constraints, and neither it nor CertificateBuilder::add_extension
document what to do.
After writing my own custom parsing logic, I then (while digging through the x509-cert
code for an unrelated issue) discovered TbsCertificateInner::{get, filter}
. TbsCertificateInner::get
is exactly what I needed. However, it was impossible to discover due to its very opaque method name and being buried well below the top-level Certificate
type.
I am guessing that the method names get
and filter
were chosen to follow a similar naming scheme to the standard Rust container types. However, certificates are not pure containers, and some certificate versions don't even support extensions, so having the untyped get
method be how you get certificate extensions is very unintuitive. I think the methods should be renamed to e.g. get_extension
and filter_extensions
.
The methods should also be extensively documented, both on themselves and on Certificate
(which is where a user will be looking for functionality, particularly after it is made opaque in #1486). In particular, these two aspects should be documented:
- The currently-named
TbsCertificateInner::filter
should be documented as invalid / unsafe to use with the RFC 5280 profile, because RFC5280 forbids having duplicate extensions, but does not specify how errors should be handled. If Rust had specialization yet then I'd recommend an alternative implementation for theRfc5280
concrete type that always returns an error (or better yet, prevents the method from being accessible at all), but at a minimum the documentation has to make this explicitly clear to the user. - The return value
(bool, T)
is currently called "the extension", but given thatT
is the user-requested extension type, thebool
is completely undocumented.
It would also be beneficial to either update the AsExtension
trait documentation, or somewhere else where custom extensions are defined, to document or give examples of the full "custom extension" process, trait impls, and interaction patterns. (In particular, someone who impls AsExtension
and then goes looking for how to use it for parsing will be out of luck, because AsExtension
is not used anywhere on the parsing side. Also, my concerns about der::Encode
in #1490 also apply to der::Decode
now that I know it is necessary, plus things like "what happens if there is excess data left over after I've parsed what I expect?")