RFR: 8308808: SunMSCAPI public keys returns internal key array [v2]

Sean Mullan mullan at openjdk.org
Thu Jun 15 15:10:03 UTC 2023


On Wed, 14 Jun 2023 20:38:18 GMT, Ben Perez <duke at openjdk.org> wrote:

>> Changed `getEncoded` for both EC and RSA to return a copy of the internal key array to avoid mutability issues.
>
> Ben Perez has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Changed array copy to be more compact

src/jdk.crypto.mscapi/windows/classes/sun/security/mscapi/CPublicKey.java line 96:

> 94:                 }
> 95:             }
> 96:             return encoding.clone();

Missed this before, but if the code on line 89 throws an Exception, then encoding will be null, so you also want to check for null on line 96 to avoid an NPE, i.e. `return (encoding == null) ? null : encoding.clone();`

Same comment on line 182.

test/jdk/sun/security/mscapi/EncodingMutability.java line 5:

> 3:  * @bug 8308808
> 4:  * @requires os.family == "windows"
> 5:  * @modules jdk.crypto.mscapi/sun.security.mscapi

I think you just need `jdk.crypto.mscapi` on this line as you are not accessing the internal `sun.security.mscapi` classes.

test/jdk/sun/security/mscapi/EncodingMutability.java line 9:

> 7:  */
> 8: 
> 9: import java.security.*;

Suggest including each import instead of wildcarding as there are not that many.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14425#discussion_r1231145007
PR Review Comment: https://git.openjdk.org/jdk/pull/14425#discussion_r1231159008
PR Review Comment: https://git.openjdk.org/jdk/pull/14425#discussion_r1231154401



More information about the security-dev mailing list