On Fri, 13 Jun 2025 17:32:12 GMT, Valerie Peng <valer...@openjdk.org> wrote:
> Based on the javadoc of `javax.crypto.Cipher` class, the cipher > transformation should be either "algorithm/mode/padding" or > "algorithm". When parsing the transformation, space(s) is trimmed off and > empty strings are considered as "unspecified". This PR adds checks to ensure > that transformations with empty "mode" and/or "padding" value in the > "algorithm/mode/padding" form leads to `NoSuchAlgorithmException`. This > reverts some changes made in > [https://bugs.openjdk.org/browse/JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) > which allows empty mode and/or padding in the transformations. > > Tier1 - 3 Mach5 job: > https://mach5.us.oracle.com/mdash/jobs/vpeng-jdkOh3-20250613-0441-29515109 > JCK javax_crypto and java_security Mach5 job: > https://mach5.us.oracle.com/mdash/jobs/vpeng-jdkOh3-20250613-0624-29519811 > > Thanks in advance for the review~ src/java.base/share/classes/javax/crypto/Cipher.java line 348: > 346: transformation.substring(0, endIdx).trim()); > 347: if (algo.isEmpty()) { > 348: throw new NoSuchAlgorithmException("Invalid transformation: > " + I don't think this exception is tested. It could be by adding `test(" ", provider);` in [test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java](https://github.com/openjdk/jdk/pull/25808/files#diff-5442ba4fbe85af64a99ca3c2d2b5795026be4c40e23af8337d52a64cefd6af35) on line 68 test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java line 75: > 73: try { > 74: Cipher c = Cipher.getInstance(transformation, provider); > 75: throw new RuntimeException("Expected NSAE not thrown"); Minor: Do you think it would be a good idea to print out `transformation` and `provider` data for debug? In `System.err` that is. Otherwise afaik the exception is thrown and no trace in system error, just in `System.out.println`, making debugging quite challenging Just something like this ```suggestion System.err.println("Error while testing " + transformation); throw new RuntimeException("Expected NSAE not thrown"); or just add it to the `RuntimeException` message ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25808#discussion_r2150062308 PR Review Comment: https://git.openjdk.org/jdk/pull/25808#discussion_r2149939460
