Thanks Sean and Max for the review.
I have updated the webrev with Max's comment regarding line 87 and 100.
http://cr.openjdk.java.net/~valeriep/8214096/webrev.01/
Will proceed with integration once the mach5 job is done.
As for calling setParameter(...) with null value, people may have
different opinions on how it should behave and what it means.
Personally, I am not too sure what is the expected behavior regarding to
parameters when key and parameters are passed in separate methods. We
should think it through before we update the javadoc. As Sean said, too
late for this bug fix at this stage. We can track this in a separate bug?
Valerie
On 12/18/2018 5:22 AM, Weijun Wang wrote:
On Dec 18, 2018, at 9:14 PM, Sean Mullan <sean.mul...@oracle.com> wrote:
On 12/17/18 10:14 PM, Weijun Wang wrote:
Hi Valerie,
Please put lines 87 and 100 into the if-not-null block. Otherwise fine.
Do you think we can enhance the Signature::setParameter method and claim a null
parameter is not meaningful at all and should not have any effect on the
internal state of the signature object? Otherwise an application really has no
idea whether to call it.
That would be a specification change, so it can't be as part of this fix, since
it is past RDP.
I understand.
It is also has a somewhat high compatibility risk, since it would require
existing 3rd-party providers (such as BouncyCastle) that throw NPE to change
their implementation.
Even if we do not specify that a certain exception must be thrown when null is
used, we need to make sure setParameter(null) is completely useless, which
means an implementation shall not use it to do any meaningful thing to set (or
reset, or reinitialize) anything. The following codes must be equivalent to
no-op:
try {
sig.setParameter(null);
} catch (Exception e) {
//
}
Thanks,
Max
It is unfortunate that the behavior of a null parameter was never clearly
defined.
--Sean
Thanks,
Max
On Dec 18, 2018, at 8:41 AM, Valerie Peng <valerie.p...@oracle.com> wrote:
Any one has time to review this straightforward fix? Details on cause and fix
is elaborated in the link below:
Bug: https://bugs.openjdk.java.net/browse/JDK-8214096
Webrev can be found at http://cr.openjdk.java.net/~valeriep/8214096/webrev.00/
Regards,
Valerie
