[14] RFR JDK-8233016: javax.crypto.Cipher throws NPE for the given custom CipherSpi and Provider

Valerie Peng valerie.peng at oracle.com
Tue Nov 26 00:16:34 UTC 2019


Hi Sean,

Thanks, I have changed the bug synopsis as well as CSR title and content 
accordingly. Please find more comments inline...

On 11/25/2019 11:52 AM, Sean Mullan wrote:
> I would change the Bug summary to describe what is being proposed. 
> Since two new exceptions are being specified in the constructor, I 
> would suggest: "Specify the exceptions that can be thrown by the 
> javax.crypto.Cipher constructor".
>
> > CSR: https://bugs.openjdk.java.net/browse/JDK-8234271
>
> I would tweak the Problem section to note that an undocumented NPE can 
> be thrown in more than one case
>
> "The protected constructor of javax.crypto.Cipher class throws an 
> undocumented NullPointerException when the provider argument is null 
> and the cipherSpi argument is invalid but not null."

After looking into the impl code of the underlying check, I removed the 
part of cipherSpi argument. The actual check is performed on the call 
stack instead of the specified cipherSpi argument. This is to catch the 
case where caller passes a trusted provider (not its own provider) into 
the constructor. Kind of hard to explain and probably too much detail 
for javadoc specification, so that's why the proposed text isn't very 
specific and used the term "invalid" as a catch-all.

Thanks,
Valerie
>
> +     * @throws IllegalArgumentException if the supplied arguments
> +     *         are deemed invalid for constructing the Cipher object
>
> This is unspecified as to what invalid actually means, but I guess it 
> is probably best to not specify anything more specific. It may make 
> sense to add an @implNote as a follow-on issue as to what validation 
> is being done by the JDK implementation.
>
> > Webrev: http://cr.openjdk.java.net/~valeriep/8233016/webrev.00/
>
> Looks fine.
>
> > Release Note: https://bugs.openjdk.java.net/browse/JDK-8234609
>
> I made a couple of minor changes but otherwise looks good.
>
> --Sean
>
> On 11/21/19 8:10 PM, Valerie Peng wrote:
>> Sean,
>>
>> Can you please help reviewing this one?
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233016
>>
>> Release Note: https://bugs.openjdk.java.net/browse/JDK-8234609
>>
>> Webrev: http://cr.openjdk.java.net/~valeriep/8233016/webrev.00/
>>
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8234271
>>
>> Thanks,
>>
>> Valerie



More information about the security-dev mailing list