> On Dec 19, 2018, at 7:24 AM, Valerie Peng <valerie.p...@oracle.com> wrote:
>
> 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/
Looks fine to me.
>
> 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?
Sure.
Thanks,
Max
>
> 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
>>>>>
>>>>>
