webrev.09 updated at the same URL. + Certificate.Builder#buildSelfSignedCertificate(KeyPair) - X509Certificate.Builder#buildCertificate(CertificateRequest, KeyPair, X500Principal)
--Max > On Jan 13, 2016, at 9:02 AM, Wang Weijun <weijun.w...@oracle.com> wrote: > > A new webrev at > > http://cr.openjdk.java.net/~weijun/8058778/webrev.09/ > http://cr.openjdk.java.net/~weijun/8058778/webrev.09/specdiff/java/security/cert/package-summary.html > > Changes from webrev.08: > > 1. X509Certificate.Builder addAuthorityKeyIdentifierExtension(); > X509Certificate.Builder addSubjectKeyIdentifierExtension(); > > 2. X509Extension getRawExtensionValue(String oid) > > 3. Spec changes we discussed. > > Still one TODO in X509Certificate.Builder subject(String name). > > Some comments below in line. > >> On Jan 13, 2016, at 5:58 AM, Sean Mullan <sean.mul...@oracle.com> wrote: >> >> A few more comments for now, but I'll need another day or so to finish my >> review: >> >> * General >> >> Use @throws instead of @exception > > I'll fix all new lines. Do I need to touch existing ones? > >> >> * X509Certificate >> >> lines 572-585 were removed, but where was it copied? It is not in >> GeneralName and probably should not be unless we add a toString method. > > I moved them to Buidler#newGeneralName(Type,String). "See {@link > GeneralName}" should be "See {@link Builder#newGeneralName(GeneralName.Type, > String)} for the formats". > > Or is it always better to keep them in the existing API and add a link the > new one? > >> >> 847 * @exception IllegalArgumentException if {@code name} >> 848 * is not a valid signature algorithm name. TODO: really? >> >> Agree, you can't detect this until the certificate is built/signed, so I >> think you should remove it, and add a note that the signature algorithm will >> not be checked for availability until it is built or signed. >> >> 867 * If Both this method and {@link #setSigAlgName} are called, the >> >> s/Both/both/ >> >> * CertificateRequest >> >> 125 * @return the encoded form of this certificate request >> 126 */ >> 127 public abstract byte[] getEncoded(); >> >> Should say that it returns a new byte array each time it is called. > > OK. > > > Thanks > Max > >> >> --Sean >> >> On 01/11/2016 02:59 AM, Wang Weijun wrote: >>> Once again >>> >>> http://cr.openjdk.java.net/~weijun/8058778/webrev.08/ >>> http://cr.openjdk.java.net/~weijun/8058778/webrev.08/specdiff/java/security/cert/package-summary.html >>> >>> Changes: >>> >>> - GeneralName is now a standalone interface. Still no getType(), useless >>> >>> - Two newGeneralName, the binary one is simply newGeneralName(byte[]) which >>> accepts every encoding including those having a string value >>> >>> There is still one TODO: >>> >>> We used to have subject(String) and subject(X500Principal), but on the >>> issuer side there is only one >>> >>> buildCertificate(CertificateRequest, KeyPair, X500Principal) >>> >>> seems not the same level. I'd prefer to remove subject(String). It's just >>> a short form and no more efficient than subject(X500Principal). >>> >>> Thanks >>> Max