crypto/cryptoJava: Add support for parsing Subject Alternative Names …#91
crypto/cryptoJava: Add support for parsing Subject Alternative Names …#91rnschwalm wants to merge 2 commits intofantom-lang:masterfrom
Conversation
…from CSRs and X509 certificates
mgiannini
left a comment
There was a problem hiding this comment.
A few small things to clean up.
| // Construction | ||
| ////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| static San dns(Str name) { San(SanType.dNSName, name) } |
There was a problem hiding this comment.
All these convenience methods should be static constructors
static new dns(Str name) { ... }
static new email(Str email) { ... }
etc.| static San ip(Obj ip) | ||
| { | ||
| IpAddr := Type.find("inet::IpAddr") | ||
| if (ip is Str) return San(SanType.iPAddress, IpAddr.make([(Str)ip])) |
There was a problem hiding this comment.
you don't need this cast. You can just call make as:
IpAddr.make([ip])| if (((Uri)uri).isRel) throw ArgErr("Parameter must not be a relative Uri") | ||
| return San(SanType.uniformResourceIdentifier, uri.toStr) | ||
| } | ||
| else if (uri is Str) |
There was a problem hiding this comment.
this is duplicate code. if uri is a Str you should call uri again with a Uri:
else if (uri is Str) return uri(Uri.fromStr(uri))| this.val = val | ||
| } | ||
|
|
||
| static new fromValue(Obj value) |
There was a problem hiding this comment.
I think you at least need to document this behavior for Str (i.e. it assumes DNS (and not email, or other Str type)). Probably the docs you have in CertSigner should be here ,and just reference this method for the suported convenience values
|
|
||
| private static AsnObj toAsn(San san) | ||
| { | ||
| tagId := ((SanType)san.type).tagId |
There was a problem hiding this comment.
i think this cast is unnecessary
tagId := san.type.tagId
| case SanType.rfc822Name: | ||
| case SanType.dNSName: | ||
| case SanType.uniformResourceIdentifier: | ||
| return Asn.tag(AsnTag.context(tagId).implicit).str((Str)(san.val), AsnTag.univIa5Str) |
There was a problem hiding this comment.
this (Str) cast is unnecessary
.str(san.val) should work
| return Asn.tag(AsnTag.context(tagId).implicit).octets(((IpAddr)san.val).bytes) | ||
|
|
||
| case SanType.registeredID: | ||
| return Asn.tag(AsnTag.context(tagId).implicit).oid((Str)san.val) |
There was a problem hiding this comment.
this (Str) cast is unnecessary
…from CSRs and X509 certificates