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
