Looks good.
--Sean
On 09/01/2015 09:14 PM, Jamil Nimeh wrote:
Hi Sean, et al.,
I've updated the review to incorporate Sean's comments. Sean, I was
able to remove encodeInternal() and make it into a one-liner inside each
of the ctors, so that shrunk things down a bit more which is nice. Let
m
Hi Sean, et al.,
I've updated the review to incorporate Sean's comments. Sean, I was
able to remove encodeInternal() and make it into a one-liner inside each
of the ctors, so that shrunk things down a bit more which is nice. Let
me know what you think.
Webrev: http://cr.openjdk.java.net/~j
Hi Sean, thanks for the comments, they all sound very reasonable to me.
I'll get on fixing those now. WRT the cloning, I figured that since the
class had public visibility and the methods in question were public
methods that it was better to err on the side of safety, even with the
class bein
On 08/28/2015 09:25 PM, Jamil Nimeh wrote:
Hello all,
I've removed the CertAttrSet interface from OCSPNonceExtension and
trimmed away a few unneeded methods. As a result the class is immutable
now.
Looks a lot cleaner. Strictly speaking, the cloning is not necessary
since this is an internal
Hello all,
I've removed the CertAttrSet interface from OCSPNonceExtension and
trimmed away a few unneeded methods. As a result the class is immutable
now.
JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8134364
Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8134364/webrev.01
Thanks,
-
Hi Sean,
Yes, I was just following Extension convention in terms of implementing
CertAttrSet. Within sun.security.x509, X509CertInfo uses the set
methods from other objects implementing CertAttrSet. But
OCSPNonceExtension really isn't a certificate attribute so it probably
doesn't need to br
Is it necessary for the class to implement CertAttrSet? I realize all of
the internal X.509 extensions implement that, but that was done a long
time ago and not something we really want to carry forward if not
needed. I think it would be cleaner not to do that as it would more
easily allow you
OCSPNonceExtension.java
===
- nonceData = (byte[])obj;
+ nonceData = ((byte[])obj).clone();
Do you want to check null obj?
- return nonceData;
+ return (nonceData != null ? nonceData.clone() : null);
I think you may want to enclose the "!=" operator as:
+
