RFR 8058778: New APIs for creating certificates and certificate requests

Wang Weijun weijun.wang at oracle.com
Wed Jan 13 13:44:17 UTC 2016


Hi David

> On Jan 13, 2016, at 8:08 PM, David M. Lloyd <david.lloyd at redhat.com> wrote:
> 
> On 01/12/2016 07:02 PM, Wang Weijun wrote:
>> A new webrev at
>> 
>> http://cr.openjdk.java.net/~weijun/8058778/webrev.09/
> 
> A couple of questions/comments...
> 
>> +    public interface Builder
>> +            <S extends Certificate,T extends Builder<S,T>> {
> 
> What is the point of the "T" self-type variable?  It does not seem to be referenced.  Also the type parameters are not documented in the interface JavaDoc, or generally anywhere.

If there were a mutator defined here, it could be written as "T setSomething()" so that when called in a child interface it can return the child type instead the base type. Will add some document.

That said, I haven't applied this trick in X509Certificate.Builder, what if there is a 2nd-level child interface? I'll do some experiments.

> 
> Also in places like this....
> 
> +    @Override
> +    public <R extends CertificateRequest> R engineGenerateCertificateRequest(
> +            InputStream is, Class<R> type) throws CertificateException {
> +        if (is == null) {
> +            // clear the caches (for debugging)
> +            certCache.clear();
> +            X509CertificatePair.clearCache();
> +            throw new CertificateException("Missing input stream");
> +        }
> +        try {
> +            byte[] encoding = readOneBlock(is);
> +            if (encoding != null) {
> +                return type.cast(new PKCS10(encoding));
> +            } else {
> +                throw new IOException("Empty input");
> +            }
> +        } catch (ClassCastException e) {
> +            throw new UnsupportedOperationException("Unsupported format");
> +        } catch (Exception e2) {
> +            throw new CertificateException(e2);
> +        }
> +    }
> 
> ...it's using UOE for unsupported format, which doesn't seem right.

It should be IllegalArgumentException, or a sub-type (although I cannot find one).

> Also, it seems like you could check "type" up at the top.

I can use Class#isAssignable.

> 
> The docs don't seem to specify whether the CSR block is consumed in the event of an invalid type Class being passed in.

I just copied the existing spec for generateCertificate(), in fact in some places I haven't done s/certificate/certificate request/ yet!

The current behavior is that it will consume one ----BEGIN...END---- block (with optional text before the block) or one DER SEQUENCE, or undefined if no such data structure are found. However, I am not sure if I need to document that as a requirement. Do you want any guarantee?

Thanks
Max

> 
> -- 
> - DML




More information about the security-dev mailing list