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,
--Jamil
On 08/25/2015 08:11 AM, Jamil Nimeh wrote:
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 bring this interface in.
I'll refactor this and alter the tests to match.
--Jamil
On 08/25/2015 06:40 AM, Sean Mullan wrote:
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 to make the class immutable, and would allow
you to remove the set method.
Thanks,
Sean
On 08/25/2015 04:14 AM, Xuelei Fan wrote:
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:
+ return (nonceData != null) ? nonceData.clone() : null;
Xuelei
On 8/25/2015 12:55 PM, Jamil Nimeh wrote:
Hi all,
This is a quick fix to the OCSPNonceExtension class to add a couple
defensive copies to public get/set methods.
JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8134364
Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8134364/webrev.00
Thanks,
--Jamil
