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 <[email protected]> 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 >
